From 6be5c06b88aeeacbb114f5a96d27b94ca0c81e73 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 5 Aug 2026 23:43:14 +0900 Subject: [PATCH 1/5] launcher: carry a command-line argument in the host's own spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pyre script.py $'bad\xff'` aborted the process. `std::env::args()` unwraps the UTF-8 conversion of every element, so the panic landed before parsing began and before anything could report it; CPython answers `sys.argv[1] == 'bad\udcff'` and exits 0. `targetpypystandalone.py:76-80` builds the list with `space.newfilename`, which is `fsdecode(newbytes(s))`, so an argument the filesystem encoding cannot spell arrives as the surrogate escape that re-encodes to the original byte. The arguments now stay in the host's own spelling until that decode: a Rust `String` cannot hold the escape, so narrowing anywhere earlier loses it. lexopt already hands out `OsString` — `RawArgs` yields it and `Arg::Value` carries it — so `drain_args` was doing the narrowing itself, and dropping `.string()?` there is what carries `sys.argv[1:]` through. The four run modes keep a `String` argv[0]: `-c` and stdin choose theirs from a literal, and a non-UTF-8 `-m` argument or script path is a separate case — those narrow at the parse, before `drain_args` is reached, and the payload flows on into the import machinery and the source reader rather than into a list. `gateway::fsdecode_os_str` is the decode, split the way the tree's other inbound OS-string boundaries are split — on `windows`, not on `unix`. Where the argument is bytes it takes the filesystem decode; where it is UTF-16 the host already has the code units and `Wtf8Buf::from_wide` carries them across, since routing those through the byte decode would turn an unpaired surrogate into three escapes and stop it round-tripping. `pyre-wasm-runner` had the same `std::env::args()` abort on its own positional script path. Its flags are ASCII by construction, so a value that does not convert is never one and takes the positional arm. `extra_tests/parity_tests/argv_undecodable_argument.py` passes the argument to a child, so it needs no such name on disk and runs wherever `execve` does; it self-skips on win32. Verified against CPython 3.14.5 first, then both backends. Not covered, measured and tracked separately. `-W` and `-X` option *values*: CPython reports `sys.warnoptions == ['ignore\udcff']` and exits 0 where this exits 2; those are inspected as text, folded with PYTHONWARNINGS by splitting on a comma, and carried across the wasm launch-env transport, so they move on their own. `-m` and the script path: CPython takes the argument and reports what it could not do with it — `No module named ba\udcffd` at exit 1, and `can't open file '…bad\udcff.py'` at exit 2 — where this rejects the argument itself. The script-path half cannot be exercised on APFS, which refuses such a name outright. Assisted-by: Claude --- .../parity_tests/argv_undecodable_argument.py | 67 +++++++++++++++++++ pyre/pyre-interpreter/src/gateway.rs | 24 +++++++ pyre/pyre-interpreter/src/importing.rs | 27 ++++++-- pyre/pyre-interpreter/src/module/sys/vm.rs | 2 +- pyre/pyre-jit/tests/gc_stress.rs | 2 +- pyre/pyre-wasm-runner/src/main.rs | 26 ++++--- pyre/pyrex/src/lib.rs | 31 +++++---- 7 files changed, 148 insertions(+), 31 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/argv_undecodable_argument.py diff --git a/pyre/extra_tests/parity_tests/argv_undecodable_argument.py b/pyre/extra_tests/parity_tests/argv_undecodable_argument.py new file mode 100644 index 00000000000..42330ee8529 --- /dev/null +++ b/pyre/extra_tests/parity_tests/argv_undecodable_argument.py @@ -0,0 +1,67 @@ +"""A command-line argument with no UTF-8 spelling reaches `sys.argv` as itself. + +`targetpypystandalone.py:76-80` builds `sys.argv` with `space.newfilename`, +which is `fsdecode(newbytes(s))`, so an argument carrying a byte the filesystem +encoding cannot spell arrives as the surrogate escape that re-encodes to that +byte — not rejected, not replaced. `sys.orig_argv` carries the same value. + +The argument is passed to a child, so the test needs no such name on disk: the +filesystem never sees it, only `execve` does. Windows has no byte argv at all +and takes the wide command line, so this shape does not exist there. +""" + +import os +import subprocess +import sys + +if sys.platform == "win32": + print("OK") + raise SystemExit + +UNDECODABLE = b"pyre_undecodable_\xff" +ESCAPED = os.fsdecode(UNDECODABLE) + +# The escape is what the filesystem decode produces, and it round-trips. +assert ESCAPED.endswith("\udcff"), ascii(ESCAPED) +assert os.fsencode(ESCAPED) == UNDECODABLE, ascii(ESCAPED) + +CHILD = r""" +import os, sys +assert sys.argv[1:] == [os.fsdecode(%r), "plain"], ascii(sys.argv) +assert os.fsencode(sys.argv[1]) == %r, ascii(sys.argv[1]) +# `orig_argv` is the launcher's own line, so the argument appears there too, +# with the same escaping. +assert sys.argv[1] in sys.orig_argv, ascii(sys.orig_argv) +assert sys.orig_argv[-2:] == sys.argv[1:], ascii(sys.orig_argv) +print("child ok") +""" % (UNDECODABLE, UNDECODABLE) + +result = subprocess.run( + [sys.executable, "-c", CHILD, ESCAPED, "plain"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, +) +assert result.returncode == 0, (result.returncode, result.stderr) +assert result.stdout == b"child ok\n", result.stdout + +# A script run the same way answers with the argument in argv[1], and the +# script's own path stays argv[0]. +import tempfile + +with tempfile.TemporaryDirectory() as tmp: + script = os.path.join(tmp, "show_argv.py") + with open(script, "w") as f: + f.write( + "import os, sys\n" + "print(os.fsencode(sys.argv[0]) == os.fsencode(%r))\n" % script + + "print(ascii(sys.argv[1]))\n" + ) + result = subprocess.run( + [sys.executable, script, ESCAPED], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + assert result.returncode == 0, (result.returncode, result.stderr) + assert result.stdout == b"True\n" + ascii(ESCAPED).encode() + b"\n", result.stdout + +print("OK") diff --git a/pyre/pyre-interpreter/src/gateway.rs b/pyre/pyre-interpreter/src/gateway.rs index 238060e16de..4a3782f726a 100644 --- a/pyre/pyre-interpreter/src/gateway.rs +++ b/pyre/pyre-interpreter/src/gateway.rs @@ -1551,6 +1551,30 @@ pub fn fsdecode_filename_wtf8(data: &[u8]) -> rustpython_wtf8::Wtf8Buf { crate::typedef::charp2uni_wtf8(data) } +/// The application-level spelling of a string the host handed us, for a caller +/// holding an `OsString` rather than the bytes behind it — a command-line +/// argument, where `targetpypystandalone.py:76-80` builds `sys.argv` out of +/// `space.newfilename` for exactly this reason. +/// +/// The two arms are the host's two spellings, not a portability shim. Where the +/// argument is bytes it takes the filesystem decode, so a byte with no UTF-8 +/// form comes back as the surrogate escape that re-encodes to itself. Where it +/// is UTF-16 the host already has the code units, and `from_wide` carries them +/// across losslessly; routing those through the byte decode instead would turn +/// an unpaired surrogate into three escapes and stop it round-tripping. +pub fn fsdecode_os_str(name: &std::ffi::OsStr) -> pyre_object::PyObjectRef { + #[cfg(windows)] + { + use std::os::windows::ffi::OsStrExt; + let units: Vec = name.encode_wide().collect(); + pyre_object::w_str_from_wtf8(rustpython_wtf8::Wtf8Buf::from_wide(&units)) + } + #[cfg(not(windows))] + { + fsdecode_filename_bytes(name.as_encoded_bytes()) + } +} + /// `interp_posix.py:194-219 Path`: the syscall spelling and the resolved path /// object travel together. For `os.PathLike`, `w_path` is the result of the /// single `__fspath__` call, not the wrapper that supplied it. diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 11f1f6cc7f2..511f224103a 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -1710,12 +1710,20 @@ pub(crate) unsafe fn walk_process_import_roots(visitor: &mut dyn FnMut(&mut PyOb /// Set the Python-visible sys.modules dict reference. Called during sys /// module initialization so subsequent set_sys_module calls keep it in sync. /// Also copies all previously cached modules into the dict. -/// Set sys.argv from a list of strings. +/// Set sys.argv from the arguments the host gave the process. /// Must be called after the first `import sys` has run (e.g. after /// `run_source` compiles the module-level code). -pub fn set_sys_argv(args: &[String]) { - let items: Vec = - args.iter().map(|s| pyre_object::w_str_new(s)).collect(); +/// +/// `targetpypystandalone.py:76-80` builds the list with `space.newfilename`, +/// which is `fsdecode(newbytes(s))`, so an argument the filesystem encoding +/// cannot spell arrives as the surrogate escape rather than being rejected or +/// replaced. The arguments stay in the host's own spelling until here for that +/// reason: a Rust `String` cannot hold the escape. +pub fn set_sys_argv(args: &[std::ffi::OsString]) { + let items: Vec = args + .iter() + .map(|s| crate::gateway::fsdecode_os_str(s)) + .collect(); let argv = pyre_object::w_list_new(items); SYS_ARGV_PENDING.with(|p| p.set(argv)); } @@ -1748,7 +1756,8 @@ static SYS_UNBUFFERED: AtomicBool = AtomicBool::new(false); // dict. Preserve that owner/storage shape rather than introducing a map here. static SYS_XOPTIONS: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); static SYS_WARNOPTIONS: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); -static SYS_ORIG_ARGV: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); +static SYS_ORIG_ARGV: LazyLock>> = + LazyLock::new(|| Mutex::new(Vec::new())); static SYS_STDIO_ENCODING: LazyLock>> = LazyLock::new(|| Mutex::new(None)); /// Record whether the launcher was given `-S` (no `site` import), so the @@ -1806,11 +1815,15 @@ pub fn stdio_encoding() -> Option { SYS_STDIO_ENCODING.lock().unwrap().clone() } -pub fn set_sys_orig_argv(argv: Vec) { +/// `app_main.py:1239 sys.orig_argv[:] = [executable] + argv`: the launcher's +/// own arguments, before parsing rewrote them into the run mode and `sys.argv`. +/// Held in the host's spelling for the same reason `set_sys_argv` takes it — +/// an argument with no UTF-8 form has to survive to the decode. +pub fn set_sys_orig_argv(argv: Vec) { *SYS_ORIG_ARGV.lock().unwrap() = argv; } -pub fn sys_orig_argv() -> Vec { +pub fn sys_orig_argv() -> Vec { SYS_ORIG_ARGV.lock().unwrap().clone() } diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index fcaadcb9212..b92545b9df6 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -1022,7 +1022,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { w_list_new( crate::importing::sys_orig_argv() .iter() - .map(|arg| w_str_new(arg)) + .map(|arg| crate::gateway::fsdecode_os_str(arg)) .collect(), ), ); diff --git a/pyre/pyre-jit/tests/gc_stress.rs b/pyre/pyre-jit/tests/gc_stress.rs index 13db29cc190..1cfb5742d70 100644 --- a/pyre/pyre-jit/tests/gc_stress.rs +++ b/pyre/pyre-jit/tests/gc_stress.rs @@ -54,7 +54,7 @@ fn run_harness(program: &str, name: &str, vacuity_label: &str) -> Result<(), Str // This harness never imports `site`, so perform the post-site `sys.path[0]` // insert directly. importing::add_sys_path_0(); - importing::set_sys_argv(&[name.to_string()]); + importing::set_sys_argv(&[std::ffi::OsString::from(name)]); let code = compile_source_with_filename(program, Mode::Exec, name) .map_err(|e| format!("compile error: {e}"))?; diff --git a/pyre/pyre-wasm-runner/src/main.rs b/pyre/pyre-wasm-runner/src/main.rs index d86b8cedd5f..27256edb092 100644 --- a/pyre/pyre-wasm-runner/src/main.rs +++ b/pyre/pyre-wasm-runner/src/main.rs @@ -163,31 +163,39 @@ fn main() { let mut inspect = false; let mut engine: Option = None; - let mut argv = std::env::args().skip(1); + // `args_os`, not `args`: the latter unwraps the UTF-8 conversion, so a + // script path spelling a byte with no UTF-8 form aborts the runner before + // it can name the file. Every flag below is ASCII by construction, so a + // value that does not convert is never one and takes the positional arm. + let mut argv = std::env::args_os().skip(1); while let Some(arg) = argv.next() { - match arg.as_str() { - "--inspect" => inspect = true, - "--module" => { + let flag = arg.to_str().map(str::to_owned); + match flag.as_deref() { + Some("--inspect") => inspect = true, + Some("--module") => { module_path = Some(PathBuf::from( argv.next() .unwrap_or_else(|| fatal("--module needs a path")), )) } - "--engine" => { + Some("--engine") => { let v = argv .next() .unwrap_or_else(|| fatal("--engine needs a value")); - engine = Some(WasmEngine::parse(&v).unwrap_or_else(|e| fatal(&e))); + let v = v + .to_str() + .unwrap_or_else(|| fatal("--engine needs a value")); + engine = Some(WasmEngine::parse(v).unwrap_or_else(|e| fatal(&e))); } - "-h" | "--help" => { + Some("-h") | Some("--help") => { eprintln!( "usage: pyre-wasm-runner [--module ] \ [--engine wasmtime|wasmi] [--inspect] " ); std::process::exit(2); } - other if other.starts_with('-') => fatal(&format!("unknown flag {other}")), - other => script = Some(PathBuf::from(other)), + Some(other) if other.starts_with('-') => fatal(&format!("unknown flag {other}")), + _ => script = Some(PathBuf::from(arg)), } } diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index 5d2f36dfd35..609ae4771a8 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -83,12 +83,8 @@ interact [--tmp DIR] [--lib DIR] [--timeout SECS] [--heapsize N] [--log FILE] [- /// Drain the parser's remaining raw arguments to become `sys.argv[1:]`. /// `-c`, `-m`, and a script path each terminate option parsing, so anything /// after them belongs to the program rather than the launcher. -fn drain_args(parser: &mut lexopt::Parser) -> Result, lexopt::Error> { - let mut rest = Vec::new(); - for raw in parser.raw_args()? { - rest.push(raw.string()?); - } - Ok(rest) +fn drain_args(parser: &mut lexopt::Parser) -> Result, lexopt::Error> { + Ok(parser.raw_args()?.collect()) } /// Emit the `preconfig_init_utf8_mode` fatal error for an invalid PYTHONUTF8 / @@ -110,7 +106,9 @@ fn finalize_flags(flags: LaunchFlags) -> LaunchFlags { } } -fn parse_args(binary_name: &str) -> Result<(RunMode, LaunchFlags, Vec), lexopt::Error> { +fn parse_args( + binary_name: &str, +) -> Result<(RunMode, LaunchFlags, Vec), lexopt::Error> { let mut parser = lexopt::Parser::from_env(); let mut flags = LaunchFlags::default(); @@ -285,7 +283,14 @@ fn parse_interact(parser: &mut lexopt::Parser) -> Result Long("verbose") => verbose = true, Value(exe) => { let exe = exe.string()?; - let args = drain_args(parser)?; + // The controller's arguments are its own, not a Python + // program's: they are rendered into the child's command line + // as text, so one with no UTF-8 form is reported here rather + // than carried the way `sys.argv` carries it. + let args = drain_args(parser)? + .into_iter() + .map(|arg| arg.into_string().map_err(lexopt::Error::NonUnicodeValue)) + .collect::, _>>()?; return Ok(RunMode::Interact { exe, args, @@ -492,7 +497,7 @@ fn real_main(binary_name: &str) { // pypy/interpreter/app_main.py `entry_point`: preserve the executable and // every original launcher argument before command-line parsing rewrites // them into the run mode and `sys.argv`. - importing::set_sys_orig_argv(std::env::args().collect()); + importing::set_sys_orig_argv(std::env::args_os().collect()); let (mode, flags, args) = match parse_args(binary_name) { Ok(v) => v, Err(e) => { @@ -568,7 +573,7 @@ fn real_main(binary_name: &str) { // origins against. let cwd = sys_path_cwd(); importing::init_sys_path(&cwd, ""); - let mut argv = vec!["-c".to_string()]; + let mut argv = vec![std::ffi::OsString::from("-c")]; argv.extend(args); importing::set_sys_argv(&argv); run_source(&cmd, Mode::Exec, "", no_site); @@ -581,7 +586,7 @@ fn real_main(binary_name: &str) { // module's resolved origin via `_run_module_as_main`). let cwd = sys_path_cwd(); importing::init_sys_path(&cwd, &cwd.to_string_lossy()); - let mut argv = vec![module.clone()]; + let mut argv = vec![std::ffi::OsString::from(&module)]; argv.extend(args); importing::set_sys_argv(&argv); run_module(&module, no_site); @@ -632,7 +637,7 @@ fn real_main(binary_name: &str) { }; importing::init_sys_path(&script_dir, &script_dir.to_string_lossy()); // sys.argv[0] is the script path; remaining values go to argv[1:]. - let mut argv = vec![path.clone()]; + let mut argv = vec![std::ffi::OsString::from(&path)]; argv.extend(args); importing::set_sys_argv(&argv); // CPython compiles a script with the same absolute path exposed @@ -658,7 +663,7 @@ fn real_main(binary_name: &str) { importing::init_sys_path(&cwd, ""); // `sys.argv` is `['']` with no script argument and `['-', …]` for // an explicit dash. - let mut argv = vec![argv0]; + let mut argv = vec![std::ffi::OsString::from(argv0)]; argv.extend(args); importing::set_sys_argv(&argv); if stdin_is_interactive(inspect) { From c6aeb72c0893912c9b5e885705a1a1d5150e6ab3 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 6 Aug 2026 03:36:44 +0900 Subject: [PATCH 2/5] posix: give os.stat the descriptor and dir_fd argument forms `FsEncodedPath` carries `Path.as_fd` (`interp_posix.py:140-152`), set only by the entry points that pass `allow_fd`, and `fsencode_path_or_fd_w` names the caller in the type error so the allowed-type list can widen with it: `stat` answers "string, bytes, os.PathLike or integer" where `lstat` answers "string, bytes or os.PathLike". The descriptor arm is probed with `__index__` and sits before the PathLike arm, so an object carrying both is read as a descriptor; `-1` is turned away as `unwrap_fd` does (:269-271). `stat_entry` now has the three arms of `do_stat` (:633-649): a descriptor goes to `fstat` (extracted from the `os.fstat` closure as `fstat_fd`), a dir_fd-relative name to `fstatat` via `stat_at`, and a bare name to `stat`/`lstat`. `dir_fd` is unwrapped with `_unwrap_dirfd`'s spelling ("integer or None"), and the two ValueErrors a descriptor triggers precede the platform's dir_fd availability. The `stat_result` assembly moves into `stat_result_from_fields`, taking the fields as a `StatFields` so `std::fs::Metadata` and the raw `libc::stat` that `fstatat` fills both reach it. `_have_functions` gains HAVE_FSTATAT under the cfg where `stat_at` is implemented. os.py:120-121 reads it as `stat` and `lstat` honouring dir_fd, which `lstat` was already being advertised for through HAVE_LSTAT while raising NotImplementedError. `stat_impl` becomes `stat_path`, taking the already-unwrapped path: the TypeError rewrite in its prologue reported `stat:` for both entry points and is no longer needed now that the message is correct at its source. Assisted-by: Claude --- .../parity_tests/os_stat_file_descriptor.py | 162 +++++++++ pyre/pyre-interpreter/src/gateway.rs | 81 ++++- .../src/module/posix/interp_posix.rs | 313 ++++++++++++++---- 3 files changed, 480 insertions(+), 76 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/os_stat_file_descriptor.py diff --git a/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py b/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py new file mode 100644 index 00000000000..4423acf50a6 --- /dev/null +++ b/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py @@ -0,0 +1,162 @@ +"""`os.stat` takes an open file descriptor where `os.lstat` does not. + +`interp_posix.py:611` declares stat's path as `path_or_fd(allow_fd=True)` and +`:659` declares lstat's as `allow_fd=False`, so the descriptor form belongs to +one of them only — and that difference is also what makes their type errors name +different allowed types. + +`os.stat in os.supports_fd` is unconditionally true (`os.py:148`, "fstat always +works"), so this is the capability the set has always advertised. + +`do_stat` (`interp_posix.py:634-644`) tests the descriptor before anything else: +holding one, neither `dir_fd` nor `follow_symlinks` has a path to apply to, and +both rejections come before the platform's `dir_fd` availability is consulted. +""" + +import os +import tempfile +import warnings + +assert os.stat in os.supports_fd, "os.stat has always been advertised as fd-capable" + +tmp = tempfile.mkdtemp() +path = os.path.join(tmp, "f") +with open(path, "wb") as f: + f.write(b"0123456789") + +fd = os.open(path, os.O_RDONLY) +try: + by_fd = os.stat(fd) + by_path = os.stat(path) + assert by_fd.st_size == 10, by_fd.st_size + # The same file either way: the descriptor form is `fstat`, not a re-open. + assert (by_fd.st_ino, by_fd.st_dev) == (by_path.st_ino, by_path.st_dev) + assert os.stat(fd) == os.fstat(fd) + + # `True` is an `int`, so it names descriptor 1. Whether a bool used as a + # descriptor warns is a separate question; the value is what matters here. + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + assert os.stat(True).st_dev == os.fstat(1).st_dev + + # Neither other argument has anything to apply to. + try: + os.stat(fd, dir_fd=fd) + except ValueError as exc: + assert str(exc) == "stat: can't specify dir_fd without matching path", str(exc) + else: + raise AssertionError("stat accepted dir_fd with a descriptor") + + try: + os.stat(fd, follow_symlinks=False) + except ValueError as exc: + assert str(exc) == "stat: cannot use fd and follow_symlinks together", str(exc) + else: + raise AssertionError("stat accepted follow_symlinks with a descriptor") + + # lstat takes no descriptor, and says so with its own name and its own + # allowed-type list. + try: + os.lstat(fd) + except TypeError as exc: + assert str(exc) == "lstat: path should be string, bytes or os.PathLike, not int", str(exc) + else: + raise AssertionError("lstat accepted a descriptor") + + # The widened list appears only where the descriptor is allowed. + try: + os.stat(1.5) + except TypeError as exc: + expected = "stat: path should be string, bytes, os.PathLike or integer, not float" + assert str(exc) == expected, str(exc) + else: + raise AssertionError("stat accepted a float") + + try: + os.lstat(1.5) + except TypeError as exc: + expected = "lstat: path should be string, bytes or os.PathLike, not float" + assert str(exc) == expected, str(exc) + else: + raise AssertionError("lstat accepted a float") + + # A descriptor no call can serve reports the descriptor, not the type: -1 + # is an OSError either way. Which errno it carries is a property of the + # libc path taken and differs between the two entry points even upstream + # (EFAULT from `stat`, EBADF from `fstat`), so only the class is pinned. + try: + os.stat(-1) + except OSError: + pass + else: + raise AssertionError("os.stat(-1) did not fail") +finally: + os.close(fd) + +# `dir_fd` is a separate capability, reported honestly: a platform that does +# not honour it says so, and a platform that does resolves a relative name +# against the descriptor. `os.py:120-121` reads HAVE_FSTATAT for `stat` and +# HAVE_LSTAT for `lstat`, so the two are advertised independently. +if os.stat in os.supports_dir_fd: + assert os.lstat in os.supports_dir_fd, "lstat takes dir_fd wherever stat does" + link = os.path.join(tmp, "link") + os.symlink("f", link) + dfd = os.open(tmp, os.O_RDONLY) + try: + # `fstatat` resolves the name against the descriptor, and reaches the + # same file the path form does. + by_dir_fd = os.stat("f", dir_fd=dfd) + assert by_dir_fd.st_size == 10, by_dir_fd.st_size + assert (by_dir_fd.st_ino, by_dir_fd.st_dev) == (by_path.st_ino, by_path.st_dev) + + # An absolute name ignores the descriptor entirely. + assert os.stat(path, dir_fd=dfd).st_ino == by_path.st_ino + + # AT_SYMLINK_NOFOLLOW is what carries follow_symlinks=False, so the + # two spellings of "do not follow" agree. + nofollow = os.stat("link", dir_fd=dfd, follow_symlinks=False) + assert nofollow.st_ino == os.lstat("link", dir_fd=dfd).st_ino + assert nofollow.st_ino == os.lstat(link).st_ino + assert nofollow.st_ino != by_path.st_ino, "lstat followed the symlink" + assert os.stat("link", dir_fd=dfd).st_ino == by_path.st_ino + + # A missing name reports the name, not the descriptor. + try: + os.stat("absent", dir_fd=dfd) + except FileNotFoundError as exc: + assert exc.filename == "absent", exc.filename + else: + raise AssertionError("stat found a name that does not exist") + + # A descriptor that is not a directory cannot resolve a relative name. + plain = os.open(path, os.O_RDONLY) + try: + os.stat("f", dir_fd=plain) + except NotADirectoryError: + pass + else: + raise AssertionError("stat resolved a name against a plain file") + finally: + os.close(plain) + + # `_unwrap_dirfd` types the argument before it reaches the syscall. + try: + os.stat("f", dir_fd=1.5) + except TypeError as exc: + assert str(exc) == "argument should be integer or None, not float", str(exc) + else: + raise AssertionError("stat accepted a float dir_fd") + + # A descriptor no call can serve is an OSError. Which errno it carries + # depends on where the rejection happens — the sentinel check or the + # syscall — so only the class is pinned. + try: + os.stat("f", dir_fd=-1) + except OSError: + pass + else: + raise AssertionError("stat accepted dir_fd=-1") + finally: + os.close(dfd) + +print("OK") diff --git a/pyre/pyre-interpreter/src/gateway.rs b/pyre/pyre-interpreter/src/gateway.rs index 4a3782f726a..706074cbd5d 100644 --- a/pyre/pyre-interpreter/src/gateway.rs +++ b/pyre/pyre-interpreter/src/gateway.rs @@ -1575,10 +1575,14 @@ pub fn fsdecode_os_str(name: &std::ffi::OsStr) -> pyre_object::PyObjectRef { } } -/// `interp_posix.py:194-219 Path`: the syscall spelling and the resolved path +/// `interp_posix.py:140-152 Path`: the syscall spelling and the resolved path /// object travel together. For `os.PathLike`, `w_path` is the result of the /// single `__fspath__` call, not the wrapper that supplied it. pub struct FsEncodedPath { + /// `Path.as_fd`, `-1` where the argument named a path rather than an open + /// descriptor. Only the entry points that pass `allow_fd` can set it, so a + /// caller that took a path-only boundary never has to test it. + pub as_fd: i32, pub as_bytes: Vec, w_path_slot: usize, _roots: pyre_object::gc_roots::RootScope, @@ -1595,11 +1599,51 @@ impl FsEncodedPath { } pub fn fsencode_path_w(obj: pyre_object::PyObjectRef) -> Result { + path_or_fd_w(obj, None, false) +} + +/// [`fsencode_path_w`] for a boundary that also takes an open file descriptor — +/// `interp_posix.py:611 path=path_or_fd(allow_fd=True)`. `funcname` names the +/// caller in the type error, whose allowed-type list widens with `allow_fd`: +/// `stat` answers "string, bytes, os.PathLike or integer" where `lstat`, which +/// takes no descriptor, answers "string, bytes or os.PathLike". +pub fn fsencode_path_or_fd_w( + obj: pyre_object::PyObjectRef, + funcname: &str, + allow_fd: bool, +) -> Result { + path_or_fd_w(obj, Some(funcname), allow_fd) +} + +fn path_or_fd_w( + obj: pyre_object::PyObjectRef, + funcname: Option<&str>, + allow_fd: bool, +) -> Result { + // interp_posix.py:170-180 builds this list from the same two flags, and the + // caller-named form is the only one CPython ever shows for these entry + // points; the unnamed form is what every path-only boundary already emits. + let reject = |obj: pyre_object::PyObjectRef| -> crate::PyError { + let tp = crate::type_methods::arg_type_name(obj); + match funcname { + Some(name) => { + let allowed = if allow_fd { + "string, bytes, os.PathLike or integer" + } else { + "string, bytes or os.PathLike" + }; + crate::PyError::type_error(format!("{name}: path should be {allowed}, not {tp}")) + } + None => crate::PyError::type_error(format!( + "expected str, bytes or os.PathLike object, not {tp}" + )), + } + }; let roots = pyre_object::gc_roots::push_roots(); let obj_slot = pyre_object::gc_roots::shadow_stack_len(); pyre_object::gc_roots::pin_root(obj); - let (data, w_path_slot) = unsafe { + let (data, w_path_slot, as_fd) = unsafe { let obj = pyre_object::gc_roots::shadow_stack_get(obj_slot); if pyre_object::bytesobject::is_bytes_like(obj) { // baseobjspace.py:1975-1977: pyre's readable-buffer set here is @@ -1614,19 +1658,33 @@ pub fn fsencode_path_w(obj: pyre_object::PyObjectRef) -> Result Result Result pyre_object::PyObjectRef { + let ( + st_mode, + st_ino, + st_dev, + st_nlink, + st_uid, + st_gid, + st_size, + st_atime, + st_mtime, + st_ctime, + st_atime_ns, + st_mtime_ns, + st_ctime_ns, + ) = ( + f.mode, + f.ino, + f.dev, + f.nlink, + f.uid, + f.gid, + f.size, + f.atime, + f.mtime, + f.ctime, + f.atime_ns, + f.mtime_ns, + f.ctime_ns, + ); + #[cfg(unix)] + let (st_blksize, st_blocks, st_rdev) = (f.blksize, f.blocks, f.rdev); // The 10 sequence slots are the integer fields (integer-seconds // times at 7..10, named `_integer_*`); the float times, `st_*_ns`, // and the platform block/device extras are named-only fields. @@ -2395,8 +2484,10 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { /// `os.stat(path, *, dir_fd=None, follow_symlinks=True)` / /// `os.lstat(path, *, dir_fd=None)` — `follow_symlinks` is keyword-only, /// so `stat` cannot take the fixed-arity carrier that rejects keywords. - /// `dir_fd` stays unimplemented (the `*at` family is absent from - /// `_have_functions`), so only `None` is accepted. + /// The three argument forms are the three arms of `do_stat` + /// (`interp_posix.py:633-649`): an open descriptor as `path` goes to + /// `fstat`, a `dir_fd`-relative name to `fstatat`, and a bare name to + /// `stat`/`lstat`. fn stat_entry( args: &[pyre_object::PyObjectRef], default_follow: bool, @@ -2423,38 +2514,123 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ))); } }; - if let Some(dir_fd) = crate::builtins::kwarg_get(kwargs, "dir_fd") - && !unsafe { pyre_object::is_none(dir_fd) } + // `stat`/`lstat` type `dir_fd` as `DirFD(rposix.HAVE_FSTATAT)` + // (`interp_posix.py:612,660`), whose `unwrap` is `_unwrap_dirfd` + // (:274-278). + let dir_fd = match crate::builtins::kwarg_get(kwargs, "dir_fd") + .filter(|&v| !unsafe { pyre_object::is_none(v) }) { - return Err(crate::PyError::not_implemented(format!( - "{name}: dir_fd unavailable on this platform" - ))); - } + Some(v) => Some(unwrap_fd(v, "integer or None")?), + None => None, + }; let follow_symlinks = match crate::builtins::kwarg_get(kwargs, "follow_symlinks") { Some(v) => crate::baseobjspace::is_true(v)?, None => default_follow, }; - stat_impl(&[path], follow_symlinks) + // interp_posix.py:611,659 — `stat` takes `path_or_fd(allow_fd=True)` + // and `lstat` takes `allow_fd=False`, which is also what makes their + // type errors name different allowed types. + let path = crate::gateway::fsencode_path_or_fd_w(path, name, default_follow)?; + // interp_posix.py:634-644 `do_stat` tests the descriptor first: with one + // in hand neither other argument has anything to apply to, and both + // rejections precede the platform's dir_fd availability. + if path.as_fd != -1 { + if dir_fd.is_some() { + return Err(crate::PyError::value_error(format!( + "{name}: can't specify dir_fd without matching path" + ))); + } + if !follow_symlinks { + return Err(crate::PyError::value_error(format!( + "{name}: cannot use fd and follow_symlinks together" + ))); + } + return fstat_fd(path.as_fd); + } + match dir_fd { + Some(dir_fd) => stat_at(name, &path, dir_fd, follow_symlinks), + None => stat_path(&path, follow_symlinks), + } } - fn stat_impl( - args: &[pyre_object::PyObjectRef], + /// `rposix_stat.build_stat_result` reads the same fields off the raw + /// `struct stat` the `*at` calls fill in. + #[cfg(all(unix, not(feature = "sandbox")))] + fn stat_fields_from_libc(st: &libc::stat) -> StatFields { + StatFields { + mode: st.st_mode as i64, + ino: st.st_ino as i64, + dev: st.st_dev as i64, + nlink: st.st_nlink as i64, + uid: st.st_uid as i64, + gid: st.st_gid as i64, + size: st.st_size as i64, + atime: st.st_atime as i64, + mtime: st.st_mtime as i64, + ctime: st.st_ctime as i64, + atime_ns: st.st_atime as i64 * 1_000_000_000 + st.st_atime_nsec as i64, + mtime_ns: st.st_mtime as i64 * 1_000_000_000 + st.st_mtime_nsec as i64, + ctime_ns: st.st_ctime as i64 * 1_000_000_000 + st.st_ctime_nsec as i64, + blksize: st.st_blksize as i64, + blocks: st.st_blocks as i64, + rdev: st.st_rdev as i64, + } + } + + /// `do_stat` (`interp_posix.py:649`) resolves a name against an open + /// directory descriptor with `fstatat`, where `AT_SYMLINK_NOFOLLOW` + /// carries `follow_symlinks=False`. An absolute name ignores `dir_fd`, + /// which is why the caller does not have to test for one. + fn stat_at( + name: &str, + path: &crate::gateway::FsEncodedPath, + dir_fd: i32, follow_symlinks: bool, ) -> Result { - if args.is_empty() { - return Err(crate::PyError::type_error("stat() missing argument")); - } - // Only a wrong *type* is re-reported under `stat`'s own wording; the - // embedded-null ValueError and the surrogate UnicodeEncodeError say - // what actually went wrong and have to reach the caller as themselves. - // `os.stat('\ud800')` is a `UnicodeEncodeError`, not a `TypeError`. - let path = crate::gateway::fsencode_path_w(args[0]).map_err(|err| { - if matches!(err.kind, crate::error::PyErrorKind::TypeError) { - crate::PyError::type_error("stat: path should be string, bytes, os.PathLike") + #[cfg(all(unix, not(feature = "sandbox")))] + { + let c_path = std::ffi::CString::new(path.as_bytes.as_slice()) + .map_err(|_| crate::PyError::value_error("embedded null character"))?; + let mut st = std::mem::MaybeUninit::::uninit(); + let flags = if follow_symlinks { + 0 } else { - err + libc::AT_SYMLINK_NOFOLLOW + }; + let ret = unsafe { libc::fstatat(dir_fd, c_path.as_ptr(), st.as_mut_ptr(), flags) }; + if ret != 0 { + let err = std::io::Error::last_os_error(); + return Err(errno_err_with_filename( + crate::builtins::io_error_posix_errno(&err, libc::EBADF), + path.w_path(), + )); } - })?; + let st = unsafe { st.assume_init() }; + #[cfg(target_os = "macos")] + let st_flags = st.st_flags; + #[cfg(not(target_os = "macos"))] + let st_flags = 0u32; + return Ok(stat_result_from_fields( + &stat_fields_from_libc(&st), + st_flags, + )); + } + // `DirFD(available=False)` (`interp_posix.py:285-292`): the platform + // has no `fstatat`, so a `dir_fd` that reached this far has nothing + // to resolve against. + #[allow(unreachable_code)] + { + let _ = (path, dir_fd, follow_symlinks); + Err(crate::PyError::not_implemented(format!( + "{name}: dir_fd unavailable on this platform" + ))) + } + } + + fn stat_path( + path: &crate::gateway::FsEncodedPath, + follow_symlinks: bool, + ) -> Result { #[cfg(feature = "sandbox")] { let buf = if follow_symlinks { @@ -2855,6 +3031,51 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "lstat", crate::make_builtin_function("lstat", |args| stat_entry(args, false)), ); + /// `rposix_stat.py fstat`: the descriptor form both `os.fstat` and + /// `os.stat` with a descriptor answer through, so the two cannot drift. + fn fstat_fd(fd: i32) -> Result { + // `rposix_stat.py:fstat` passes the descriptor to libc, where + // `-1` reports EBADF. Rust's `OwnedFd::from_raw_fd(-1)` + // asserts before `File::metadata` can produce that error. + if fd == -1 { + return Err(crate::PyError::os_error_with_errno( + libc::EBADF, + std::io::Error::from_raw_os_error(libc::EBADF).to_string(), + )); + } + #[cfg(feature = "sandbox")] + { + let buf = crate::host_seam::ops::fstat(fd) + .map_err(|e| crate::host_seam::seam_os_err(e, ""))?; + Ok(make_stat_result_from_statbuf(&buf)) + } + #[cfg(all(unix, not(feature = "sandbox")))] + { + use std::os::unix::io::FromRawFd; + let f = unsafe { std::fs::File::from_raw_fd(fd) }; + let meta = f.metadata(); + let _ = std::mem::ManuallyDrop::new(f); // don't close + match meta { + Ok(m) => { + #[cfg(target_os = "macos")] + let st_flags = macos_fd_st_flags(fd); + #[cfg(not(target_os = "macos"))] + let st_flags = 0u32; + Ok(make_stat_result(&m, st_flags)) + } + Err(e) => Err(crate::PyError::os_error_with_errno( + crate::builtins::io_error_posix_errno(&e, 9), + format!("{}", e), + )), + } + } + #[cfg(not(any(unix, feature = "sandbox")))] + Err(crate::PyError::os_error_with_errno( + 9, + "fstat unsupported".to_string(), + )) + } + crate::module_ns_store( ns, "fstat", @@ -2864,47 +3085,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { if args.is_empty() { return Err(crate::PyError::type_error("fstat() missing argument")); } - let fd = crate::baseobjspace::c_int_w(args[0])?; - // `rposix_stat.py:fstat` passes the descriptor to libc, where - // `-1` reports EBADF. Rust's `OwnedFd::from_raw_fd(-1)` - // asserts before `File::metadata` can produce that error. - if fd == -1 { - return Err(crate::PyError::os_error_with_errno( - libc::EBADF, - std::io::Error::from_raw_os_error(libc::EBADF).to_string(), - )); - } - #[cfg(feature = "sandbox")] - { - let buf = crate::host_seam::ops::fstat(fd) - .map_err(|e| crate::host_seam::seam_os_err(e, ""))?; - Ok(make_stat_result_from_statbuf(&buf)) - } - #[cfg(all(unix, not(feature = "sandbox")))] - { - use std::os::unix::io::FromRawFd; - let f = unsafe { std::fs::File::from_raw_fd(fd) }; - let meta = f.metadata(); - let _ = std::mem::ManuallyDrop::new(f); // don't close - match meta { - Ok(m) => { - #[cfg(target_os = "macos")] - let st_flags = macos_fd_st_flags(fd); - #[cfg(not(target_os = "macos"))] - let st_flags = 0u32; - Ok(make_stat_result(&m, st_flags)) - } - Err(e) => Err(crate::PyError::os_error_with_errno( - crate::builtins::io_error_posix_errno(&e, 9), - format!("{}", e), - )), - } - } - #[cfg(not(any(unix, feature = "sandbox")))] - Err(crate::PyError::os_error_with_errno( - 9, - "fstat unsupported".to_string(), - )) + fstat_fd(crate::baseobjspace::c_int_w(args[0])?) }, 1, ), From 0ed154d93351441524979d16cd4611bae1c6ee24 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 6 Aug 2026 04:56:49 +0900 Subject: [PATCH 3/5] posix: unwrap stat's arguments in signature order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gateway.py:705` applies the unwrap specs in the order the signature declares them, so `stat(path, *, dir_fd, follow_symlinks)` (`interp_posix.py:610-614`) resolves `path` first. `stat_entry` unwrapped `dir_fd` before converting `path`, which is observable both in which error answers when more than one argument is bad and in the order the arguments' user code runs — `__fspath__` for `path`, `__index__` for `dir_fd`, `__bool__` for `follow_symlinks`. Measured on 3.14: `os.stat(1.5, dir_fd=1.5)` reports the path type error, and `os.stat(PathLike, dir_fd=1.5)` calls `__fspath__` before rejecting `dir_fd`, with `follow_symlinks.__bool__` never read in either case. The parity test observes the call order, not only the message, so the arrangement cannot regress silently. The descriptor-plus-dir_fd message keeps the 3.14 wording and now cites the `interp_posix.py:639` spelling it differs from. Assisted-by: Claude --- .../parity_tests/os_stat_file_descriptor.py | 33 +++++++++++++++++++ .../src/module/posix/interp_posix.rs | 17 +++++++--- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py b/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py index 4423acf50a6..2947df8f7de 100644 --- a/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py +++ b/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py @@ -80,6 +80,39 @@ else: raise AssertionError("lstat accepted a float") + # The arguments are unwrapped in signature order, so with more than one of + # them bad it is the leftmost that answers. Each can also run user code — + # `__fspath__`, `__index__`, `__bool__` — so the order is observable even + # when nothing raises. + try: + os.stat(1.5, dir_fd=1.5) + except TypeError as exc: + expected = "stat: path should be string, bytes, os.PathLike or integer, not float" + assert str(exc) == expected, str(exc) + else: + raise AssertionError("stat accepted a float path") + + order = [] + + class Spy: + def __fspath__(self): + order.append("path") + return tmp + + class Truthy: + def __bool__(self): + order.append("follow_symlinks") + return True + + try: + os.stat(Spy(), dir_fd=1.5, follow_symlinks=Truthy()) + except TypeError as exc: + assert str(exc) == "argument should be integer or None, not float", str(exc) + else: + raise AssertionError("stat accepted a float dir_fd") + # `path` was resolved, `dir_fd` then rejected, `follow_symlinks` never read. + assert order == ["path"], order + # A descriptor no call can serve reports the descriptor, not the type: -1 # is an OSError either way. Which errno it carries is a property of the # libc path taken and differs between the two entry points even upstream diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index a2a475378b7..f6bbe19e1b6 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -2514,6 +2514,16 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ))); } }; + // The three arguments are unwrapped in signature order — `path`, + // `dir_fd`, `follow_symlinks` (`interp_posix.py:610-614`) — because + // `gateway.py:705` applies the unwrap specs in that order and each can + // both raise and run user code: `__fspath__` for `path`, `__index__` + // for `dir_fd`, `__bool__` for `follow_symlinks`. + // + // interp_posix.py:611,659 — `stat` takes `path_or_fd(allow_fd=True)` + // and `lstat` takes `allow_fd=False`, which is also what makes their + // type errors name different allowed types. + let path = crate::gateway::fsencode_path_or_fd_w(path, name, default_follow)?; // `stat`/`lstat` type `dir_fd` as `DirFD(rposix.HAVE_FSTATAT)` // (`interp_posix.py:612,660`), whose `unwrap` is `_unwrap_dirfd` // (:274-278). @@ -2527,15 +2537,14 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { Some(v) => crate::baseobjspace::is_true(v)?, None => default_follow, }; - // interp_posix.py:611,659 — `stat` takes `path_or_fd(allow_fd=True)` - // and `lstat` takes `allow_fd=False`, which is also what makes their - // type errors name different allowed types. - let path = crate::gateway::fsencode_path_or_fd_w(path, name, default_follow)?; // interp_posix.py:634-644 `do_stat` tests the descriptor first: with one // in hand neither other argument has anything to apply to, and both // rejections precede the platform's dir_fd availability. if path.as_fd != -1 { if dir_fd.is_some() { + // 3.14 words this "can't specify dir_fd without matching + // path"; `interp_posix.py:639` says "can't specify both + // dir_fd and fd". The parity suite's oracle is CPython. return Err(crate::PyError::value_error(format!( "{name}: can't specify dir_fd without matching path" ))); From 72e4cf0a0fc6182ac71035864ebdd568a38f57b3 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 6 Aug 2026 05:32:14 +0900 Subject: [PATCH 4/5] posix: reject an unavailable dir_fd while unwrapping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DirFD(available=False)` is `_DirFD_Unavailable` (`interp_posix.py:285-292`), whose `unwrap` turns a non-default `dir_fd` away before the call body runs. The availability test sat in `stat_at` instead, so on a target without `fstatat` `stat(fd, dir_fd=...)` reached `do_stat`'s descriptor conflict and answered ValueError where the argument itself is what is unsupported. `HAVE_FSTATAT` names the condition `_have_functions` already advertises on, and the message moves into `dir_fd_unavailable` so the unwrap and the (now unreachable, but still compiled) `stat_at` arm cannot drift apart. The descriptor probe's behaviour on an `__index__` that raises is recorded at the probe: `:202-207` swallows it with `except OperationError: pass` and falls through to `__fspath__`, while 3.14 propagates it. Measured — an object with a raising `__index__` and a working `__fspath__` reports that exception rather than being statted — and the parity test now pins both that and the `lstat` side, which takes no descriptor and so never probes `__index__` at all. Assisted-by: Claude --- .../parity_tests/os_stat_file_descriptor.py | 20 +++++++++++ pyre/pyre-interpreter/src/gateway.rs | 7 ++++ .../src/module/posix/interp_posix.rs | 33 +++++++++++++++---- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py b/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py index 2947df8f7de..8dccae4ebf6 100644 --- a/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py +++ b/pyre/extra_tests/parity_tests/os_stat_file_descriptor.py @@ -113,6 +113,26 @@ def __bool__(self): # `path` was resolved, `dir_fd` then rejected, `follow_symlinks` never read. assert order == ["path"], order + # The descriptor probe is `__index__`, and an object carrying both it and + # `__fspath__` is taken as a descriptor — so an `__index__` that raises + # reports its own exception instead of falling through to the path. + class BadIndex: + def __index__(self): + raise RuntimeError("boom") + + def __fspath__(self): + return path + + try: + os.stat(BadIndex()) + except RuntimeError as exc: + assert str(exc) == "boom", str(exc) + else: + raise AssertionError("stat fell through a raising __index__ to __fspath__") + + # lstat takes no descriptor, so it never probes __index__ at all. + assert os.lstat(BadIndex()).st_size == 10 + # A descriptor no call can serve reports the descriptor, not the type: -1 # is an OSError either way. Which errno it carries is a property of the # libc path taken and differs between the two entry points even upstream diff --git a/pyre/pyre-interpreter/src/gateway.rs b/pyre/pyre-interpreter/src/gateway.rs index 706074cbd5d..13c37d8eb61 100644 --- a/pyre/pyre-interpreter/src/gateway.rs +++ b/pyre/pyre-interpreter/src/gateway.rs @@ -1670,6 +1670,13 @@ fn path_or_fd_w( // `__index__` and sits BEFORE the PathLike case, so an object // carrying both is taken as a descriptor and its `__fspath__` is // never called. + // + // Where `:202-207` wraps the probe in `except OperationError: + // pass` and falls through to `__fspath__`, 3.14 lets an + // `__index__` that raises propagate — measured, an object with a + // raising `__index__` and a working `__fspath__` reports that + // exception rather than being statted. The parity suite's oracle + // is CPython. let fd = crate::baseobjspace::c_int_w(obj)?; // interp_posix.py:269-271 `unwrap_fd` — `-1` is the sentinel for // "not a descriptor", so a caller naming it has to be turned away diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index f6bbe19e1b6..1f65b078bc5 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -2530,7 +2530,18 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { let dir_fd = match crate::builtins::kwarg_get(kwargs, "dir_fd") .filter(|&v| !unsafe { pyre_object::is_none(v) }) { - Some(v) => Some(unwrap_fd(v, "integer or None")?), + Some(v) => { + let fd = unwrap_fd(v, "integer or None")?; + // `DirFD(available=False)` is `_DirFD_Unavailable` + // (:285-292), which turns a non-default `dir_fd` away while + // unwrapping — so where the platform has no `fstatat` the + // answer is this, not the descriptor conflict `do_stat` + // would reach first. + if !HAVE_FSTATAT { + return Err(dir_fd_unavailable(name)); + } + Some(fd) + } None => None, }; let follow_symlinks = match crate::builtins::kwarg_get(kwargs, "follow_symlinks") { @@ -2586,6 +2597,16 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } } + /// `rposix.HAVE_FSTATAT` — what `DirFD` is parameterised on + /// (`interp_posix.py:612,660`), and what `_have_functions` advertises. + const HAVE_FSTATAT: bool = cfg!(all(unix, not(feature = "sandbox"))); + + fn dir_fd_unavailable(name: &str) -> crate::PyError { + crate::PyError::not_implemented(format!( + "{name}: dir_fd unavailable on this platform" + )) + } + /// `do_stat` (`interp_posix.py:649`) resolves a name against an open /// directory descriptor with `fstatat`, where `AT_SYMLINK_NOFOLLOW` /// carries `follow_symlinks=False`. An absolute name ignores `dir_fd`, @@ -2624,15 +2645,13 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { st_flags, )); } - // `DirFD(available=False)` (`interp_posix.py:285-292`): the platform - // has no `fstatat`, so a `dir_fd` that reached this far has nothing - // to resolve against. + // Unreachable in practice — `stat_entry` turns a `dir_fd` away at + // unwrap time wherever `HAVE_FSTATAT` is false — but the arm has to + // exist for those targets to compile. #[allow(unreachable_code)] { let _ = (path, dir_fd, follow_symlinks); - Err(crate::PyError::not_implemented(format!( - "{name}: dir_fd unavailable on this platform" - ))) + Err(dir_fd_unavailable(name)) } } From 218a5c4e7936a196a0eb92f09d50631e22c483e2 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 6 Aug 2026 06:59:17 +0900 Subject: [PATCH 5/5] launcher: carry -W, -X and PYTHONWARNINGS in the host's own spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `app_main.py:785-786` splits an `-X` value on the first `=` and stores both halves in `sys._xoptions` verbatim; `:892-906` appends the `-W` values and the PYTHONWARNINGS pieces to `sys.warnoptions` verbatim. None of them is an identifier, so none is required to have a UTF-8 spelling — `-W $'ignore\xff'` reaches `sys.warnoptions` as `'ignore\udcff'` on 3.14, where pyre exited 2 out of `lexopt`'s `.string()?`. `LaunchFlags.warnoptions` / `xoptions` and the `importing` statics behind them become `OsString`, and `sys` decodes them with `gateway::fsdecode_os_str` — the boundary the launcher's argv already goes through. `-X` still matches the options pyre acts on, through `to_str()`: every one of them is ASCII, so a value with no UTF-8 form cannot be one. `_xoptions` splits at the first `=` over the encoded bytes, which `OsStr` documents as sound at an ASCII byte, and puts both halves through WTF-8: `-X $'k\xff=v'` is `{'k\udcff': 'v'}`, so the key carries an escape as readily as the value. PYTHONWARNINGS moves from `read` to `read_raw`. `read` is `String::from_utf8(..).ok()`, so one undecodable byte was discarding the whole variable rather than the one comma-separated entry that carried it. `fsdecode_os_str_wtf8` is `fsdecode_os_str`'s buffer, pairing with it the way `fsdecode_filename_wtf8` pairs with `fsdecode_filename_bytes`, for the dict key that has to be hashed rather than handed out. The script path is a separate boundary and still narrows to `String`; its `__file__` cannot round-trip until `co_filename` is WTF-8. Assisted-by: Claude --- .../parity_tests/option_value_undecodable.py | 69 +++++++++++++++++++ pyre/pyre-interpreter/src/gateway.rs | 18 +++++ pyre/pyre-interpreter/src/importing.rs | 10 +-- pyre/pyre-interpreter/src/launch_env.rs | 38 +++++++--- pyre/pyre-interpreter/src/module/sys/vm.rs | 23 +++++-- pyre/pyrex/src/lib.rs | 18 +++-- 6 files changed, 152 insertions(+), 24 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/option_value_undecodable.py diff --git a/pyre/extra_tests/parity_tests/option_value_undecodable.py b/pyre/extra_tests/parity_tests/option_value_undecodable.py new file mode 100644 index 00000000000..b8ddfe80d68 --- /dev/null +++ b/pyre/extra_tests/parity_tests/option_value_undecodable.py @@ -0,0 +1,69 @@ +"""`-W`, `-X` and PYTHONWARNINGS carry a value with no UTF-8 spelling. + +These are free text, not identifiers: `app_main.py:785-786` splits an `-X` +value on the first `=` and puts both halves into `sys._xoptions` verbatim, and +`:892-906` appends the `-W` values and the PYTHONWARNINGS pieces to +`sys.warnoptions` verbatim. None of them is required to be spellable in UTF-8, +so a byte the filesystem encoding cannot spell arrives as the surrogate escape +that re-encodes to that byte — in the `_xoptions` key as much as in its value. + +An option value never reaches the filesystem, so like +`argv_undecodable_argument.py` this needs no such name on disk and passes the +value to a child instead. Windows takes a wide command line and has no byte +argv, so this shape does not exist there. +""" + +import os +import subprocess +import sys + +if sys.platform == "win32": + print("OK") + raise SystemExit + +ESC = os.fsdecode(b"\xff") +assert ESC == "\udcff", ascii(ESC) + + +def child(*args, env=None): + result = subprocess.run( + [sys.executable, *args], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + assert result.returncode == 0, (result.returncode, result.stderr) + return result.stdout.decode() + + +# -W keeps the value it was given. The warnings module rejects it as a filter +# later — it is not a valid action — but that is a separate stage, and the +# option list records what the command line said. +out = child("-W", "ignore" + ESC, "-c", "import sys; print(ascii(sys.warnoptions))") +assert "'ignore\\udcff'" in out, out + +# -X splits on the first `=`; the value half keeps the escape. +out = child("-X", "k=v" + ESC, "-c", "import sys; print(ascii(sys._xoptions))") +assert out.strip() == "{'k': 'v\\udcff'}", out + +# ... and so does the key half, which is a dict key, not a name. +out = child("-X", "k" + ESC + "=v", "-c", "import sys; print(ascii(sys._xoptions))") +assert out.strip() == "{'k\\udcff': 'v'}", out + +# A bare -X with no `=` is the key, and its value is True. +out = child("-X", "bare" + ESC, "-c", "import sys; print(ascii(sys._xoptions))") +assert out.strip() == "{'bare\\udcff': True}", out + +# Only the first `=` splits, so a value may carry more of them. +out = child("-X", "k=a=b" + ESC, "-c", "import sys; print(ascii(sys._xoptions))") +assert out.strip() == "{'k': 'a=b\\udcff'}", out + +# PYTHONWARNINGS is the same free text arriving through the environment, and it +# is comma-separated: one undecodable piece must not cost the whole variable. +env = dict(os.environ) +env["PYTHONWARNINGS"] = "ignore" + ESC + ",error" +out = child("-c", "import sys; print(ascii(sys.warnoptions))", env=env) +assert "'ignore\\udcff'" in out, out +assert "'error'" in out, out + +print("OK") diff --git a/pyre/pyre-interpreter/src/gateway.rs b/pyre/pyre-interpreter/src/gateway.rs index 13c37d8eb61..fc25edf6364 100644 --- a/pyre/pyre-interpreter/src/gateway.rs +++ b/pyre/pyre-interpreter/src/gateway.rs @@ -1575,6 +1575,24 @@ pub fn fsdecode_os_str(name: &std::ffi::OsStr) -> pyre_object::PyObjectRef { } } +/// [`fsdecode_os_str`]'s buffer, for a caller that needs the spelling as text +/// rather than as an object — a `dict` key it has to hash, say. Pairs with +/// [`fsdecode_os_str`] the way [`fsdecode_filename_wtf8`] pairs with +/// [`fsdecode_filename_bytes`], and for the same reason: a Rust `String` +/// cannot hold the lone surrogate an undecodable byte becomes. +pub fn fsdecode_os_str_wtf8(name: &std::ffi::OsStr) -> rustpython_wtf8::Wtf8Buf { + #[cfg(windows)] + { + use std::os::windows::ffi::OsStrExt; + let units: Vec = name.encode_wide().collect(); + rustpython_wtf8::Wtf8Buf::from_wide(&units) + } + #[cfg(not(windows))] + { + fsdecode_filename_wtf8(name.as_encoded_bytes()) + } +} + /// `interp_posix.py:140-152 Path`: the syscall spelling and the resolved path /// object travel together. For `os.PathLike`, `w_path` is the result of the /// single `__fspath__` call, not the wrapper that supplied it. diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 511f224103a..67f3cab07bd 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -1754,8 +1754,10 @@ static SYS_UNBUFFERED: AtomicBool = AtomicBool::new(false); // pypy/interpreter/app_main.py keeps the raw `-X` strings in // `options['_xoptions']` (a list) until sys initialization builds the public // dict. Preserve that owner/storage shape rather than introducing a map here. -static SYS_XOPTIONS: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); -static SYS_WARNOPTIONS: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); +static SYS_XOPTIONS: LazyLock>> = + LazyLock::new(|| Mutex::new(Vec::new())); +static SYS_WARNOPTIONS: LazyLock>> = + LazyLock::new(|| Mutex::new(Vec::new())); static SYS_ORIG_ARGV: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); static SYS_STDIO_ENCODING: LazyLock>> = LazyLock::new(|| Mutex::new(None)); @@ -1795,7 +1797,7 @@ pub fn set_runtime_flags(flags: &crate::launch_env::LaunchFlags) { } /// Raw `-X` values recorded by the launcher, in command-line order. -pub fn xoptions() -> Vec { +pub fn xoptions() -> Vec { SYS_XOPTIONS.lock().unwrap().clone() } @@ -1807,7 +1809,7 @@ pub fn unbuffered_flag() -> bool { SYS_UNBUFFERED.load(Ordering::Relaxed) } -pub fn warnoptions() -> Vec { +pub fn warnoptions() -> Vec { SYS_WARNOPTIONS.lock().unwrap().clone() } diff --git a/pyre/pyre-interpreter/src/launch_env.rs b/pyre/pyre-interpreter/src/launch_env.rs index d5cef37bb4f..ce593e99205 100644 --- a/pyre/pyre-interpreter/src/launch_env.rs +++ b/pyre/pyre-interpreter/src/launch_env.rs @@ -33,13 +33,17 @@ pub struct LaunchFlags { pub bytes_warning: i64, pub dont_write_bytecode: bool, pub unbuffered: bool, - pub warnoptions: Vec, + /// Both option lists stay in the host's own spelling until sys module + /// initialization decodes them, because neither is required to be text the + /// host can spell in UTF-8: `-W $'ignore\xff'` reaches `sys.warnoptions` as + /// `'ignore\udcff'`, and PYTHONWARNINGS carries the same bytes. + pub warnoptions: Vec, /// `app_main.py` passes the raw PYTHONIOENCODING value to initstdio after /// applying -E/-I. Keep it raw until stdio parses the optional errors part. pub stdio_encoding: Option, /// Every raw `-X` value stays in a list until sys module initialization /// turns it into `sys._xoptions`. - pub xoptions: Vec, + pub xoptions: Vec, } impl Default for LaunchFlags { @@ -126,6 +130,21 @@ fn read(name: &str) -> Option { read_raw(name).and_then(|value| String::from_utf8(value).ok()) } +/// Environment bytes in the host's own spelling. The seam hands back what the +/// platform stores: bytes on unix, where any byte is legal, and the UTF-8 form +/// of the wide value on Windows, where the host has already validated it. +fn os_string_from_bytes(value: &[u8]) -> std::ffi::OsString { + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + std::ffi::OsString::from_vec(value.to_vec()) + } + #[cfg(not(unix))] + { + std::ffi::OsString::from(String::from_utf8_lossy(value).into_owned()) + } +} + /// Presence of a variable, without decoding it. `_Py_GetEnv` tests the raw /// bytes, so a value that is not valid Unicode still counts as set. Both paths /// preserve that: the seam hands back bytes, and the installed table stores @@ -246,23 +265,26 @@ pub fn finalize(mut flags: LaunchFlags) -> Result { }; // pypy/interpreter/app_main.py:892-906 — lowest-precedence entries first; // the warnings module installs later entries ahead of earlier ones. - let mut warnoptions = Vec::new(); + let mut warnoptions: Vec = Vec::new(); if flags.dev_mode { - warnoptions.push("default".to_string()); + warnoptions.push("default".into()); } if !flags.ignore_environment { - if let Some(value) = read("PYTHONWARNINGS") { + // Read as bytes: a filter is free text, so `read`'s `env::var` + // contract would drop the whole variable for one undecodable byte + // rather than carry the entry `-W` would have carried. + if let Some(value) = read_raw("PYTHONWARNINGS") { if !value.is_empty() { - warnoptions.extend(value.split(',').map(str::to_string)); + warnoptions.extend(value.split(|&b| b == b',').map(os_string_from_bytes)); } } } warnoptions.append(&mut flags.warnoptions); if flags.bytes_warning > 0 { warnoptions.push(if flags.bytes_warning > 1 { - "error::BytesWarning".to_string() + "error::BytesWarning".into() } else { - "default::BytesWarning".to_string() + "default::BytesWarning".into() }); } flags.warnoptions = warnoptions; diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index b92545b9df6..4be0ee168ed 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -1031,12 +1031,25 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // for x in options['_xoptions']) let xoptions = w_dict_new(); for option in crate::importing::xoptions() { - let (name, value) = match option.split_once('=') { - Some((name, value)) => (name, w_str_new(value)), - None => (option.as_str(), w_bool_from(true)), + // `split('=', 1)` over a value that need not have a UTF-8 form. `=` is + // ASCII, and `OsStr` documents its encoded form as splittable at an + // ASCII byte, so the halves are whole `OsStr`s either side of it. + let bytes = option.as_encoded_bytes(); + let (name, value) = match bytes.iter().position(|&b| b == b'=') { + Some(eq) => { + let (name, value) = bytes.split_at(eq); + let value = unsafe { std::ffi::OsStr::from_encoded_bytes_unchecked(&value[1..]) }; + (name, crate::gateway::fsdecode_os_str(value)) + } + None => (bytes, w_bool_from(true)), }; + let name = unsafe { std::ffi::OsStr::from_encoded_bytes_unchecked(name) }; unsafe { - pyre_object::dictmultiobject::w_dict_setitem_str_no_proxy(xoptions, name, value); + pyre_object::dictmultiobject::w_dict_setitem_wtf8_no_proxy( + xoptions, + &crate::gateway::fsdecode_os_str_wtf8(name), + value, + ); } } module_ns_store(ns, "_xoptions", xoptions); @@ -1732,7 +1745,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { w_list_new( crate::importing::warnoptions() .iter() - .map(|option| w_str_new(option)) + .map(|option| crate::gateway::fsdecode_os_str(option)) .collect(), ), ); diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index 609ae4771a8..b4d94e6e29d 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -139,12 +139,16 @@ fn parse_args( flags.safe_path = true; } Short('X') => { - let option = parser.value()?.string()?; - match option.as_str() { - "dev" => flags.dev_mode = true, - "utf8" | "utf8=1" => flags.utf8_mode = Some(1), - "utf8=0" => flags.utf8_mode = Some(0), - _ if option.starts_with("utf8=") => { + // The value reaches `sys._xoptions` in the host's own + // spelling, so it is kept as an `OsString`; only the options + // pyre acts on are matched, and every one of those is ASCII, + // so a value with no UTF-8 form simply cannot be one of them. + let option: std::ffi::OsString = parser.value()?; + match option.to_str() { + Some("dev") => flags.dev_mode = true, + Some("utf8") | Some("utf8=1") => flags.utf8_mode = Some(1), + Some("utf8=0") => flags.utf8_mode = Some(0), + Some(value) if value.starts_with("utf8=") => { fatal_utf8_config_error("invalid -X utf8 option value") } _ => {} @@ -156,7 +160,7 @@ fn parse_args( // the option/value in launcher parsing lets stdlib subprocesses // reach their command or module. Short('W') => { - flags.warnoptions.push(parser.value()?.string()?); + flags.warnoptions.push(parser.value()?); } // `-O` / `-OO` raise the optimization level; each occurrence counts // (app_main.py `optimize`). PYTHONOPTIMIZE folds in during finalize.