Skip to content

Commit

Permalink
Make NULL check in argument parsing the same on all unix platforms
Browse files Browse the repository at this point in the history
  • Loading branch information
@madsmtm
madsmtm committed May 20, 2024
1 parent abd5d0e commit 38ad851
Showing 1 changed file with 64 additions and 76 deletions.
140 changes: 64 additions & 76 deletions library/std/src/sys/pal/unix/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@

#![allow(dead_code)] // runtime init functions not used during testing

use crate::ffi::OsString;
use crate::ffi::{CStr, OsString};
use crate::fmt;
use crate::os::unix::ffi::OsStringExt;
use crate::vec;

/// One-time global initialization.
Expand All @@ -16,7 +17,46 @@ pub unsafe fn init(argc: isize, argv: *const *const u8) {

/// Returns the command line arguments
pub fn args() -> Args {
imp::args()
let (argc, argv) = imp::argc_argv();

let mut vec = Vec::with_capacity(argc as usize);

for i in 0..argc {
// SAFETY: `argv` is non-null if `argc` is positive, and it is
// guaranteed to be at least as long as `argc`, so reading from it
// should be safe.
let ptr = unsafe { argv.offset(i).read() };

// Some C commandline parsers (e.g. GLib and Qt) are replacing already
// handled arguments in `argv` with `NULL` and move them to the end.
//
// Since they can't directly ensure updates to `argc` as well, this
// means that `argc` might be bigger than the actual number of
// non-`NULL` pointers in `argv` at this point.
//
// To handle this we simply stop iterating at the first `NULL`
// argument. `argv` is also guaranteed to be `NULL`-terminated so any
// non-`NULL` arguments after the first `NULL` can safely be ignored.
if ptr.is_null() {
// NOTE: On Apple platforms, `-[NSProcessInfo arguments]` does not
// stop iterating here, but instead `continue`, always iterating
// up until it reached `argc`.
//
// This difference will only matter in very specific circumstances
// where `argc`/`argv` have been modified, but in unexpected ways,
// so it likely doesn't really matter which option we choose.
// See the following PR for further discussion:
// <https://github.com/rust-lang/rust/pull/125225>
break;
}

// SAFETY: Just checked that the pointer is not NULL, and arguments
// are otherwise guaranteed to be valid C strings.
let cstr = unsafe { CStr::from_ptr(ptr) };
vec.push(OsStringExt::from_vec(cstr.to_bytes().to_vec()));
}

Args { iter: vec.into_iter() }
}

pub struct Args {
Expand Down Expand Up @@ -75,9 +115,7 @@ impl DoubleEndedIterator for Args {
target_os = "hurd",
))]
mod imp {
use super::Args;
use crate::ffi::{CStr, OsString};
use crate::os::unix::prelude::*;
use crate::ffi::c_char;
use crate::ptr;
use crate::sync::atomic::{AtomicIsize, AtomicPtr, Ordering};

Expand Down Expand Up @@ -126,45 +164,19 @@ mod imp {
init_wrapper
};

pub fn args() -> Args {
Args { iter: clone().into_iter() }
}

fn clone() -> Vec<OsString> {
unsafe {
// Load ARGC and ARGV, which hold the unmodified system-provided
// argc/argv, so we can read the pointed-to memory without atomics
// or synchronization.
//
// If either ARGC or ARGV is still zero or null, then either there
// really are no arguments, or someone is asking for `args()`
// before initialization has completed, and we return an empty
// list.
let argv = ARGV.load(Ordering::Relaxed);
let argc = if argv.is_null() { 0 } else { ARGC.load(Ordering::Relaxed) };
let mut args = Vec::with_capacity(argc as usize);
for i in 0..argc {
let ptr = *argv.offset(i) as *const libc::c_char;

// Some C commandline parsers (e.g. GLib and Qt) are replacing already
// handled arguments in `argv` with `NULL` and move them to the end. That
// means that `argc` might be bigger than the actual number of non-`NULL`
// pointers in `argv` at this point.
//
// To handle this we simply stop iterating at the first `NULL` argument.
//
// `argv` is also guaranteed to be `NULL`-terminated so any non-`NULL` arguments
// after the first `NULL` can safely be ignored.
if ptr.is_null() {
break;
}

let cstr = CStr::from_ptr(ptr);
args.push(OsStringExt::from_vec(cstr.to_bytes().to_vec()));
}

args
}
pub fn argc_argv() -> (isize, *const *const c_char) {
// Load ARGC and ARGV, which hold the unmodified system-provided
// argc/argv, so we can read the pointed-to memory without atomics or
// synchronization.
//
// If either ARGC or ARGV is still zero or null, then either there
// really are no arguments, or someone is asking for `args()` before
// initialization has completed, and we return an empty list.
let argv = ARGV.load(Ordering::Relaxed);
let argc = if argv.is_null() { 0 } else { ARGC.load(Ordering::Relaxed) };

// Cast from `*mut *const u8` to `*const *const c_char`
(argc, argv.cast())
}
}

Expand All @@ -183,16 +195,14 @@ mod imp {
// of this used `[[NSProcessInfo processInfo] arguments]`.
#[cfg(target_vendor = "apple")]
mod imp {
use super::Args;
use crate::ffi::{c_char, c_int, CStr};
use crate::os::unix::prelude::*;
use crate::ffi::{c_char, c_int};

pub unsafe fn init(_argc: isize, _argv: *const *const u8) {
// No need to initialize anything in here, `libdyld.dylib` has already
// done the work for us.
}

pub fn args() -> Args {
pub fn argc_argv() -> (isize, *const *const c_char) {
extern "C" {
// These functions are in crt_externs.h.
fn _NSGetArgc() -> *mut c_int;
Expand All @@ -212,42 +222,20 @@ mod imp {
// SAFETY: Same as above.
let argv = unsafe { _NSGetArgv().read() };

let mut vec = Vec::with_capacity(argc as usize);

for i in 0..argc {
// SAFETY: `argv` is at least as long as `argc`, so reading from
// it should be safe.
let ptr = unsafe { argv.offset(i as isize).read() };

// Entries may have been removed from `argv` by setting them to
// NULL, without updating `argc`.
if ptr.is_null() {
// We continue instead of break here, as an argument may have
// been set to `NULL` in the middle, instead of at the end of
// the list.
//
// This is the same as what `-[NSProcessInfo arguments]` does.
continue;
}

// SAFETY: Just checked that the pointer is not NULL, and
// arguments are otherwise guaranteed to be valid C strings.
let cstr = unsafe { CStr::from_ptr(ptr) };
vec.push(OsStringExt::from_vec(cstr.to_bytes().to_vec()));
}

Args { iter: vec.into_iter() }
// Cast from `*mut *mut c_char` to `*const *const c_char`
(argc as isize, argv.cast())
}
}

#[cfg(any(target_os = "espidf", target_os = "vita"))]
mod imp {
use super::Args;
use crate::ffi::c_char;
use crate::ptr;

#[inline(always)]
pub unsafe fn init(_argc: isize, _argv: *const *const u8) {}

pub fn args() -> Args {
Args { iter: Vec::new().into_iter() }
pub fn argc_argv() -> (isize, *const *const c_char) {
(0, ptr::null())
}
}

0 comments on commit 38ad851

Please sign in to comment.