diff --git a/.github/workflows/pyre-ci.yml b/.github/workflows/pyre-ci.yml index 4a30d97abd0..a418dedb363 100644 --- a/.github/workflows/pyre-ci.yml +++ b/.github/workflows/pyre-ci.yml @@ -439,6 +439,15 @@ jobs: # semantics fails here. check.py has just built the release pyre-dynasm # and pyre-cranelift binaries the runner drives, so this costs only the # per-script process spawns. + # + # Run it even when check.py above failed. The two gates answer different + # questions and check.py is the one that goes red on a jitstats drift + # nobody has re-recorded yet; while it does, every later step in this job + # is skipped and the parity suite reports nothing at all. Windows spent + # its whole history that way: thirteen real parity failures sat unseen + # until one run happened to get past check.py. The job still fails on + # either — this only stops one from hiding the other. + if: ${{ !cancelled() }} env: PYRE_CHECK_PYTHON3: ${{ steps.cpython.outputs.python-path }} run: ${{ steps.cpython.outputs.python-path }} pyre/extra_tests/parity_tests/run.py diff --git a/pyre/bench/synth/unary_negative.py b/pyre/bench/synth/unary_negative.py index a9040a51672..8d16db3179e 100644 --- a/pyre/bench/synth/unary_negative.py +++ b/pyre/bench/synth/unary_negative.py @@ -25,9 +25,9 @@ def main(): # UNARY_NEGATIVE on INT_MIN: -INT_MIN overflows the machine-int range, so # descr_neg (intobject.py:628) takes the long branch and returns 2**63 as a -# W_LongObject. generated_unary_int_value declines the int fast path at the -# concrete INT_MIN operand and traces the residual long-neg, so the compiled -# loop must agree with the long result rather than wrapping back to INT_MIN. +# W_LongObject. The walker fold pins the operand with GUARD_VALUE and takes +# the _make_ovf2long tail, so the compiled loop must agree with the long result +# rather than wrapping back to INT_MIN. def main_int_min(): m = -9223372036854775807 - 1 # INT_MIN as a machine int acc = 0 diff --git a/pyre/extra_tests/parity_tests/audit_and_parsercreate_name_argument.py b/pyre/extra_tests/parity_tests/audit_and_parsercreate_name_argument.py new file mode 100644 index 00000000000..8d80815d400 --- /dev/null +++ b/pyre/extra_tests/parity_tests/audit_and_parsercreate_name_argument.py @@ -0,0 +1,269 @@ +"""`sys.audit` and `pyexpat.ParserCreate` check the name they are handed. + +`sys.audit` was registered as `|_| Ok(w_none())` — a no-op that accepted +anything, so `sys.audit()` with no event at all, with an `int`, or by keyword +all returned None. 22 stdlib modules call it (`os.walk`, `glob.glob`, +`pickle.find_class`, `subprocess`, ...), so a bad event name reached none of the +checks that upstream's `@unwrap_spec(event="text")` performs. + +`ParserCreate` did check that `encoding` is `str` or `None`, but stored it +without asking whether it has a UTF-8 spelling — the sibling +`namespace_separator` arm did ask. A `str` holding a lone surrogate therefore +built a parser whose stored encoding no `&str` reader can see. The same arm +also spelled its type name as a literal `int`, so every other type was reported +as `int`. + +No audit hook can be installed yet (`sys.addaudithook` stores nothing), so +`sys.audit` still does nothing past the checks below; the hook mechanism is a +separate gap. The surrogate is built from bytes rather than written as a +literal because that is how it arrives in practice and because a source file +cannot carry one. + +Only one surrogate is used for the messages that are compared verbatim: a run +of adjacent unencodable code points is reported by CPython as one error with a +range (`characters in position 3-4`) and by pyre as the first one alone, which +is a separate divergence in the encoder rather than in these entry points. +""" + +import sys + +import pyexpat + +# b'\xff' has no UTF-8 spelling in any position, so surrogateescape maps it to +# U+DCFF — what a filesystem name or an argv element yields on a host that does +# not enforce UTF-8. +SURR = b"bad\xffname".decode("utf-8", "surrogateescape") +ENCODE_ERROR = ( + "'utf-8' codec can't encode character '\\udcff' in position 3: " + "surrogates not allowed" +) + +ERRORS = [] + + +def check(cond, what): + if not cond: + ERRORS.append(what) + + +def raises(what, exc, expected, fn): + """Assert fn() raises `exc` whose message is exactly `expected`.""" + try: + fn() + except exc as e: + check( + str(e) == expected, + f"{what}: got {str(e)!r}, expected {expected!r}", + ) + return + except BaseException as e: + ERRORS.append( + f"{what}: raised {type(e).__name__}({e!r}), expected {exc.__name__}" + ) + return + ERRORS.append(f"{what}: no exception, expected {exc.__name__}") + + +# The payload is the point of the test, so verify it before using it. +check(len(SURR) == 8, f"surrogate payload is {len(SURR)} code points, expected 8") +check(SURR.encode("utf-8", "surrogateescape") == b"bad\xffname", "payload lost its byte") + + +class Odd: + pass + + +# ── sys.audit: the event name is a str that must encode ─────────────────── +raises( + "audit with no event", + TypeError, + "audit expected at least 1 argument, got 0", + lambda: sys.audit(), +) +for value, spelling in ( + (123, "int"), + (1.5, "float"), + (None, "None"), + (b"x", "bytes"), + (bytearray(b"x"), "bytearray"), + ((), "tuple"), + (Odd(), "Odd"), +): + raises( + f"audit({spelling})", + TypeError, + f"audit() argument 1 must be str, not {spelling}", + lambda value=value: sys.audit(value), + ) +raises( + "audit with a surrogate event", + UnicodeEncodeError, + ENCODE_ERROR, + lambda: sys.audit(SURR), +) +# Every parameter is positional-only, so the event cannot be named. +raises( + "audit by keyword", + TypeError, + "sys.audit() takes no keyword arguments", + lambda: sys.audit(event="x"), +) +# ...and the accepting calls still accept, so the checks above are not passing +# because audit stopped working. +check(sys.audit("pyre.test") is None, "audit rejected a plain event name") +check(sys.audit("pyre.test", 1, 2) is None, "audit rejected trailing parameters") + + +class SubStr(str): + pass + + +check(sys.audit(SubStr("pyre.test")) is None, "audit rejected a str subclass") + + +# ── pyexpat.ParserCreate: both str-or-None parameters report their type ──── +for value, spelling in ((123, "int"), (1.5, "float"), (b"x", "bytes")): + raises( + f"ParserCreate(encoding={spelling})", + TypeError, + f"ParserCreate() argument 'encoding' must be str or None, not {spelling}", + lambda value=value: pyexpat.ParserCreate(value), + ) + raises( + f"ParserCreate(namespace_separator={spelling})", + TypeError, + f"ParserCreate() argument 'namespace_separator' must be str or None, " + f"not {spelling}", + lambda value=value: pyexpat.ParserCreate(None, value), + ) + +raises( + "ParserCreate with a surrogate encoding", + UnicodeEncodeError, + ENCODE_ERROR, + lambda: pyexpat.ParserCreate(SURR), +) +raises( + "ParserCreate with a surrogate separator", + UnicodeEncodeError, + ENCODE_ERROR, + lambda: pyexpat.ParserCreate(None, SURR), +) +# The encoding check runs before the separator's length check, so a too-long +# separator carrying a surrogate reports the encoding, not the length. +raises( + "ParserCreate with a long surrogate separator", + UnicodeEncodeError, + ENCODE_ERROR, + lambda: pyexpat.ParserCreate(None, SURR + "x"), +) +raises( + "ParserCreate with a two-character separator", + ValueError, + "namespace_separator must be at most one character, omitted, or None", + lambda: pyexpat.ParserCreate(None, "ab"), +) + +# The accepting calls, so the checks above are not passing because +# ParserCreate stopped building parsers. +check( + type(pyexpat.ParserCreate()).__name__ == "xmlparser", + "ParserCreate() no longer builds a parser", +) +check( + type(pyexpat.ParserCreate("utf-8")).__name__ == "xmlparser", + "ParserCreate('utf-8') no longer builds a parser", +) +check( + type(pyexpat.ParserCreate(encoding="utf-8")).__name__ == "xmlparser", + "ParserCreate(encoding=) no longer binds", +) +check( + type(pyexpat.ParserCreate(None, ":")).__name__ == "xmlparser", + "ParserCreate with a separator no longer builds a parser", +) +check( + type(pyexpat.ParserCreate(SubStr("utf-8"))).__name__ == "xmlparser", + "ParserCreate rejected a str subclass encoding", +) + + +# ── an omitted `intern` is not the same as an explicit None ─────────────── +# Omitting it asks for a fresh dictionary; passing None asks for no interning +# at all. A default of `None` cannot tell the two apart, and the parser then +# interns names the caller asked it not to. +def interned(parser): + """The number of names `parser` interned while parsing, or None.""" + parser.StartElementHandler = lambda name, attrs: None + parser.Parse("", True) + return None if parser.intern is None else len(parser.intern) + + +check( + isinstance(pyexpat.ParserCreate().intern, dict), + "an omitted intern did not produce a dictionary", +) +check(interned(pyexpat.ParserCreate()) == 2, "an omitted intern interned nothing") +check( + pyexpat.ParserCreate(None, None, None).intern is None, + "an explicit intern=None was replaced with a dictionary", +) +check( + interned(pyexpat.ParserCreate(None, None, None)) is None, + "an explicit intern=None still interned names", +) +# A dictionary of the caller's own is used as given. +own = {} +p = pyexpat.ParserCreate(None, None, own) +check(p.intern is own, "a supplied intern dictionary was not the one used") +check(interned(p) == 2 and len(own) == 2, "the supplied dictionary was not filled") + +# ── ctypes.CDLL: the library name reaches dlopen in filesystem units ────── +# Filed alongside the two above as a third disagreement, and already correct: +# `_ctypes.dlopen` takes the name through `fsencode`, so a surrogate escape +# folds back to the byte it stands for and dlopen is the thing that fails. +# Pinned because the earlier reading came from a binary built before that +# routing landed. A surrogate that is *not* an escape (U+D800 is outside +# U+DC80..U+DCFF) has no byte to fold to, so it is refused before the call — +# which is what the second row separates from the first. +try: + import ctypes +except ImportError: + ctypes = None +if ctypes is not None: + + def raises_class(what, exc, fn): + """Assert fn() raises `exc`; the message is the host's dlopen text.""" + try: + fn() + except exc: + return + except BaseException as e: + ERRORS.append( + f"{what}: raised {type(e).__name__}, expected {exc.__name__}" + ) + return + ERRORS.append(f"{what}: no exception, expected {exc.__name__}") + + raises_class( + "CDLL with a surrogate-escaped name", + OSError, + lambda: ctypes.CDLL(SURR), + ) + raises_class( + "CDLL with a non-escape surrogate", + UnicodeEncodeError, + lambda: ctypes.CDLL("bad\ud800name"), + ) + raises_class( + "CDLL with a missing library", + OSError, + lambda: ctypes.CDLL("nosuchlib_pyre_parity_test"), + ) + +if ERRORS: + for e in ERRORS: + sys.stderr.write(f"FAIL: {e}\n".encode("utf-8", "backslashreplace").decode()) + raise AssertionError(f"{len(ERRORS)} divergence(s)") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/os_access_modifiers.py b/pyre/extra_tests/parity_tests/os_access_modifiers.py new file mode 100644 index 00000000000..00e4d717c3c --- /dev/null +++ b/pyre/extra_tests/parity_tests/os_access_modifiers.py @@ -0,0 +1,236 @@ +# pyre-check: platforms=linux,darwin +"""`os.access` honours dir_fd, effective_ids and follow_symlinks. + +Named for these two platforms because `faccessat(2)` is what carries all three +modifiers, and Windows has none: the reference CPython refuses `dir_fd` there +with a NotImplementedError of its own and omits `access` from the same three +capability sets, so every assertion below would fail against a failing +reference. + + +All three are keyword-only modifiers that `access` has always *bound* — an +unknown keyword was an error, and a third positional was refused — while the +body called `access(2)`, which takes none of them. A `dir_fd` was therefore +accepted and then ignored, so a relative name resolved against the working +directory instead of the descriptor, and `follow_symlinks=False` answered about +the file a symlink points at rather than the link. Neither raised; both +answered confidently and wrongly. `os.supports_dir_fd`, +`os.supports_effective_ids` and `os.supports_follow_symlinks` all omitted +`access`, which is the honest report of that, so a caller reading the +capability set before calling was told the truth and a caller passing the +modifier was not. + +The modified call is one `faccessat(2)`; the plain call stays on `access(2)`. + +The discriminating assertion is the pair below: the *same* relative name +answers True with `dir_fd` and False without it. A test that only checked the +`dir_fd` form would have passed on the old build for any name that also +resolves from the working directory. + +`effective_ids` cannot be discriminated by an unprivileged process — the real +and effective ids are equal, so both answers agree — so it is asserted to be +accepted and consistent rather than to change anything. What pins it is +membership in `os.supports_effective_ids`, which is the same `HAVE_FACCESSAT` +bit the other two read. +""" + +import os +import sys +import tempfile + +ERRORS = [] + + +def check(cond, what): + if not cond: + ERRORS.append(what) + + +def raises(what, exc, expected, fn): + try: + fn() + except exc as e: + check(str(e) == expected, f"{what}: got {str(e)!r}, expected {expected!r}") + return + except BaseException as e: + ERRORS.append(f"{what}: raised {type(e).__name__}({e}), expected {exc.__name__}") + return + ERRORS.append(f"{what}: no exception, expected {exc.__name__}") + + +def check_class(what, exc, fn): + """Assert fn() raises exactly `exc`, without pinning the message.""" + try: + fn() + except BaseException as e: + if type(e) is not exc: + ERRORS.append( + f"{what}: raised {type(e).__name__}({e}), expected {exc.__name__}" + ) + return + ERRORS.append(f"{what}: no exception, expected {exc.__name__}") + + +# ── the capability sets say which modifiers work ────────────────────────── +check(os.access in os.supports_dir_fd, "os.access missing from supports_dir_fd") +check( + os.access in os.supports_effective_ids, + "os.access missing from supports_effective_ids", +) +check( + os.access in os.supports_follow_symlinks, + "os.access missing from supports_follow_symlinks", +) + +root = tempfile.mkdtemp() +plain = os.path.join(root, "plain") +with open(plain, "w"): + pass +sub = os.path.join(root, "sub") +os.mkdir(sub) +nested = os.path.join(sub, "g") +with open(nested, "w"): + pass +dangling = os.path.join(root, "dangling") +os.symlink(os.path.join(root, "no-such-target"), dangling) + +# The relative names below must not also resolve from the working directory, +# or the dir_fd assertions would pass without dir_fd doing anything. +check(not os.path.exists("sub/g"), "the test's relative name exists in the cwd") +check(not os.path.exists("dangling"), "the test's link name exists in the cwd") + +dir_fd = os.open(root, os.O_RDONLY) +try: + # ── dir_fd actually resolves the name ───────────────────────────────── + check( + os.access("sub/g", os.F_OK, dir_fd=dir_fd) is True, + "access(dir_fd=) did not find a name under the descriptor", + ) + check( + os.access("sub/g", os.F_OK) is False, + "the relative name resolved without dir_fd, so the pair proves nothing", + ) + check( + os.access("sub/no-such-file", os.F_OK, dir_fd=dir_fd) is False, + "access(dir_fd=) claimed a missing name exists", + ) + # An absolute name ignores the descriptor, so even a closed one is fine. + check( + os.access(plain, os.F_OK, dir_fd=dir_fd) is True, + "an absolute name stopped resolving when dir_fd was given", + ) + # `dir_fd=None` is the default spelled out, not a descriptor. + check( + os.access(plain, os.F_OK, dir_fd=None) is True, + "dir_fd=None was not treated as absent", + ) + + # ── follow_symlinks asks about the link itself ──────────────────────── + check( + os.access(dangling, os.F_OK) is False, + "a dangling link reported its own existence through the target", + ) + check( + os.access(dangling, os.F_OK, follow_symlinks=False) is True, + "access(follow_symlinks=False) did not see the link itself", + ) + # Both modifiers at once, so the flag word carries two bits. + check( + os.access("dangling", os.F_OK, dir_fd=dir_fd, follow_symlinks=False) is True, + "dir_fd and follow_symlinks=False together did not find the link", + ) + check( + os.access("dangling", os.F_OK, dir_fd=dir_fd) is False, + "dir_fd with the default follow_symlinks resolved a dangling link", + ) + + # ── effective_ids is accepted and consistent ────────────────────────── + check( + os.access(plain, os.R_OK, effective_ids=True) + == os.access(plain, os.R_OK), + "effective_ids changed the answer for a process whose ids are equal", + ) + check( + os.access("sub/g", os.F_OK, dir_fd=dir_fd, effective_ids=True) is True, + "effective_ids with dir_fd lost the descriptor", + ) + + # ── the mode is still the mode ──────────────────────────────────────── + # A modifier that reached the syscall as a mode bit, or a mode that got + # dropped on the way, would show up here. + check(os.access(plain, os.R_OK) is True, "a readable file reported unreadable") + check( + os.access(plain, os.X_OK) is False, + "a file with no execute bit reported executable", + ) + os.chmod(plain, 0o755) + check( + os.access(plain, os.X_OK) is True, + "the execute bit did not become visible after chmod", + ) + check( + os.access("sub/g", os.R_OK | os.W_OK, dir_fd=dir_fd) is True, + "a combined mode through dir_fd answered False", + ) + + # ── a bytes path still works ────────────────────────────────────────── + check( + os.access(os.fsencode(plain), os.F_OK) is True, + "a bytes path stopped resolving", + ) + + # ── the modifiers are still type-checked ────────────────────────────── + raises( + "dir_fd given a str", + TypeError, + "argument should be integer or None, not str", + lambda: os.access("sub/g", os.F_OK, dir_fd="x"), + ) + raises( + "a descriptor passed as the path", + TypeError, + "access: path should be string, bytes or os.PathLike, not int", + lambda: os.access(dir_fd, os.F_OK), + ) + + # ── the parameters convert in declaration order ─────────────────────── + # Every parameter can raise, so which one reports first is observable. + # `mode` is converted before any modifier is touched: giving both a bad + # mode and a bad modifier must report the mode. Only the exception + # *class* is asserted, because the two interpreters word the integer + # conversion differently for reasons that have nothing to do with access. + class RaisingBool: + def __bool__(self): + raise RuntimeError("__bool__ ran") + + check_class( + "a bad mode reports before a bad dir_fd", + OverflowError, + lambda: os.access("sub/g", 2**40, dir_fd="not a descriptor"), + ) + check_class( + "a bad mode reports before a flag's __bool__ runs", + OverflowError, + lambda: os.access("sub/g", 2**40, effective_ids=RaisingBool()), + ) + # ...and with an acceptable mode the flag *is* consulted, so the two + # assertions above are not passing because the flags stopped being read. + check_class( + "a flag's __bool__ runs once the mode converts", + RuntimeError, + lambda: os.access("sub/g", os.F_OK, effective_ids=RaisingBool()), + ) +finally: + os.close(dir_fd) + os.unlink(dangling) + os.unlink(nested) + os.rmdir(sub) + os.unlink(plain) + os.rmdir(root) + +if ERRORS: + for e in ERRORS: + print("FAIL:", e, file=sys.stderr) + raise AssertionError(f"{len(ERRORS)} divergence(s)") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/os_arg_binding_surface.py b/pyre/extra_tests/parity_tests/os_arg_binding_surface.py new file mode 100644 index 00000000000..34ff7d3ef03 --- /dev/null +++ b/pyre/extra_tests/parity_tests/os_arg_binding_surface.py @@ -0,0 +1,153 @@ +"""Nine posix entry points bind their arguments, and say so the same way. + +Each of these took its arguments straight off the raw slice: the trailing +`__pyre_kw__` marker dict was never split away, so a keyword either vanished or +arrived as a positional value, and a surplus positional was dropped instead of +refused. The loudest case was `symlink(src, dst, target_is_directory, dir_fd)`, +which created the link and returned None where CPython raises. + +Unlike the dup2 test, this one *does* assert message text. These entry points +bind through the posix module's own binder, which carries the clinic spellings, +so text parity is reachable here — and the two spellings are not +interchangeable: an entry point with a keyword-bindable parameter reports +`f() takes at most 1 argument (2 given)`, one that is entirely positional-only +reports `f expected at most 1 argument, got 2`. Asserting only the type would +let the two swap places unnoticed. +""" + +import os +import sys + +ERRORS = [] + + +def check(cond, what): + if not cond: + ERRORS.append(what) + + +def raises(what, expected, fn): + """Assert fn() raises TypeError whose message is exactly `expected`.""" + try: + fn() + except TypeError as e: + check(str(e) == expected, f"{what}: got {str(e)!r}, expected {expected!r}") + return + except Exception as e: + ERRORS.append(f"{what}: raised {type(e).__name__}({e}), expected TypeError") + return + ERRORS.append(f"{what}: no exception, expected TypeError") + + +d = os.getcwd() + +# ── the keyword-bindable family: `f() takes at most N argument(s) (M given)` ── +raises( + "listdir surplus", + "listdir() takes at most 1 argument (2 given)", + lambda: os.listdir(d, 1), +) +raises( + "listdir unknown keyword", + "listdir() got an unexpected keyword argument 'zzz'", + lambda: os.listdir(zzz=1), +) +check(isinstance(os.listdir(path=d), list), "listdir(path=) did not bind") + +raises( + "scandir surplus", + "scandir() takes at most 1 argument (2 given)", + lambda: os.scandir(d, 1), +) +with os.scandir(path=d) as it: + check(any(True for _ in it), "scandir(path=) did not bind") + +# ── a keyword-only tail makes the count positional-only ────────────────── +raises( + "access third positional", + "access() takes exactly 2 positional arguments (3 given)", + lambda: os.access(d, os.F_OK, True), +) +raises( + "access unknown keyword", + "access() got an unexpected keyword argument 'zzz'", + lambda: os.access(d, os.F_OK, zzz=1), +) +check(os.access(path=d, mode=os.F_OK) is True, "access(path=, mode=) did not bind") + +raises( + "readlink second positional", + "readlink() takes exactly 1 positional argument (2 given)", + lambda: os.readlink(d, 1), +) + +raises( + "symlink fourth positional", + "symlink() takes at most 3 positional arguments (4 given)", + lambda: os.symlink("a", "b", False, 1), +) + +raises( + "sendfile missing count", + "sendfile() missing required argument 'count' (pos 4)", + lambda: os.sendfile(1, 2, 3), +) + +# ── positional-only: no `()`, no parenthesised count ───────────────────── +raises( + "get_terminal_size surplus", + "get_terminal_size expected at most 1 argument, got 2", + lambda: os.get_terminal_size(1, 2), +) +raises( + "get_terminal_size by keyword", + "posix.get_terminal_size() takes no keyword arguments", + lambda: os.get_terminal_size(fd=1), +) + +# ── positional-only prefix + keyword-only tail ─────────────────────────── +for name in ("posix_spawn", "posix_spawnp"): + spawn = getattr(os, name, None) + if spawn is None: + continue + raises( + f"{name} too few", + f"{name}() takes exactly 3 positional arguments (2 given)", + lambda spawn=spawn: spawn("/bin/true", ["x"]), + ) + raises( + f"{name} names are positional-only", + f"{name}() takes exactly 3 positional arguments (0 given)", + lambda spawn=spawn: spawn(path="/bin/true", argv=["x"], env={}), + ) + raises( + f"{name} unknown keyword", + f"{name}() got an unexpected keyword argument 'zzz'", + lambda spawn=spawn: spawn("/bin/true", ["true"], {}, zzz=1), + ) + +# ── symlink actually refuses, rather than creating the link ────────────── +# The surplus-positional assertion above passes just as well if the call +# raises *after* doing the work, so the effect is checked separately. +target = os.path.join(os.environ.get("TMPDIR", "/tmp"), f"pyre_sym_{os.getpid()}") +try: + os.unlink(target) +except OSError: + pass +try: + os.symlink("nowhere", target, False, 1) +except TypeError: + pass +check(not os.path.islink(target), "symlink raised but still created the link") +# ...and that the by-name form does create it, so the check above is not +# passing because symlink stopped working. +os.symlink("nowhere", dst=target) +check(os.path.islink(target), "symlink(dst=) did not create the link") +os.unlink(target) + +if ERRORS: + for e in ERRORS: + print("FAIL:", e, file=sys.stderr) + raise AssertionError(f"{len(ERRORS)} binding divergence(s)") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/os_cpu_count.py b/pyre/extra_tests/parity_tests/os_cpu_count.py new file mode 100644 index 00000000000..d08a3102223 --- /dev/null +++ b/pyre/extra_tests/parity_tests/os_cpu_count.py @@ -0,0 +1,89 @@ +"""`os.cpu_count()` reports the host's processors, not the interpreter's threads. + +The host's processor count is not knowable from inside the test, so what is +asserted is the shape it must have on every host and, above all, the property +that separates a processor count from a thread count: it does not move when +threads come and go. +""" + +import os +import posix +import threading + + +def check(cond, what): + if not cond: + raise AssertionError(what) + + +n = os.cpu_count() +check(n is None or isinstance(n, int), f"cpu_count answered a {type(n).__name__}") +# A bool is an int and would pass the line above while meaning nothing. +check(not isinstance(n, bool), "cpu_count answered a bool") +if n is not None: + check(n > 0, f"cpu_count answered {n}") + +# The private alias is reached through `posix`: `os` star-imports, which skips +# every underscore name, so `os._cpu_count` does not exist on either side. +check(not hasattr(os, "_cpu_count"), "os grew a _cpu_count") +if hasattr(posix, "_cpu_count"): + check(posix._cpu_count() == n, f"_cpu_count {posix._cpu_count()} disagrees with cpu_count {n}") + +# ── the property ───────────────────────────────────────────────────────── +# A count wired to the process's own thread table — /proc/self/stat's +# num_threads, or the mach task_threads count — rises here and falls again. +# A processor count is the same number all three times. +started = threading.Semaphore(0) +release = threading.Event() + + +def hold(): + started.release() + release.wait() + + +before = os.cpu_count() +threads = [threading.Thread(target=hold) for _ in range(6)] +for t in threads: + t.start() +try: + for _ in threads: + started.acquire() + # All six are alive and parked at this point. + during = os.cpu_count() +finally: + release.set() + for t in threads: + t.join() +after = os.cpu_count() + +check( + before == during == after, + f"cpu_count moved with the live thread count: {before} -> {during} -> {after}", +) + +# ── the value, not just its stability ──────────────────────────────────── +# A constant wrong answer would satisfy everything above. The processor count +# the host reports through sysconf is the one both sides are built on — the +# `sysconf(_SC_NPROCESSORS_ONLN)` arm directly, the `sysctl(CTL_HW, HW_NCPU)` +# arm because a host reports the same processors either way. +if before is not None and hasattr(os, "sysconf"): + try: + onln = os.sysconf("SC_NPROCESSORS_ONLN") + except (ValueError, OSError): + onln = None + if onln is not None and onln > 0: + check(before == onln, f"cpu_count {before} is not the host's {onln} processors") + +# ── consistency with the neighbouring counts ───────────────────────────── +# The affinity mask is a subset of the processors, so it can never be wider. +if hasattr(os, "sched_getaffinity") and before is not None: + mask = len(os.sched_getaffinity(0)) + check(before >= mask, f"cpu_count {before} is narrower than the affinity mask {mask}") + +# process_cpu_count is either the mask or cpu_count itself; neither exceeds it. +if hasattr(os, "process_cpu_count") and before is not None: + p = os.process_cpu_count() + check(p is None or p <= before, f"process_cpu_count {p} exceeds cpu_count {before}") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/os_dup2_inheritable.py b/pyre/extra_tests/parity_tests/os_dup2_inheritable.py new file mode 100644 index 00000000000..f19cc409c83 --- /dev/null +++ b/pyre/extra_tests/parity_tests/os_dup2_inheritable.py @@ -0,0 +1,81 @@ +"""`os.dup2` binds its third argument by name, and honours it. + +`dup2(fd, fd2, inheritable=False)` is the call that asks for a descriptor an +`exec` will not carry. A registration that cannot split keywords off the +argument slice drops the request silently — the answer is still a descriptor, +so nothing raises and only `get_inheritable` tells you the flag went the other +way. That is what this asserts first; the binding errors around it are the +same defect seen from the side where it is loud. + +The arity *message* text is deliberately not asserted. CPython spells it two +ways depending on whether the parameters sit after the positional-only `/`, and +pyre's builtins do not yet carry that marker — only the exception type is +parity today. +""" + +import os + + +def check(cond, what): + if not cond: + raise AssertionError(what) + + +def raises(what, fn): + try: + fn() + except TypeError: + return + except Exception as e: + raise AssertionError(f"{what}: raised {type(e).__name__}, expected TypeError") + raise AssertionError(f"{what}: no exception") + + +r, w = os.pipe() +# A descriptor we own and may overwrite. dup2 closes its target first, so +# every case below can reuse this number. +target = os.dup(r) +try: + # ── the property ───────────────────────────────────────────────────── + os.dup2(r, target, inheritable=False) + check( + os.get_inheritable(target) is False, + "dup2(..., inheritable=False) produced an inheritable descriptor", + ) + # The same request positionally, so a failure tells keyword binding from + # the flag being ignored outright. + os.dup2(r, target, False) + check( + os.get_inheritable(target) is False, + "dup2(..., False) produced an inheritable descriptor", + ) + # The default is the other way, so the assertion above cannot pass by a + # constant answer. + os.dup2(r, target) + check( + os.get_inheritable(target) is True, + "dup2 defaulted to a non-inheritable descriptor", + ) + os.dup2(r, target, True) + check(os.get_inheritable(target) is True, "dup2(..., True) was not inheritable") + + # ── the binding, from the side that raises ─────────────────────────── + # Both required arguments by name. + check(os.dup2(fd=r, fd2=target) == target, "dup2(fd=, fd2=) did not answer fd2") + # One positional, one by name. + check(os.dup2(r, fd2=target) == target, "dup2(r, fd2=) did not answer fd2") + + raises("dup2 with an unknown keyword", lambda: os.dup2(r, target, zzz=1)) + raises("dup2 with 5 positionals", lambda: os.dup2(r, target, True, "x", "y")) + raises("dup2 with fd2 given twice", lambda: os.dup2(r, target, fd2=target)) + raises("dup2 with one argument", lambda: os.dup2(r)) + raises("dup2 with no arguments", lambda: os.dup2()) + raises("dup2 with a str fd", lambda: os.dup2("a", target)) +finally: + for fd in (target, r, w): + try: + os.close(fd) + except OSError: + pass + +print("OK") diff --git a/pyre/extra_tests/parity_tests/os_path_argument_types.py b/pyre/extra_tests/parity_tests/os_path_argument_types.py index 07359f19c07..fa22b61465b 100644 --- a/pyre/extra_tests/parity_tests/os_path_argument_types.py +++ b/pyre/extra_tests/parity_tests/os_path_argument_types.py @@ -5,10 +5,17 @@ boundaries reports a TypeError rather than addressing a file the caller never named. -The message has two shapes: an entry point that converts its own argument names -itself and lists what it takes, and the two that convert a path on someone -else's behalf (`os.fspath`, `os.fsencode`) name no caller. Both report the type -by its own name, without the module that qualifies it in other messages. +The message has two shapes. An entry point that converts its own argument names +itself, names the argument it turned away, and lists what that argument takes — +the list widens with `integer` exactly where the call can work on a descriptor, +so `stat` and `lstat` word it differently. The ones that convert a path on +someone else's behalf (`os.fspath`, `os.fsencode`) name no caller. Both report +the type by its own name, without the module that qualifies it in other +messages. + +`os.startfile`, `os.listmounts`, Windows' `os.system` and the `_get*name` +family are left out: they exist on Windows alone, so their wording cannot be +measured against the oracle from here. """ import array @@ -37,6 +44,12 @@ def rejects(fn, arg, what, message=None): d = tempfile.mkdtemp() atexit.register(shutil.rmtree, d, ignore_errors=True) b = d.encode() +# A name that exists, for the boundaries that take a second path: the argument +# under test is the one that has to be refused, so the other has to be good. +# It is never opened — the conversion fails before any of these calls reaches +# the filesystem. +GOOD = os.path.join(d, "good") +open(GOOD, "wb").close() # `array.array` is here for its name alone: the qualified `array.array` is what # other messages report ("sequence item 0: expected str instance, array.array @@ -47,13 +60,15 @@ def rejects(fn, arg, what, message=None): ("array", array.array("B", b)), ] -# Every path-taking entry point, with whatever it wants after the path. The -# message is pinned only for the boundaries that word it the same way on both -# interpreters today; the rest are asserted to reject, which is the property -# this file is about. (os.unlink and its neighbours name themselves on CPython -# and do not here — see the follow-up task on the unnamed path-only message.) +# Every path-taking entry point, with whatever it wants after the path, and the +# message it words the rejection with. `path_converter` fills the function name +# and the argument name from the argument clinic, so the entry point names +# itself and says which of its arguments it turned away — `link` calls its two +# `src` and `dst` rather than both `path`. +PATH_ONLY = "string, bytes or os.PathLike" +WITH_FD = "string, bytes, os.PathLike or integer" PINNED = { - "stat": ((), "stat: path should be string, bytes, os.PathLike or integer, not {}"), + "stat": ((), f"stat: path should be {WITH_FD}, not {{}}"), # listdir names `integer` only where it can open a directory descriptor; # the Windows build has no fdopendir and leaves that word out. "listdir": ( @@ -62,22 +77,62 @@ def rejects(fn, arg, what, message=None): if sys.platform != "win32" else "listdir: path should be string, bytes, os.PathLike or None, not {}", ), - "lchown": ((-1, -1), "lchown: path should be string, bytes or os.PathLike, not {}"), + "lchown": ((-1, -1), f"lchown: path should be {PATH_ONLY}, not {{}}"), + "access": ((0,), f"access: path should be {PATH_ONLY}, not {{}}"), + # `chdir` names `integer` for the same reason `listdir` does — it can + # `fchdir`, and the Windows build cannot. + "chdir": ( + (), + f"chdir: path should be {WITH_FD}, not {{}}" + if sys.platform != "win32" + else f"chdir: path should be {PATH_ONLY}, not {{}}", + ), + "chmod": ((0o644,), f"chmod: path should be {WITH_FD}, not {{}}"), + "chroot": ((), f"chroot: path should be {PATH_ONLY}, not {{}}"), + "mkdir": ((), f"mkdir: path should be {PATH_ONLY}, not {{}}"), + "open": ((0,), f"open: path should be {PATH_ONLY}, not {{}}"), + "readlink": ((), f"readlink: path should be {PATH_ONLY}, not {{}}"), + "remove": ((), f"remove: path should be {PATH_ONLY}, not {{}}"), + "rmdir": ((), f"rmdir: path should be {PATH_ONLY}, not {{}}"), + "scandir": ( + (), + "scandir: path should be string, bytes, os.PathLike, integer or None, not {}" + if sys.platform != "win32" + else "scandir: path should be string, bytes, os.PathLike or None, not {}", + ), + "truncate": ((0,), f"truncate: path should be {WITH_FD}, not {{}}"), + "unlink": ((), f"unlink: path should be {PATH_ONLY}, not {{}}"), + "utime": ((), f"utime: path should be {WITH_FD}, not {{}}"), + "mkfifo": ((), f"mkfifo: path should be {PATH_ONLY}, not {{}}"), + "lstat": ((), f"lstat: path should be {PATH_ONLY}, not {{}}"), + "statvfs": ((), f"statvfs: path should be {WITH_FD}, not {{}}"), + "chown": ((-1, -1), f"chown: path should be {WITH_FD}, not {{}}"), + "pathconf": (("PC_NAME_MAX",), f"pathconf: path should be {WITH_FD}, not {{}}"), + "mknod": ((), f"mknod: path should be {PATH_ONLY}, not {{}}"), + # The `l`-prefixed calls act on the link itself, so none of them can be + # handed a descriptor and none names `integer`. chflags and its neighbours + # are BSD's, absent elsewhere, and skipped by the getattr below. + "lchmod": ((0o644,), f"lchmod: path should be {PATH_ONLY}, not {{}}"), + "chflags": ((0,), f"chflags: path should be {PATH_ONLY}, not {{}}"), + "lchflags": ((0,), f"lchflags: path should be {PATH_ONLY}, not {{}}"), + # execv and execve name their path and nothing else: the argv entries and + # the environment keys and values are converted on the sequence's or the + # mapping's behalf, so those report the caller-less message. + "execv": ((["a"],), f"execv: path should be {PATH_ONLY}, not {{}}"), + "execve": ((["a"], {}), f"execve: path should be {PATH_ONLY}, not {{}}"), } -UNPINNED = { - "access": (0,), - "chdir": (), - "chmod": (0o644,), - "mkdir": (), - "open": (0,), - "readlink": (), - "remove": (), - "rmdir": (), - "scandir": (), - "truncate": (0,), - "unlink": (), - "utime": (), +# The boundaries that take two paths name them apart. `rename` and `replace` +# are one implementation here and two clinic declarations there, so each has to +# answer with its own name. +PAIRS = { + "rename": ("src", "dst"), + "replace": ("src", "dst"), + "link": ("src", "dst"), + "symlink": ("src", "dst"), } +# posix_spawn and posix_spawnp share a body too, and the path they reject is +# named after whichever the caller reached. +SPAWN = ("posix_spawn", "posix_spawnp") for name, buf in BUFFERS: for fn_name, (rest, message) in PINNED.items(): @@ -86,14 +141,38 @@ def rejects(fn, arg, what, message=None): continue rejects(lambda a: fn(a, *rest), buf, f"{fn_name}({name})", message.format(name)) - for fn_name, rest in UNPINNED.items(): + for fn_name, (first, second) in PAIRS.items(): + fn = getattr(os, fn_name, None) + if fn is None: + continue + rejects( + lambda a: fn(a, GOOD), + buf, + f"{fn_name}({name}, ...)", + f"{fn_name}: {first} should be {PATH_ONLY}, not {name}", + ) + rejects( + lambda a: fn(GOOD, a), + buf, + f"{fn_name}(..., {name})", + f"{fn_name}: {second} should be {PATH_ONLY}, not {name}", + ) + + for fn_name in SPAWN: fn = getattr(os, fn_name, None) if fn is None: continue - rejects(lambda a: fn(a, *rest), buf, f"{fn_name}({name})") + rejects( + lambda a: fn(a, ["x"], {}), + buf, + f"{fn_name}({name})", + f"{fn_name}: path should be {PATH_ONLY}, not {name}", + ) - # The two that convert on someone else's behalf name no caller. - for fn in (os.fsencode, os.fspath): + # The ones that convert on someone else's behalf name no caller: the two + # public converters, the POSIX `system`, and every element a sequence or a + # mapping is walked for. + for fn in (os.fsencode, os.fspath, os.fsdecode): rejects( fn, buf, @@ -101,6 +180,24 @@ def rejects(fn, arg, what, message=None): f"expected str, bytes or os.PathLike object, not {name}", ) + UNNAMED = f"expected str, bytes or os.PathLike object, not {name}" + if sys.platform != "win32": + rejects(os.system, buf, f"system({name})", UNNAMED) + rejects( + lambda a: os.execv(GOOD, [a]), + buf, + f"execv(argv item {name})", + UNNAMED, + ) + # An environment *key* never reaches the converter: every buffer here is + # mutable and so unhashable, and the dict turns it away first. + rejects( + lambda a: os.execve(GOOD, ["a"], {"k": a}), + buf, + f"execve(env value {name})", + UNNAMED, + ) + # A buffer being refused must not have cost `bytes` its own arm. check(os.stat(d).st_mode == os.stat(b).st_mode, "a bytes path stopped working") check(os.fsencode(b) == b, "fsencode(bytes)") diff --git a/pyre/extra_tests/parity_tests/os_sched_policy_pipe2.py b/pyre/extra_tests/parity_tests/os_sched_policy_pipe2.py new file mode 100644 index 00000000000..14a72d1ac8e --- /dev/null +++ b/pyre/extra_tests/parity_tests/os_sched_policy_pipe2.py @@ -0,0 +1,202 @@ +"""The Linux-only posix names: pipe2 and the scheduling-policy group. + +`pipe2` and `sched_getparam`/`sched_setparam`/`sched_getscheduler`/ +`sched_setscheduler` were absent, and so was the `sched_param` type the four +exchange. The group is the host's, not the module's — a host whose libc has no +`sched_setscheduler` publishes none of it — so what is asserted here is the +shape that holds wherever the names do exist, plus the fact that they exist +nowhere else. + +`os.dup3` is deliberately not checked for: no host publishes such a name. +""" + +import os +import sys + + +def check(cond, what): + if not cond: + raise AssertionError(what) + + +def raises(call, exc): + try: + call() + except exc: + return + raise AssertionError(f"{exc.__name__} was not raised") + + +check(not hasattr(os, "dup3"), "os grew a dup3") + +# ── the flag set ─────────────────────────────────────────────── +# The values are the host header's own, so what is checked is which names +# exist and that each is an int. The split is the hosts' own: seven on every +# Unix, then one group per platform. `nt` has none of them. +FLAGS = { + "": ("O_ACCMODE", "O_ASYNC", "O_CLOEXEC", "O_DIRECTORY", "O_FSYNC", "O_NOCTTY", + "O_NOFOLLOW"), + "linux": ("O_DIRECT", "O_LARGEFILE", "O_NOATIME", "O_PATH", "O_RSYNC", "O_TMPFILE"), + "darwin": ("O_EVTONLY", "O_EXEC", "O_EXLOCK", "O_NOFOLLOW_ANY", "O_SEARCH", "O_SHLOCK", + "O_SYMLINK"), +} +if sys.platform != "win32": + expected = FLAGS[""] + for key, names in FLAGS.items(): + if key and sys.platform.startswith(key): + expected += names + for name in expected: + check(hasattr(os, name), f"os has no {name}") + check(isinstance(getattr(os, name), int), f"{name} is not an int") + # O_CLOEXEC is the flag `pipe2` exists for, and a zero there would be a flag + # that silently leaves the descriptor inheritable. + check(os.O_CLOEXEC != 0, "O_CLOEXEC is 0") + +# ── pipe2 ──────────────────────────────────────────────────────────────── +if hasattr(os, "pipe2"): + # Unlike `pipe`, no inheritance is forced on the pair: the flags argument is + # the whole of the caller's control over it. + r, w = os.pipe2(0) + try: + check(isinstance(r, int) and isinstance(w, int), "pipe2 answered non-ints") + check(r != w, "pipe2 answered one descriptor twice") + check(os.get_inheritable(r), "pipe2(0) read end is not inheritable") + check(os.get_inheritable(w), "pipe2(0) write end is not inheritable") + os.write(w, b"x") + check(os.read(r, 1) == b"x", "the pipe2 pair is not connected") + finally: + os.close(r) + os.close(w) + + r, w = os.pipe2(os.O_CLOEXEC) + try: + check(not os.get_inheritable(r), "pipe2(O_CLOEXEC) read end is inheritable") + check(not os.get_inheritable(w), "pipe2(O_CLOEXEC) write end is inheritable") + finally: + os.close(r) + os.close(w) + + raises(lambda: os.pipe2(), TypeError) + raises(lambda: os.pipe2("x"), TypeError) + raises(lambda: os.pipe2(-1), OSError) +elif sys.platform.startswith(("linux", "freebsd", "netbsd", "openbsd", "dragonfly")): + raise AssertionError("this host has pipe2 and os does not publish it") + +# ── the CPU affinity mask ──────────────────────────────────────────────── +if hasattr(os, "sched_getaffinity"): + cpus = os.sched_getaffinity(0) + check(isinstance(cpus, set), f"sched_getaffinity answered a {type(cpus).__name__}") + check(cpus, "this process is affine to no CPU at all") + check(all(isinstance(c, int) for c in cpus), "a CPU number is not an int") + check(all(c >= 0 for c in cpus), "a CPU number is negative") + + raises(lambda: os.sched_getaffinity(), TypeError) + raises(lambda: os.sched_getaffinity("x"), TypeError) + # A pid nobody runs under. ProcessLookupError is an OSError. + raises(lambda: os.sched_getaffinity(-1), OSError) + + try: + # The mask this process already has, spelled three ways: a set, a list + # and a bare iterator are all accepted, and all answer None. + check(os.sched_setaffinity(0, cpus) is None, "sched_setaffinity answered a value") + check(os.sched_setaffinity(0, list(cpus)) is None, "a list mask was refused") + check(os.sched_setaffinity(0, iter(cpus)) is None, "an iterator mask was refused") + check(os.sched_getaffinity(0) == cpus, "the mask changed under a no-op write") + except PermissionError: + # A host that refuses a scheduler write refuses it here too. + pass + + # An empty mask leaves nothing to run on, and the kernel says so. + raises(lambda: os.sched_setaffinity(0, []), OSError) + # A CPU that cannot be one: negative, wider than the mask, wider than a C int. + raises(lambda: os.sched_setaffinity(0, [-1]), ValueError) + raises(lambda: os.sched_setaffinity(0, [99999]), OSError) + raises(lambda: os.sched_setaffinity(0, [2**40]), OverflowError) + # Not a CPU number at all, and not an iterable at all. + raises(lambda: os.sched_setaffinity(0, ["x"]), TypeError) + raises(lambda: os.sched_setaffinity(0, 5), TypeError) + raises(lambda: os.sched_setaffinity(0), TypeError) + raises(lambda: os.sched_setaffinity(-1, cpus), OSError) + + try: + os.sched_setaffinity(0, cpus) + except PermissionError: + pass + check(os.sched_getaffinity(0) == cpus, "the mask did not survive the refusals") + +# ── sched_param and the policy calls ───────────────────────────────────── +if not hasattr(os, "sched_getparam"): + check(not hasattr(os, "sched_param"), "sched_param without sched_getparam") + check(not hasattr(os, "sched_setparam"), "sched_setparam without sched_getparam") + check(not hasattr(os, "sched_getscheduler"), "sched_getscheduler alone") + print("OK") + raise SystemExit + +# The type carries one field and takes the priority itself, not a sequence. +param = os.sched_param(5) +check(type(param).__name__ == "sched_param", f"the type is {type(param).__name__}") +check(os.sched_param.__module__ == "posix", f"module is {os.sched_param.__module__!r}") +check(os.sched_param.n_fields == 1, f"n_fields is {os.sched_param.n_fields}") +check(isinstance(param, tuple), "sched_param is not a tuple") +check(len(param) == 1, f"len(sched_param(5)) is {len(param)}") +check(param[0] == 5, f"sched_param(5)[0] is {param[0]!r}") +check(param.sched_priority == 5, f"sched_priority is {param.sched_priority!r}") +check(repr(param) == "posix.sched_param(sched_priority=5)", f"repr is {repr(param)}") +check(os.sched_param(0) == os.sched_param(0), "two equal sched_params differ") + +# Reading this process's own policy and priority always works. +policy = os.sched_getscheduler(0) +check(isinstance(policy, int), f"sched_getscheduler answered {policy!r}") +# SCHED_BATCH and SCHED_IDLE are Linux's alone, so the set is the host's. +known = [getattr(os, n) for n in ("SCHED_OTHER", "SCHED_FIFO", "SCHED_RR", + "SCHED_BATCH", "SCHED_IDLE") if hasattr(os, n)] +check(policy in known, f"sched_getscheduler answered an unknown policy {policy!r}") +current = os.sched_getparam(0) +check(isinstance(current, os.sched_param), f"sched_getparam answered {type(current).__name__}") +check(isinstance(current.sched_priority, int), "sched_priority is not an int") + +raises(lambda: os.sched_getparam(), TypeError) +raises(lambda: os.sched_getscheduler("x"), TypeError) +# A pid nobody runs under, and one that is not a pid at all. +raises(lambda: os.sched_getparam(-1), OSError) +raises(lambda: os.sched_getscheduler(-1), OSError) + +# The round-robin quantum is one float, not the timespec pair the call fills. +if hasattr(os, "sched_rr_get_interval"): + quantum = os.sched_rr_get_interval(0) + check(isinstance(quantum, float), f"sched_rr_get_interval answered {quantum!r}") + check(quantum >= 0.0, f"sched_rr_get_interval answered {quantum!r}") + raises(lambda: os.sched_rr_get_interval(), TypeError) + raises(lambda: os.sched_rr_get_interval("x"), TypeError) + raises(lambda: os.sched_rr_get_interval(-1), OSError) + +if not hasattr(os, "sched_setparam"): + print("OK") + raise SystemExit + +# Both setters answer None, and both demand the type rather than a bare int or +# a tuple that would index the same. +try: + check(os.sched_setparam(0, current) is None, "sched_setparam answered a value") + check(os.sched_setscheduler(0, policy, current) is None, + "sched_setscheduler answered a value") +except PermissionError: + # A host that refuses a scheduler write refuses it here too, and nothing + # below depends on the write having gone through. + pass +raises(lambda: os.sched_setparam(0, 5), TypeError) +raises(lambda: os.sched_setparam(0, (5,)), TypeError) +raises(lambda: os.sched_setscheduler(0, policy, 5), TypeError) +raises(lambda: os.sched_setparam(0), TypeError) + +# A priority the C int cannot hold is refused before the call is made. +raises(lambda: os.sched_setparam(0, os.sched_param(2**31)), OverflowError) +raises(lambda: os.sched_setparam(0, os.sched_param(-(2**31) - 1)), OverflowError) +raises(lambda: os.sched_setscheduler(0, policy, os.sched_param(2**31)), OverflowError) + +# A policy the host does not define, and a priority outside the policy's band, +# are the host's to refuse. +raises(lambda: os.sched_setscheduler(0, 12345, current), OSError) +raises(lambda: os.sched_setparam(0, os.sched_param(99)), OSError) + +print("OK") diff --git a/pyre/extra_tests/parity_tests/os_utime_pathconf_truncate.py b/pyre/extra_tests/parity_tests/os_utime_pathconf_truncate.py index 9df2f029801..0da16f8e49a 100644 --- a/pyre/extra_tests/parity_tests/os_utime_pathconf_truncate.py +++ b/pyre/extra_tests/parity_tests/os_utime_pathconf_truncate.py @@ -14,6 +14,7 @@ import shutil import sys import tempfile +import time def check(cond, what): @@ -136,6 +137,30 @@ def storable(ns): if st.st_mtime == FAR: check(st.st_mtime_ns == 880_000_000_000_000_000_000, st.st_mtime_ns) +# ── utime with no time named ────────────────────────────────────────────── +# Neither `times` nor `ns` means "now", and the filesystem is the one that +# answers what now is — the call carries UTIME_NOW rather than a stamp read +# off this process's clock. That distinction is not observable from here (both +# land within a second of `time.time()`); what is observable is that the flag +# reached the call at all, since a dropped one would write the zero pair and +# date the file to the epoch. +OLD = 10**18 # 2001-09-09 in nanoseconds — a stamp no clock here will answer + +os.utime(p, ns=(OLD, OLD)) +before = time.time() +os.utime(p) +check(abs(os.stat(p).st_mtime - before) < 60, f"utime(p) -> {os.stat(p).st_mtime}") +check(abs(os.stat(p).st_atime - before) < 60, "utime(p) left the access time behind") + +if os.utime in os.supports_fd: + os.utime(p, ns=(OLD, OLD)) + fd = os.open(p, os.O_RDWR) + try: + os.utime(fd) + finally: + os.close(fd) + check(abs(os.stat(p).st_mtime - before) < 60, f"utime(fd) -> {os.stat(p).st_mtime}") + # Back to a time the rest of the file can be reasoned about. os.utime(p, ns=(1_000_000_000, 2_000_000_000)) diff --git a/pyre/extra_tests/parity_tests/str_lone_surrogate_names.py b/pyre/extra_tests/parity_tests/str_lone_surrogate_names.py new file mode 100644 index 00000000000..bf25f22d0a1 --- /dev/null +++ b/pyre/extra_tests/parity_tests/str_lone_surrogate_names.py @@ -0,0 +1,80 @@ +"""A `str` holding a lone surrogate survives being used as a name. + +pyre stores every `str` as WTF-8 and decodes every operating-system-supplied +name with `surrogateescape`, so U+DC80..U+DCFF is reachable from ordinary code: +`sys.argv`, and on any filesystem that does not enforce UTF-8, every filename. +The `&str` accessor that most internals read through has no view of such a +value and used to abort the process rather than return one. + +`func.__name__ = ` was the reachable instance: it aborted with +`w_str_get_value: backing Wtf8Buf is not valid UTF-8 (lone surrogate)`. The +surrogate is built here from bytes rather than written as a literal, because +that is the only way it arrives in practice — and because a source file cannot +carry one. + +Round-tripping is asserted, not just survival. The name is mirrored into a +UTF-8-only slot for the internal `&str` readers, and an escaped mirror would +satisfy "did not crash" while quietly returning a different string. +""" + +import sys +import types + +# b'\xff\xfe' is not valid UTF-8 in any position, so surrogateescape maps each +# byte to U+DCFF / U+DCFE — exactly what a filesystem name or an argv element +# produces on a host that does not enforce UTF-8. +SURR = b"bad\xff\xfename".decode("utf-8", "surrogateescape") + + +def check(cond, what): + if not cond: + raise AssertionError(what) + + +# The payload is the point of the test, so verify it before using it: a build +# that silently dropped the surrogates would otherwise pass everything below. +check(len(SURR) == 9, f"surrogate payload is {len(SURR)} code points, expected 9") +check(SURR.encode("utf-8", "surrogateescape") == b"bad\xff\xfename", "payload does not round-trip") +check(any(0xDC80 <= ord(c) <= 0xDCFF for c in SURR), "payload carries no lone surrogate") + + +def f(): + return 0 + + +# ── the assignment that used to abort ──────────────────────────────────── +f.__name__ = SURR +check(f.__name__ == SURR, f"__name__ came back as {f.__name__!r}, not the value set") +check(isinstance(repr(f), str), "repr of a surrogate-named function is not a str") + +# The qualname slot is separate and must not have been clobbered by the name. +f.__qualname__ = SURR +check(f.__qualname__ == SURR, f"__qualname__ came back as {f.__qualname__!r}") + +# ── the constructor arm takes the same name ────────────────────────────── +g = types.FunctionType(f.__code__, {}, SURR) +check(g.__name__ == SURR, f"FunctionType name came back as {g.__name__!r}") + +# ── a plain function is unaffected ─────────────────────────────────────── +def h(): + return 0 + + +check(h.__name__ == "h", "an ordinary function lost its name") +h.__name__ = "renamed" +check(h.__name__ == "renamed", "an ordinary rename stopped working") + +# ── the same value through the surfaces that read names ────────────────── +check(SURR in {SURR: 1}, "a surrogate-bearing key does not find itself in a dict") +check(hash(SURR) == hash(SURR), "hashing a surrogate-bearing str is not stable") +o = type("T", (), {})() +setattr(o, SURR, 7) +check(getattr(o, SURR) == 7, "attribute set/get by a surrogate-bearing name lost the value") +check(SURR in vars(o), "the surrogate-bearing attribute is missing from vars()") + +# sys.argv is the other producer reachable without a filesystem; when this test +# is handed the payload it must arrive intact. +if len(sys.argv) > 1: + check(sys.argv[1] == SURR, f"argv did not carry the payload: {sys.argv[1]!r}") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/sys_audit_hooks.py b/pyre/extra_tests/parity_tests/sys_audit_hooks.py new file mode 100644 index 00000000000..2a40c390bdb --- /dev/null +++ b/pyre/extra_tests/parity_tests/sys_audit_hooks.py @@ -0,0 +1,203 @@ +"""`sys.addaudithook` installs a hook that `sys.audit` actually calls. + +`addaudithook` was `|_| Ok(w_none())` — it accepted a hook and dropped it — so +no hook could ever fire and the 21 stdlib modules that call `sys.audit` +(`shutil`, `subprocess`, `webbrowser`, `glob`, `pickle`, ...) reported nothing +to anyone. + +A hook can never be removed, so every assertion below has to be written for a +process whose hook set only grows; the raising cases route through one +module-level switch that is put back immediately. The interpreters also +disagree on which *internal* operations raise an event at all — CPython audits +`compile`, `exec`, `import` and much else, pyre raises only what a +`sys.audit(...)` call names — so the recorder keeps just the events this file +names, plus `sys.addaudithook`, which both raise. +""" + +import sys + +ERRORS = [] + + +def check(cond, what): + if not cond: + ERRORS.append(what) + + +def raises(what, exc, fn): + try: + fn() + except exc as e: + return e + except BaseException as e: + ERRORS.append(f"{what}: raised {type(e).__name__}({e}), expected {exc.__name__}") + return None + ERRORS.append(f"{what}: no exception, expected {exc.__name__}") + return None + + +# Only these reach a recorder, so an interpreter that audits more of its own +# internals than the other does not change what is compared. +WATCHED = {"pyre.first", "pyre.second", "pyre.raising", "sys.addaudithook"} + +# Set to an exception instance to make every recorder raise it; put back to +# None on the next line. A hook that raises stops the hooks after it, so this +# is never left set. +raiser = None + + +class Recorder: + def __init__(self, name): + self.name = name + self.seen = [] + + def __call__(self, event, args): + if event in WATCHED: + self.seen.append((event, args)) + if raiser is not None: + raise raiser + + def events(self): + return [event for event, _ in self.seen] + + +# ── with no hook installed, audit is still the no-op it was ─────────────── +check(sys.audit("pyre.first") is None, "audit with no hook did not return None") + + +# ── the first hook ──────────────────────────────────────────────────────── +first = Recorder("first") +sys.addaudithook(first) +# The `sys.addaudithook` event is raised BEFORE the hook is stored, so a hook +# never sees its own installation. +check(first.seen == [], f"a new hook saw its own installation: {first.seen}") + +sys.audit("pyre.first", 1, 2) +check(len(first.seen) == 1, f"hook was not called: {first.seen}") +if first.seen: + event, args = first.seen[0] + check(event == "pyre.first", f"event was {event!r}") + check(args == (1, 2), f"args were {args!r}") + check(type(args) is tuple, f"args was a {type(args).__name__}, expected tuple") + # `@unwrap_spec(event="text")` unwraps and re-wraps, so what the hook is + # handed is a plain `str` even when the caller named a subclass. + check(type(event) is str, f"event was a {type(event).__name__}, expected str") + +# No argument at all is an empty tuple, not None. +first.seen.clear() +sys.audit("pyre.first") +check(first.seen == [("pyre.first", ())], f"no-argument audit gave {first.seen}") + + +class SubStr(str): + pass + + +first.seen.clear() +sys.audit(SubStr("pyre.first")) +check( + first.seen and type(first.seen[0][0]) is str, + f"a str subclass event reached the hook as {first.seen}", +) + + +# ── a second hook, and the order they run in ────────────────────────────── +second = Recorder("second") +first.seen.clear() +sys.addaudithook(second) +check( + first.events() == ["sys.addaudithook"], + f"installing a hook did not raise sys.addaudithook: {first.events()}", +) +check(second.seen == [], f"the second hook saw its own installation: {second.seen}") + +first.seen.clear() +second.seen.clear() +sys.audit("pyre.second", "x") +check(first.seen == [("pyre.second", ("x",))], f"first hook: {first.seen}") +check(second.seen == [("pyre.second", ("x",))], f"second hook: {second.seen}") + + +# ── a hook that raises on the install event refuses the new hook ────────── +# The new hook is never stored, whatever was raised. What the class decides is +# only whether the caller hears about it: anything derived from `Exception` is +# swallowed and `addaudithook` returns None, while a `BaseException` that is +# not an `Exception` comes back out. +def install_under(exc, hook): + """Install `hook` while every recorder raises `exc`; report what happened.""" + global raiser + first.seen.clear() + second.seen.clear() + hook.seen.clear() + raiser = exc + try: + return ("returned", sys.addaudithook(hook)) + except BaseException as e: # noqa: BLE001 - the outcome is the measurement + return ("raised", type(e)) + finally: + raiser = None + + +def installed(hook): + """Whether `hook` is in the hook list, asked by raising an event.""" + hook.seen.clear() + sys.audit("pyre.second") + return hook.seen != [] + + +for exc, expected in ( + (RuntimeError("no more hooks"), ("returned", None)), + (ValueError("not a veto"), ("returned", None)), + (Exception("plain"), ("returned", None)), + (KeyboardInterrupt(), ("raised", KeyboardInterrupt)), +): + name = type(exc).__name__ + refused = Recorder(name) + outcome = install_under(exc, refused) + check(outcome == expected, f"addaudithook under {name}: {outcome}, expected {expected}") + # The refusal came from the FIRST hook, so the second one never ran. + check(first.events() == ["sys.addaudithook"], f"under {name}, first hook: {first.events()}") + check(second.events() == [], f"under {name}, the refusal did not stop hook 2") + check(not installed(refused), f"a hook refused under {name} was installed anyway") + +# The hooks that were already there are untouched by any of that. +first.seen.clear() +second.seen.clear() +sys.audit("pyre.second") +check(len(first.seen) == 1 and len(second.seen) == 1, "the surviving hooks stopped firing") + + +# ── an exception from a hook during an ordinary audit propagates ────────── +first.seen.clear() +second.seen.clear() +raiser = ValueError("from a hook") +try: + raises("audit with a raising hook", ValueError, lambda: sys.audit("pyre.raising")) +finally: + raiser = None +check(first.events() == ["pyre.raising"], f"first hook: {first.events()}") +check( + second.events() == [], + f"a raising hook did not stop the ones after it: {second.events()}", +) + + +# ── and the hooks still work afterwards ─────────────────────────────────── +first.seen.clear() +second.seen.clear() +check(sys.audit("pyre.second", 7) is None, "audit stopped returning None") +check(first.seen == [("pyre.second", (7,))], f"first hook: {first.seen}") +check(second.seen == [("pyre.second", (7,))], f"second hook: {second.seen}") + +# The argument checks in front of the dispatch still report, with hooks +# installed, before any hook is reached. +first.seen.clear() +raises("audit(123) with hooks installed", TypeError, lambda: sys.audit(123)) +check(first.seen == [], f"a bad event name reached the hooks: {first.seen}") + +if ERRORS: + for e in ERRORS: + sys.stderr.write(f"FAIL: {e}\n") + raise AssertionError(f"{len(ERRORS)} divergence(s)") + +print("OK") diff --git a/pyre/extra_tests/parity_tests/unary_negative_int_min_jit.py b/pyre/extra_tests/parity_tests/unary_negative_int_min_jit.py new file mode 100644 index 00000000000..8b9bed9a78c --- /dev/null +++ b/pyre/extra_tests/parity_tests/unary_negative_int_min_jit.py @@ -0,0 +1,146 @@ +"""`-x` on an exact int under a JIT-hot loop, across the INT_MIN promote. + +`descr_neg` (intobject.py:628) answers `-x` with a machine int for every +operand except `INT_MIN`, which `_make_ovf2long` promotes to a `W_LongObject`. +The walker fold spells `-x` as `IntSubOvf(0, x)` and picks its guard from the +recorded operand -- `GUARD_NO_OVERFLOW` for a plain one, `GUARD_VALUE INT_MIN` +plus the bigint tail for the promoting one -- so each loop below pins one +direction of that split: a trace recorded on the promoting operand still has to +answer plain ones, and a trace recorded on a plain operand must not wrap +`INT_MIN` back to itself. + +The unary folds read the operand's exact builtin class at record time, which +settles only the object the trace saw. An `int` subclass keeps the builtin +`ob_type`, so the fold's `GUARD_CLASS INT` does not stop one from entering the +trace later; the subclass loops below hand the fold a plain int first and the +subclass only after the loop is hot, which is the arrival the `w_class` guard +has to side-exit. +""" + +ROUNDS = 3000 +INT_MIN = -9223372036854775807 - 1 +INT_MAX = 9223372036854775807 +TWO_63 = 9223372036854775808 + + +def promoting_only(): + """Every iteration promotes, so the trace records the INT_MIN operand.""" + out = [] + for _ in range(ROUNDS): + m = INT_MIN + n = -m + out = [n, type(n) is int, n == TWO_63, n - 1 == INT_MAX, n // 2] + return out + + +def promoting_then_plain(): + """A trace recorded on INT_MIN must still answer a plain operand.""" + out = [] + for i in range(ROUNDS): + m = INT_MIN if i < ROUNDS // 2 else 7 + n = -m + out.append((n, type(n) is int)) + return out[0], out[-1], len(out) + + +def plain_then_promoting(): + """A trace recorded on a plain operand must not wrap INT_MIN.""" + seen = [] + promoted = 0 + for i in range(ROUNDS): + m = 7 if i < ROUNDS // 2 else INT_MIN + n = -m + if n == TWO_63: + promoted += 1 + seen = [n, type(n) is int] + return seen, promoted, promoted == ROUNDS - ROUNDS // 2 + + +def alternating(): + """Both arms live in the same trace, taken every other iteration.""" + acc = 0 + wrapped = 0 + for i in range(ROUNDS): + m = INT_MIN if i % 2 else -1 + n = -m + if n == TWO_63: + acc += 1 + elif n == INT_MIN: + wrapped += 1 + return acc, wrapped + + +class Derived(int): + def __neg__(self): + return "NEG" + + def __pos__(self): + return "POS" + + def __invert__(self): + return "INV" + + +def _late_subclass(op, plain): + """Iterate plain ints until the loop is hot, then hand `op` a subclass. + + The arrival has to be data-driven: selecting the subclass through a branch + (`x = plain if i < N else Derived(plain)`) side-exits on the branch guard + before the fold is reached, so it answers correctly whether or not the + operand's class is pinned. + """ + items = [plain] * ROUNDS + [Derived(plain)] + out = None + for x in items: + out = op(x) + return out + + +def subclass_neg(): + """`-Derived(...)` after a plain-int trace must reach `Derived.__neg__`.""" + return ( + _late_subclass(lambda x: -x, 7), + _late_subclass(lambda x: -x, INT_MIN), + ) + + +def subclass_pos(): + """`+Derived(...)` must reach `Derived.__pos__`, not forward the operand.""" + return _late_subclass(lambda x: +x, 7) + + +def subclass_invert(): + """`~Derived(...)` must reach `Derived.__invert__`.""" + return _late_subclass(lambda x: ~x, 7) + + +def bool_operand(): + """`-True` is `-1`; a bool operand never reaches the promote.""" + out = [] + for _ in range(ROUNDS): + t = True + f = False + out = [-t, -f, type(-t) is int] + return out + + +def round_trip(): + """`-(-INT_MIN)` negates the promoted long straight back.""" + out = [] + for _ in range(ROUNDS): + m = INT_MIN + n = -(-m) + out = [n, n == INT_MIN, type(n) is int, n + 1 == -INT_MAX] + return out + + +print(promoting_only()) +print(promoting_then_plain()) +print(plain_then_promoting()) +print(alternating()) +print(subclass_neg()) +print(subclass_pos()) +print(subclass_invert()) +print(bool_operand()) +print(round_trip()) +print("OK") diff --git a/pyre/pyre-interpreter/src/function.rs b/pyre/pyre-interpreter/src/function.rs index 671c1986d69..d7cfc91cf19 100644 --- a/pyre/pyre-interpreter/src/function.rs +++ b/pyre/pyre-interpreter/src/function.rs @@ -1939,6 +1939,24 @@ pub unsafe fn function_get_func_name(obj: PyObjectRef) -> &'static str { unsafe { function_get_name(obj) } } +/// The UTF-8 mirror a `Function`'s raw `name` slot holds. +/// +/// `NameStorage` is a `String`, so a name carrying a lone surrogate — which +/// `surrogateescape` puts there for any byte an operating-system name failed +/// to decode — has no exact form in that slot. The `w_name` slot keeps the +/// object the caller supplied and is what `__name__` reads, so the stored +/// value survives; this is the `&str` view `repr` and the `__qualname__` +/// default consume, and it renders the surrogate rather than aborting the +/// process on it. +unsafe fn name_utf8_mirror(w_name: PyObjectRef) -> String { + match unsafe { pyre_object::w_str_get_value_opt(w_name) } { + Some(value) => value.to_string(), + None => unsafe { pyre_object::w_str_get_wtf8(w_name) } + .to_string_lossy() + .into_owned(), + } +} + /// PyPy-compatible `__name__` setter. /// /// A GC-managed (user) function boxes the new name in a GC-managed storage box @@ -1956,7 +1974,7 @@ pub unsafe fn function_set_func_name(obj: PyObjectRef, name: PyObjectRef) { } function_write_barrier(obj); (*(obj as *mut Function)).w_name = name; - let raw_name = pyre_object::w_str_get_value(name).to_string(); + let raw_name = name_utf8_mirror(name); let raw_name = if pyre_object::gc_hook::try_gc_owns_object(obj as *mut u8) { pyre_object::gc_storage::gc_alloc_storage_box( raw_name, @@ -2292,7 +2310,7 @@ pub unsafe fn descr_function__new__( unsafe { let _ = _argdefs; let name = if !w_name.is_null() && !pyre_object::is_none(w_name) { - pyre_object::w_str_get_value(w_name).to_string() + name_utf8_mirror(w_name) } else { String::new() }; @@ -2301,7 +2319,15 @@ pub unsafe fn descr_function__new__( } else { w_closure }; - function_new_with_closure(code, name, w_globals, closure) + let func = function_new_with_closure(code, name, w_globals, closure); + // `name` above is only the UTF-8 mirror. The object the caller passed + // is what `__name__` must answer with, so store it: otherwise the + // getter rebuilds one from the mirror and a name carrying a lone + // surrogate comes back escaped instead of equal to what went in. + if !w_name.is_null() && !pyre_object::is_none(w_name) { + function_set_name_obj(func, w_name); + } + func } } @@ -2413,7 +2439,7 @@ pub fn descr_function_new(args: &[PyObjectRef]) -> Result Result Result { path_or_fd_w(obj, None, false, false) } +/// [`fsencode_path_w`] for a path-only boundary that names itself. The argument +/// name is the second half: `path_converter` fills `function_name` and +/// `argument_name` from the argument clinic, so `link` rejects its first +/// argument as `src` and its second as `dst` rather than calling both `path`. +pub fn fsencode_path_named_w( + obj: pyre_object::PyObjectRef, + funcname: &str, + argname: &str, +) -> Result { + path_or_fd_w(obj, Some((funcname, argname)), false, 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`: @@ -1704,7 +1720,7 @@ pub fn fsencode_path_or_fd_w( funcname: &str, allow_fd: bool, ) -> Result { - path_or_fd_w(obj, Some(funcname), allow_fd, false) + path_or_fd_w(obj, Some((funcname, "path")), allow_fd, false) } /// [`fsencode_path_or_fd_w`] for a boundary whose path argument also takes @@ -1717,7 +1733,7 @@ pub fn fsencode_path_or_fd_nullable_w( funcname: &str, allow_fd: bool, ) -> Result { - path_or_fd_w(obj, Some(funcname), allow_fd, true) + path_or_fd_w(obj, Some((funcname, "path")), allow_fd, true) } /// `_PyType_Name` — the type's own name, with any module that qualifies it @@ -1734,13 +1750,13 @@ pub(crate) fn short_type_name(obj: pyre_object::PyObjectRef) -> String { fn path_or_fd_w( obj: pyre_object::PyObjectRef, - funcname: Option<&str>, + caller: Option<(&str, &str)>, allow_fd: bool, nullable: 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. + // interp_posix.py:170-180 builds this list from the same two flags. The + // caller pair is `path_converter`'s `function_name` and `argument_name`; + // the entry points that convert on someone else's behalf carry neither. let allowed_types = match (nullable, allow_fd) { (true, true) => "string, bytes, os.PathLike, integer or None", (true, false) => "string, bytes, os.PathLike or None", @@ -1749,9 +1765,9 @@ fn path_or_fd_w( }; let reject = |obj: pyre_object::PyObjectRef| -> crate::PyError { let tp = short_type_name(obj); - match funcname { - Some(name) => crate::PyError::type_error(format!( - "{name}: path should be {allowed_types}, not {tp}" + match caller { + Some((name, arg)) => crate::PyError::type_error(format!( + "{name}: {arg} should be {allowed_types}, not {tp}" )), None => crate::PyError::type_error(format!( "expected str, bytes or os.PathLike object, not {tp}" diff --git a/pyre/pyre-interpreter/src/host_seam.rs b/pyre/pyre-interpreter/src/host_seam.rs index 8ba5cab421d..3869e85a670 100644 --- a/pyre/pyre-interpreter/src/host_seam.rs +++ b/pyre/pyre-interpreter/src/host_seam.rs @@ -49,6 +49,16 @@ pub mod sys { c_char, c_int, c_long, c_uint, c_void, clockid_t, gid_t, mode_t, off_t, pid_t, rusage, size_t, time_t, timespec, timeval, tm, uid_t, }; + // The struct `sched_setparam`/`sched_setscheduler` fill in before handing it + // to host_env. Naming it allocates nothing and calls nothing; the calls + // themselves are overwritten by the sandbox stubs in interp_posix. + #[cfg(any( + target_os = "android", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" + ))] + pub use ::libc::sched_param; // Calendar/formatting on a caller-supplied value: `gmtime_r` reads only glibc's // timezone cache — which the seccomp backstop primes before lockdown // (see `pyre_sandbox::seccomp`), so at runtime it doesn't open a host file. @@ -64,12 +74,19 @@ pub mod sys { // Constants (added as sandbox-reachable modules need them). pub use ::libc::{ AT_FDCWD, CODESET, EBADF, EINTR, EINVAL, F_OK, LC_ALL, LC_COLLATE, LC_CTYPE, LC_MESSAGES, - LC_MONETARY, LC_NUMERIC, LC_TIME, O_APPEND, O_CREAT, O_DSYNC, O_EXCL, O_NONBLOCK, O_RDONLY, - O_RDWR, O_SYNC, O_TRUNC, O_WRONLY, PRIO_PGRP, PRIO_PROCESS, PRIO_USER, R_OK, RTLD_GLOBAL, + LC_MONETARY, LC_NUMERIC, LC_TIME, O_ACCMODE, O_APPEND, O_ASYNC, O_CLOEXEC, O_CREAT, + O_DIRECTORY, O_DSYNC, O_EXCL, O_FSYNC, O_NOCTTY, O_NOFOLLOW, O_NONBLOCK, O_RDONLY, O_RDWR, + O_SYNC, O_TRUNC, O_WRONLY, PRIO_PGRP, PRIO_PROCESS, PRIO_USER, R_OK, RTLD_GLOBAL, RTLD_LAZY, RTLD_LOCAL, RTLD_NODELETE, RTLD_NOLOAD, RTLD_NOW, RUSAGE_SELF, S_IFDIR, S_IFMT, S_IFREG, SEEK_CUR, SEEK_END, SEEK_SET, ST_NOSUID, ST_RDONLY, TIOCGWINSZ, W_OK, WCONTINUED, WEXITED, WNOHANG, WNOWAIT, WSTOPPED, WUNTRACED, X_OK, }; + // The `` flags only one platform declares, carrying the same gates + // as the table that publishes them. + #[cfg(any(target_os = "linux", target_os = "android"))] + pub use ::libc::{O_DIRECT, O_LARGEFILE, O_NOATIME, O_PATH, O_RSYNC, O_TMPFILE}; + #[cfg(any(target_os = "macos", target_os = "ios"))] + pub use ::libc::{O_EVTONLY, O_EXEC, O_EXLOCK, O_NOFOLLOW_ANY, O_SEARCH, O_SHLOCK, O_SYMLINK}; // `lockf`'s commands and `waitid`'s two vocabularies — which process it is // asked about, and what it reports happened. All are numbers the module // publishes; the calls themselves are refused under sandbox. diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index 162d75e079a..2658032a82e 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -424,6 +424,106 @@ fn waitid_result_seq_type() -> PyObjectRef { }) as PyObjectRef } +/// `posix.sched_param` structseq — the single field `app_posix.py:140-147` +/// declares. Its `__new__` takes the priority itself rather than a sequence, +/// which is what `_structseq.py:102-107` already gives every 1-field structseq. +#[cfg(all( + unix, + any( + target_os = "android", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" + ) +))] +fn sched_param_seq_type() -> PyObjectRef { + static T: std::sync::OnceLock = std::sync::OnceLock::new(); + *T.get_or_init(|| { + crate::_structseq::make_struct_seq("posix.sched_param", &["sched_priority"]) as usize + }) as PyObjectRef +} + +/// The `w_param` argument `sched_setparam` and `sched_setscheduler` share. +/// `interp_posix.py:3086-3092` refuses anything that is not a `sched_param`, +/// reads field 0 through the sequence protocol, and refuses a priority the C +/// `int` cannot hold. +#[cfg(all( + unix, + not(target_env = "musl"), + any( + target_os = "android", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" + ) +))] +fn sched_priority_w(w_param: PyObjectRef) -> Result { + if !crate::baseobjspace::isinstance(w_param, sched_param_seq_type())? { + return Err(crate::PyError::type_error("must have a sched_param object")); + } + let w_priority = crate::baseobjspace::getitem(w_param, pyre_object::w_int_new(0))?; + let priority = crate::baseobjspace::int_w(w_priority)?; + i32::try_from(priority) + .map_err(|_| crate::PyError::overflow_error("sched_priority out of range")) +} + +/// `rpy_cpu_count` — `rposix.py:2968-3006` splits the processor count three +/// ways; these are its two Unix arms, `sysconf(_SC_NPROCESSORS_ONLN)` on linux +/// and gnu and `sysctl(CTL_HW, HW_NCPU)` on the Apple and BSD targets. The +/// third is Windows' and stays with the Windows registration. Anywhere else +/// there is no answer and the count is 0, which `cpu_count` reports as None +/// (`interp_posix.py:2910-2914` `if count <= 0`). +/// +/// This is deliberately not the thread count. `get_number_of_os_threads` reads +/// `/proc/self/stat`'s `num_threads` and the mach `task_threads` count, and +/// serves `warn_if_multi_threaded` in the fork path; answering `cpu_count` with +/// it reports how many threads happen to be alive, which moves under the +/// caller's feet. +#[cfg(not(feature = "sandbox"))] +fn host_cpu_count() -> i64 { + #[cfg(any(target_os = "linux", target_os = "android"))] + let ncpu = { + let n = unsafe { libc::sysconf(libc::_SC_NPROCESSORS_ONLN) }; + if n < 0 { 0 } else { n as i64 } + }; + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + ))] + let ncpu = { + let mut ncpu: libc::c_int = 0; + let mut mib: [libc::c_int; 2] = [libc::CTL_HW, libc::HW_NCPU]; + let mut len = core::mem::size_of::(); + let rc = unsafe { + libc::sysctl( + mib.as_mut_ptr(), + 2, + (&mut ncpu as *mut libc::c_int).cast(), + &mut len, + core::ptr::null_mut(), + 0, + ) + }; + if rc != 0 { 0 } else { ncpu as i64 } + }; + #[cfg(not(any( + target_os = "android", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "linux", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + )))] + let ncpu = 0i64; + ncpu +} + /// `os.times_result` structseq — `(user, system, children_user, /// children_system, elapsed)`; repr renders "posix.times_result(...)", or /// "nt.times_result(...)" on the host whose module is spelled that way. The @@ -574,6 +674,11 @@ mod win_nt { /// Read argument 0 as a filesystem path; the flag reports whether the /// input was bytes so the result can be encoded back to match. + /// + /// The conversion is the caller-less one. Every entry point reached through + /// here is Windows-only, so what it should name itself and its argument is + /// not something this host can measure, and a guessed wording would be + /// worse than the one uniform gap. See the follow-up task. fn arg_path( args: &[PyObjectRef], func: &str, @@ -916,13 +1021,13 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // _have_functions — list of HAVE_* macro names that were defined at // build time. os.py uses this to populate the supports_* capability sets - // (supports_dir_fd / supports_fd / supports_follow_symlinks), which - // callers like shutil.rmtree consult to choose between fd-relative and - // path-based implementations. Only the macros whose functionality is - // actually implemented may be listed: of the `*at` family that is - // HAVE_FSTATAT, HAVE_FCHOWNAT and HAVE_UTIMENSAT, the three calls that - // resolve a dir_fd-relative name. HAVE_LSTAT remains so os.stat is reported - // in supports_follow_symlinks (follow_symlinks=False works). + // (supports_dir_fd / supports_effective_ids / supports_fd / + // supports_follow_symlinks), which callers like shutil.rmtree consult to + // choose between fd-relative and path-based implementations. Only the + // macros whose functionality is actually implemented may be listed — the + // entry beside each one below names the claim os.py reads out of it, so a + // bit whose call is still missing has no entry at all rather than a + // qualified one. // // Each bit is the same constant the entry point itself branches on, so the // advertisement cannot drift from the behaviour: a build where `chdir` @@ -931,6 +1036,9 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // probes/mutators are raising stubs, and on the hosts that carry no // `host_env::posix` at all. let have_functions: &[(&str, bool)] = &[ + // os.py:117,137,158 reads this as `access` honouring all three of its + // modifiers, which is the one `faccessat` its body makes. + ("HAVE_FACCESSAT", HAVE_FACCESSAT), ("HAVE_FCHDIR", HAVE_FCHDIR), ("HAVE_FCHMOD", HAVE_FCHMOD), ("HAVE_FCHOWN", HAVE_FCHOWN), @@ -1037,6 +1145,57 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ("O_NDELAY", libc::O_NONBLOCK as i64), #[cfg(not(any(unix, windows)))] ("O_NDELAY", 0i64), + // `moduledef.py:264-266` publishes O_CLOEXEC by name wherever the host + // has it, which is every Unix. `nt` has no such flag -- it spells the + // same intent O_NOINHERIT -- so a zero there would be a flag that + // silently leaves the descriptor inheritable. + #[cfg(unix)] + ("O_CLOEXEC", libc::O_CLOEXEC as i64), + // The rest of the `` set. Each value is the host header's own, + // and the split below is the hosts' own too: these six are on every + // Unix, the next two groups are one platform's each. `nt` has none of + // them and is left with the flags it does have. + #[cfg(unix)] + ("O_ACCMODE", libc::O_ACCMODE as i64), + #[cfg(unix)] + ("O_ASYNC", libc::O_ASYNC as i64), + #[cfg(unix)] + ("O_DIRECTORY", libc::O_DIRECTORY as i64), + #[cfg(unix)] + ("O_FSYNC", libc::O_FSYNC as i64), + #[cfg(unix)] + ("O_NOCTTY", libc::O_NOCTTY as i64), + #[cfg(unix)] + ("O_NOFOLLOW", libc::O_NOFOLLOW as i64), + // Linux's own. O_LARGEFILE is 0 on the targets that are already 64-bit, + // which is the header answering that there is nothing to widen. + #[cfg(any(target_os = "linux", target_os = "android"))] + ("O_DIRECT", libc::O_DIRECT as i64), + #[cfg(any(target_os = "linux", target_os = "android"))] + ("O_LARGEFILE", libc::O_LARGEFILE as i64), + #[cfg(any(target_os = "linux", target_os = "android"))] + ("O_NOATIME", libc::O_NOATIME as i64), + #[cfg(any(target_os = "linux", target_os = "android"))] + ("O_PATH", libc::O_PATH as i64), + #[cfg(any(target_os = "linux", target_os = "android"))] + ("O_RSYNC", libc::O_RSYNC as i64), + #[cfg(any(target_os = "linux", target_os = "android"))] + ("O_TMPFILE", libc::O_TMPFILE as i64), + // The Apple targets' own. + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_EVTONLY", libc::O_EVTONLY as i64), + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_EXEC", libc::O_EXEC as i64), + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_EXLOCK", libc::O_EXLOCK as i64), + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_NOFOLLOW_ANY", libc::O_NOFOLLOW_ANY as i64), + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_SEARCH", libc::O_SEARCH as i64), + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_SHLOCK", libc::O_SHLOCK as i64), + #[cfg(any(target_os = "macos", target_os = "ios"))] + ("O_SYMLINK", libc::O_SYMLINK as i64), #[cfg(unix)] ("O_DSYNC", libc::O_DSYNC as i64), #[cfg(not(any(unix, windows)))] @@ -1325,8 +1484,10 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "setgroups", "setpgrp", "nice", - // "pipe2"/"dup3" — the flag-taking forms, which Linux adds and the - // other hosts do not have. Neither is served here on any of them. + // "pipe2" — the flag-taking form of `pipe`, published below on the + // hosts whose libc declares it. "dup3" is not a name `moduledef.py` + // defines, nor one `os` publishes on any host, so there is nothing + // here for it to stand in for. "fdatasync", "mkfifo", "getloadavg", @@ -1336,9 +1497,9 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "sched_get_priority_max", "sched_get_priority_min", // "sched_getparam"/"sched_setparam"/"sched_getscheduler"/ - // "sched_setscheduler" — the policy calls, which are Linux's and - // which hand a `sched_param` back and forth; there is no such type - // here. + // "sched_setscheduler" — the policy calls, published below together + // with the `sched_param` type they hand back and forth. + // `moduledef.py:168-174` gates the five as one group. "sched_yield", // "confstr"/"confstr_names" — the host's string-valued configuration // table, published below where the host defines one. A build with no @@ -1775,6 +1936,68 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } } + /// Bind an entry point whose positional parameters all sit before the + /// clinic `/`. None of them binds by name, so the count is over + /// positionals alone. + /// + /// `kwonly` decides which parser reports a bad count, and the two word it + /// differently. With no keyword-capable parameter at all the call is + /// parsed by `_PyArg_CheckPositional`, whose wording carries neither the + /// trailing `()` nor a parenthesised count, and every keyword is refused + /// against the module-qualified name. A keyword-only tail puts the call + /// back on `_PyArg_UnpackKeywords`, which reports the positional count in + /// the parenthesised form and accepts the named modifiers. + fn bind_posonly_args( + args: &[pyre_object::PyObjectRef], + name: &str, + qualname: &str, + total: usize, + required: usize, + kwonly: &[&'static str], + ) -> Result< + ( + Vec>, + Option, + ), + crate::PyError, + > { + let (pos, kwargs) = crate::builtins::split_builtin_kwargs(args); + if kwonly.is_empty() && crate::builtins::real_kwarg_count(kwargs) > 0 { + return Err(crate::PyError::type_error(format!( + "{qualname}() takes no keyword arguments" + ))); + } + // The count is checked before the keyword names: a call that supplies + // neither the positionals nor a recognised keyword is reported against + // the positionals. + if pos.len() < required || pos.len() > total { + let plural = if total == 1 { "" } else { "s" }; + let text = if !kwonly.is_empty() { + let limit = if required == total { "exactly" } else { "at most" }; + format!( + "{name}() takes {limit} {total} positional argument{plural} ({} given)", + pos.len() + ) + } else if required == total { + format!("{name} expected {total} argument{plural}, got {}", pos.len()) + } else { + let bound = if pos.len() > total { total } else { required }; + let plural = if bound == 1 { "" } else { "s" }; + let at = if pos.len() > total { "at most" } else { "at least" }; + format!( + "{name} expected {at} {bound} argument{plural}, got {}", + pos.len() + ) + }; + return Err(crate::PyError::type_error(text)); + } + crate::builtins::kwarg_reject_unknown(kwargs, kwonly, name)?; + Ok(( + (0..total).map(|index| pos.get(index).copied()).collect(), + kwargs, + )) + } + /// Bind the positional-or-keyword prefix of a path-taking entry point. /// `params` names that prefix in order, `path` first, and the leading /// `required` of them carry no default; the rest are reported absent as @@ -2232,11 +2455,18 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ns, "readlink", crate::make_builtin_function("readlink", |args| { - let arg = args - .first() - .copied() - .ok_or_else(|| crate::PyError::type_error("readlink() requires 1 argument"))?; - let path = crate::gateway::fsencode_path_w(arg)?; + let (bound, kwargs) = bind_path_args(args, "readlink", &["path"], 1, &["dir_fd"])?; + // `readlink` types `dir_fd` as `DirFD(rposix.HAVE_READLINKAT)`. + // This build resolves the name through `std::fs::read_link`, which + // has no at-variant, so a descriptor is refused rather than + // silently resolved against the working directory — matching what + // `os.supports_dir_fd` advertises. + let _dir_fd = dir_fd_kwarg(kwargs, false)?; + let path = crate::gateway::fsencode_path_named_w( + bound[0].expect("path is required"), + "readlink", + "path", + )?; let bytes_mode = unsafe { path.is_bytes() }; match std::fs::read_link(path_from_bytes(&path.as_bytes).as_ref()) { Ok(target) => { @@ -2377,8 +2607,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ))); } crate::builtins::kwarg_reject_unknown(kwargs, &["src_dir_fd", "dst_dir_fd"], name)?; - let src = crate::gateway::fsencode_path_w(pos[0])?; - let dst = crate::gateway::fsencode_path_w(pos[1])?; + // `rename` and `replace` are one body here and two argument-clinic + // declarations there, so the rejected argument is named after whichever + // of the two the caller reached. + let src = crate::gateway::fsencode_path_named_w(pos[0], name, "src")?; + let dst = crate::gateway::fsencode_path_named_w(pos[1], name, "dst")?; let dir_fd = |name: &str| -> Result, crate::PyError> { match crate::builtins::kwarg_get(kwargs, name) { // interp_posix.py:274-278 `_unwrap_dirfd` — a non-`None` value @@ -2448,10 +2681,15 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { /// `interp_posix.py:1901-1904` answers a descriptor with `futimens`, which /// is the call HAVE_FUTIMENS names. - fn utime_fd(fd: i32, access: UTime, modified: UTime) -> Result { + fn utime_fd( + fd: i32, + now: bool, + access: UTime, + modified: UTime, + ) -> Result { #[cfg(all(unix, not(feature = "sandbox")))] { - let times = [timespec_of(access), timespec_of(modified)]; + let times = [timespec_of(access, now), timespec_of(modified, now)]; if unsafe { libc::futimens(fd, times.as_ptr()) } < 0 { return Err(io_err(std::io::Error::last_os_error(), "")); } @@ -2459,18 +2697,25 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } #[allow(unreachable_code)] { - let _ = (fd, access, modified); + let _ = (fd, now, access, modified); Err(crate::PyError::not_implemented( "utime: fd is unavailable on this platform", )) } } + /// `do_utimens` (`interp_posix.py:1948-1953`) writes `UTIME_NOW` over both + /// nanosecond fields when the caller named no time, rather than reading a + /// time off its own clock and asking for that one. The two are different + /// requests: `UTIME_NOW` on both stamps is granted to anyone the file is + /// writable to, while naming a timestamp asks for ownership, so a writable + /// descriptor onto someone else's file answers `utime(fd)` and refuses + /// `utime(fd, ns=(now, now))` with EPERM. #[cfg(all(unix, not(feature = "sandbox")))] - fn timespec_of(t: UTime) -> libc::timespec { + fn timespec_of(t: UTime, now: bool) -> libc::timespec { libc::timespec { tv_sec: t.sec as libc::time_t, - tv_nsec: t.nsec as _, + tv_nsec: if now { libc::UTIME_NOW as _ } else { t.nsec as _ }, } } @@ -2644,7 +2889,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { }) }; - let (access, modified) = match (times, ns) { + // `parse_utime_args` (`interp_posix.py:1918-1946`) answers a "now" flag + // beside the pair and leaves the pair itself at zero when it is set; + // each of the calls below is what turns that flag into its own spelling + // of "now". + let (now, access, modified) = match (times, ns) { (Some(_), Some(_)) => { return Err(crate::PyError::value_error( "utime: you may specify either 'times' or 'ns' but not both", @@ -2652,22 +2901,13 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } (Some(t), None) => { let (a, m) = unpack_two(t, "times")?; - (time_from_secs(a)?, time_from_secs(m)?) + (false, time_from_secs(a)?, time_from_secs(m)?) } (None, Some(n)) => { let (a, m) = unpack_two(n, "ns")?; - (time_from_ns(a)?, time_from_ns(m)?) - } - (None, None) => { - let now = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or(std::time::Duration::ZERO); - let now = UTime { - sec: now.as_secs() as i64, - nsec: now.subsec_nanos() as i64, - }; - (now, now) + (false, time_from_ns(a)?, time_from_ns(m)?) } + (None, None) => (true, UTime { sec: 0, nsec: 0 }, UTime { sec: 0, nsec: 0 }), }; if path.as_fd != -1 { @@ -2686,7 +2926,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "utime: cannot use fd and follow_symlinks together", )); } - return utime_fd(path.as_fd, access, modified); + return utime_fd(path.as_fd, now, access, modified); } #[cfg(all(windows, feature = "host_env"))] @@ -2725,6 +2965,25 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { dwHighDateTime: (ticks >> 32) as u32, } }; + // `rposix.py:1568-1576` reads a clock here when the caller named no + // time — `GetSystemTime` into both stamps — and reaches + // `time_t_to_FILE_TIME` only for a named pair. `SetFileTime` has no + // word for "now", which is what the `utimensat` arm below spells + // `UTIME_NOW`; the pair arrives at zero while the flag carries the + // meaning, so reading it here is what keeps `os.utime(path)` off + // 1970. + let (access, modified) = if now { + let d = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or(std::time::Duration::ZERO); + let t = UTime { + sec: d.as_secs() as i64, + nsec: d.subsec_nanos() as i64, + }; + (t, t) + } else { + (access, modified) + }; let atime = to_filetime(access); let mtime = to_filetime(modified); let wide = wide_path(&path.as_bytes)?; @@ -2768,7 +3027,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } else { libc::AT_SYMLINK_NOFOLLOW }; - let times = [timespec_of(access), timespec_of(modified)]; + let times = [timespec_of(access, now), timespec_of(modified, now)]; let error = unsafe { libc::utimensat( dir_fd.unwrap_or(libc::AT_FDCWD), @@ -2787,7 +3046,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } #[allow(unreachable_code)] { - let _ = (access, modified, dir_fd, follow_symlinks, &path, pos); + let _ = (now, access, modified, dir_fd, follow_symlinks, &path, pos); Err(crate::PyError::not_implemented( "utime is unavailable on this platform", )) @@ -2814,6 +3073,9 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "_path_splitroot() missing required argument 'path'", )); }; + // Windows-only, so what it should name itself with is not + // measurable from a POSIX host; it keeps the caller-less + // conversion meanwhile. See the follow-up task. let path = extract_path(arg)?; // Splitting a drive or UNC prefix is a text operation on a // Windows path, and both halves are handed back as `str`, so @@ -2935,10 +3197,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ns, "listdir", crate::make_builtin_function("listdir", |args| { + let (bound, _kwargs) = bind_path_args(args, "listdir", &["path"], 0, &[])?; // One resolution yields both the path and its bytes-ness, so // `__fspath__` runs exactly once. The omitted argument is the same // `None` the signature names, which resolves to `"."` there. - let arg = args.first().copied().unwrap_or(pyre_object::w_none()); + let arg = bound[0].unwrap_or(pyre_object::w_none()); let resolved = crate::gateway::fsencode_path_or_fd_nullable_w( arg, "listdir", @@ -3068,8 +3331,19 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ns, "get_terminal_size", crate::make_builtin_function("get_terminal_size", |args| { - let fd = match args.first() { - Some(&w) => crate::baseobjspace::c_int_w(w)?, + // `($module, fd=, /)` — the descriptor is + // positional-only, so `fd=1` is a keyword this entry point does + // not take rather than a binding. + let (bound, _kwargs) = bind_posonly_args( + args, + "get_terminal_size", + "posix.get_terminal_size", + 1, + 0, + &[], + )?; + let fd = match bound[0] { + Some(w) => crate::baseobjspace::c_int_w(w)?, None => 1, }; #[cfg(unix)] @@ -3687,6 +3961,14 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { /// descriptor. `os.py:140-155` reads them into `supports_fd`, so a bit set /// where the call still rejects an integer hands the caller a capability it /// cannot use. + /// `rposix.HAVE_FACCESSAT` — what `access` types its `dir_fd` as + /// (`interp_posix.py:745`) and what its two flag modifiers are tested + /// against (`:771-775`). All three of `access`'s modifiers are the one + /// `faccessat` call, so the same bit carries them: `os.py:117,137,158` read + /// it into `supports_dir_fd`, `supports_effective_ids` and + /// `supports_follow_symlinks` alike, and it is the only bit any of those + /// three reads for `access`. + const HAVE_FACCESSAT: bool = HOST_POSIX; const HAVE_FCHDIR: bool = HOST_POSIX; const HAVE_FCHMOD: bool = HOST_POSIX; const HAVE_FCHOWN: bool = HOST_POSIX; @@ -3788,6 +4070,14 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::PyError::not_implemented("dir_fd unavailable on this platform") } + /// `argument_unavailable` (`interp_posix.py:298-301`) — a modifier this + /// platform has no call to apply, named together with the entry point that + /// was asked to apply it. + #[cfg(all(unix, feature = "host_env"))] + fn argument_unavailable(funcname: &str, arg: &str) -> crate::PyError { + crate::PyError::not_implemented(format!("{funcname}: {arg} unavailable on this platform")) + } + /// `os.link` takes its two names positionally and everything else by /// keyword, so a third positional argument is a `src_dir_fd` that would /// otherwise be dropped on the floor. @@ -4442,10 +4732,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { unsafe { pyre_object::w_list_append(list, obj) }; } fn scandir_fn(args: &[PyObjectRef]) -> Result { + let (bound, _kwargs) = bind_path_args(args, "scandir", &["path"], 0, &[])?; // One resolution yields both the path and its bytes-ness, so // `__fspath__` runs exactly once. The omitted argument is the same // `None` the signature names, which resolves to `"."` there. - let arg = args.first().copied().unwrap_or(pyre_object::w_none()); + let arg = bound[0].unwrap_or(pyre_object::w_none()); let resolved = crate::gateway::fsencode_path_or_fd_nullable_w(arg, "scandir", HAVE_FDOPENDIR)?; let bytes_mode = unsafe { resolved.is_bytes() }; @@ -4936,6 +5227,10 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { } let mut argv = Vec::with_capacity(items.len()); for item in items { + // An element is converted on the sequence's behalf, not as an + // argument of the call, so the caller-less message is the one + // it reports — measured, and the same for the environment + // below and for `posix_spawn`'s file actions. let value = extract_path(item)?; argv.push(std::ffi::CString::new(value).map_err(|_| { crate::PyError::value_error(format!( @@ -4965,7 +5260,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::make_builtin_function_with_arity( "execv", |args| { - let command = extract_path(args[0])?; + // The path names itself; the argv entries below do not, + // because each of those is converted on the sequence's + // behalf rather than as an argument of its own. + let command = + crate::gateway::fsencode_path_named_w(args[0], "execv", "path")?.as_bytes; let command_c = std::ffi::CString::new(command).map_err(|_| { crate::PyError::value_error("execv() path contains an embedded null byte") })?; @@ -4989,7 +5288,8 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::make_builtin_function_with_arity( "execve", |args| { - let command = extract_path(args[0])?; + let command = + crate::gateway::fsencode_path_named_w(args[0], "execve", "path")?.as_bytes; let command_c = std::ffi::CString::new(command).map_err(|_| { crate::PyError::value_error("execve() path contains an embedded null byte") })?; @@ -5092,6 +5392,45 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ), ); + // os.pipe2(flags) -> (r_fd, w_fd) + // + // `interp_posix.py:1188-1195`, which — unlike `pipe` two blocks up — + // forces no inheritance on the pair afterwards: the flags argument is + // the whole of the caller's control over it. + #[cfg(any( + target_os = "android", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd", + target_os = "openbsd" + ))] + crate::module_ns_store( + ns, + "pipe2", + crate::make_builtin_function_with_arity( + "pipe2", + |args| { + if args.is_empty() { + return Err(crate::PyError::type_error("pipe2() requires 1 argument")); + } + // interp_posix.py:1187 `@unwrap_spec(flags=c_int)`. + let flags = crate::baseobjspace::c_int_w(args[0])?; + match host_posix::pipe2(flags) { + Ok((rfd, wfd)) => { + use std::os::fd::IntoRawFd; + Ok(pyre_object::w_tuple_new(vec![ + pyre_object::w_int_new(rfd.into_raw_fd() as i64), + pyre_object::w_int_new(wfd.into_raw_fd() as i64), + ])) + } + Err(e) => Err(io_err(e, "")), + } + }, + 1, + ), + ); + // os.sched_yield() crate::module_ns_store( ns, @@ -5225,6 +5564,274 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ), ); + // The scheduling-policy group `moduledef.py:168-174` publishes as one — + // the two getters, the two setters and the `sched_param` type they + // exchange — plus `sched_rr_get_interval`, which `moduledef.py:166-167` + // gates on its own but which the same libcs carry. The setters are left + // out where the libc is musl, which declares neither. + #[cfg(any( + target_os = "android", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" + ))] + { + crate::module_ns_store(ns, "sched_param", sched_param_seq_type()); + + // os.sched_rr_get_interval(pid) -> seconds + // + // host_env wraps none of this one, so the call is made here — which + // is why it is absent from a sandbox build: `host_seam::sys` + // re-exports no syscall function, and the name is served there by + // the raising stub registered at the end of this module instead. + #[cfg(not(feature = "sandbox"))] + crate::module_ns_store( + ns, + "sched_rr_get_interval", + crate::make_builtin_function_with_arity( + "sched_rr_get_interval", + |args| { + if args.is_empty() { + return Err(crate::PyError::type_error( + "sched_rr_get_interval() requires 1 argument", + )); + } + // interp_posix.py:3061 `@unwrap_spec(pid=int)`; the + // timespec the call fills is answered as one float + // (`rposix.py:2525`). + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let mut interval: libc::timespec = + unsafe { core::mem::zeroed::() }; + if unsafe { libc::sched_rr_get_interval(pid, &mut interval) } == -1 { + return Err(io_err(std::io::Error::last_os_error(), "")); + } + Ok(pyre_object::w_float_new( + interval.tv_sec as f64 + 1e-9 * interval.tv_nsec as f64, + )) + }, + 1, + ), + ); + + // os.sched_getscheduler(pid) -> policy + crate::module_ns_store( + ns, + "sched_getscheduler", + crate::make_builtin_function_with_arity( + "sched_getscheduler", + |args| { + if args.is_empty() { + return Err(crate::PyError::type_error( + "sched_getscheduler() requires 1 argument", + )); + } + // interp_posix.py:3073 `@unwrap_spec(pid=int)`. + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let policy = + host_posix::sched_getscheduler(pid).map_err(|e| io_err(e, ""))?; + Ok(pyre_object::w_int_new(policy as i64)) + }, + 1, + ), + ); + + // os.sched_getparam(pid) -> sched_param + crate::module_ns_store( + ns, + "sched_getparam", + crate::make_builtin_function_with_arity( + "sched_getparam", + |args| { + if args.is_empty() { + return Err(crate::PyError::type_error( + "sched_getparam() requires 1 argument", + )); + } + // interp_posix.py:3103 `@unwrap_spec(pid=int)`; the + // priority the call fills in is handed back wrapped in + // the type, not bare (`interp_posix.py:3113`). + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let param = host_posix::sched_getparam(pid).map_err(|e| io_err(e, ""))?; + Ok(crate::_structseq::new_instance( + sched_param_seq_type(), + vec![pyre_object::w_int_new(param.sched_priority as i64)], + )) + }, + 1, + ), + ); + + // Both setters answer None. `interp_posix.py:3097`/`:3131` hand + // back the raw `handle_posix_error` result instead, which is 0 on + // every success and which `os.sched_setparam` does not publish. + #[cfg(not(target_env = "musl"))] + { + // os.sched_setscheduler(pid, policy, param) + crate::module_ns_store( + ns, + "sched_setscheduler", + crate::make_builtin_function_with_arity( + "sched_setscheduler", + |args| { + if args.len() < 3 { + return Err(crate::PyError::type_error( + "sched_setscheduler() requires 3 arguments", + )); + } + // interp_posix.py:3085 `@unwrap_spec(pid=int, policy=int)`. + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let policy = crate::baseobjspace::int_w(args[1])? as libc::c_int; + let priority = sched_priority_w(args[2])?; + let mut param: libc::sched_param = + unsafe { core::mem::zeroed::() }; + param.sched_priority = priority; + host_posix::sched_setscheduler(pid, policy, ¶m) + .map_err(|e| io_err(e, ""))?; + Ok(pyre_object::w_none()) + }, + 3, + ), + ); + + // os.sched_setparam(pid, param) + crate::module_ns_store( + ns, + "sched_setparam", + crate::make_builtin_function_with_arity( + "sched_setparam", + |args| { + if args.len() < 2 { + return Err(crate::PyError::type_error( + "sched_setparam() requires 2 arguments", + )); + } + // interp_posix.py:3117 `@unwrap_spec(pid=int)`. + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let priority = sched_priority_w(args[1])?; + let mut param: libc::sched_param = + unsafe { core::mem::zeroed::() }; + param.sched_priority = priority; + host_posix::sched_setparam(pid, ¶m).map_err(|e| io_err(e, ""))?; + Ok(pyre_object::w_none()) + }, + 2, + ), + ); + } + } + + // os.sched_getaffinity(pid) / os.sched_setaffinity(pid, mask) — the CPU + // mask, which `moduledef.py` does not publish and `rposix` does not + // wrap, so both are written against the host header rather than ported. + // + // The mask is the fixed `cpu_set_t`, `CPU_SETSIZE` CPUs wide: the libc + // crate exposes no `CPU_ALLOC`. A CPU number at or past that width is + // refused with the EINVAL the kernel would answer for it, and a host + // with more CPUs than that gets the kernel's own EINVAL out of + // `sched_getaffinity` rather than a silently truncated mask. + // + // Both name libc directly, so neither is compiled into a sandbox build; + // `host_seam::sys` re-exports no syscall and the stubs at the end of + // this module serve the names there. + #[cfg(all( + not(feature = "sandbox"), + any(target_os = "linux", target_os = "android") + ))] + { + crate::module_ns_store( + ns, + "sched_getaffinity", + crate::make_builtin_function_with_arity( + "sched_getaffinity", + |args| { + if args.is_empty() { + return Err(crate::PyError::type_error( + "sched_getaffinity() requires 1 argument", + )); + } + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let mut mask: libc::cpu_set_t = + unsafe { core::mem::zeroed::() }; + unsafe { libc::CPU_ZERO(&mut mask) }; + let res = unsafe { + libc::sched_getaffinity( + pid, + core::mem::size_of::(), + &mut mask, + ) + }; + if res == -1 { + return Err(io_err(std::io::Error::last_os_error(), "")); + } + let items: Vec<_> = (0..libc::CPU_SETSIZE as usize) + .filter(|&cpu| unsafe { libc::CPU_ISSET(cpu, &mask) }) + .map(|cpu| pyre_object::w_int_new(cpu as i64)) + .collect(); + Ok(pyre_object::w_set_from_items(&items)) + }, + 1, + ), + ); + + crate::module_ns_store( + ns, + "sched_setaffinity", + crate::make_builtin_function_with_arity( + "sched_setaffinity", + |args| { + if args.len() < 2 { + return Err(crate::PyError::type_error( + "sched_setaffinity() requires 2 arguments", + )); + } + let pid = crate::baseobjspace::c_int_w(args[0])? as libc::pid_t; + let items = crate::builtins::collect_iterable(args[1])?; + let int_type = + crate::typedef::gettypeobject(&pyre_object::pyobject::INT_TYPE); + let mut mask: libc::cpu_set_t = + unsafe { core::mem::zeroed::() }; + unsafe { libc::CPU_ZERO(&mut mask) }; + for item in items { + if !crate::baseobjspace::isinstance(item, int_type)? { + return Err(crate::PyError::type_error(format!( + "expected an iterator of ints, but iterator yielded ", + crate::type_methods::arg_type_name(item) + ))); + } + let cpu = crate::baseobjspace::int_w(item)?; + if cpu < 0 { + return Err(crate::PyError::value_error("negative CPU number")); + } + if cpu > libc::c_int::MAX as i64 { + return Err(crate::PyError::overflow_error( + "CPU number too large", + )); + } + if cpu >= libc::CPU_SETSIZE as i64 { + return Err(io_err( + std::io::Error::from_raw_os_error(libc::EINVAL), + "", + )); + } + unsafe { libc::CPU_SET(cpu as usize, &mut mask) }; + } + let res = unsafe { + libc::sched_setaffinity( + pid, + core::mem::size_of::(), + &mask, + ) + }; + if res == -1 { + return Err(io_err(std::io::Error::last_os_error(), "")); + } + Ok(pyre_object::w_none()) + }, + 2, + ), + ); + } + // os.sync() #[cfg(not(any(target_os = "redox", target_os = "android")))] crate::module_ns_store( @@ -5721,53 +6328,73 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ); // os.dup2(fd, fd2, inheritable=True) -> fd2 + // + // Carries a `Signature`, so `inheritable` binds by name. Registered + // raw it did not: the trailing `__pyre_kw__` marker dict was never + // split off the argument slice, so it landed in the third positional + // slot and read truthy, and `dup2(fd, fd2, inheritable=False)` + // returned an *inheritable* descriptor that an exec would carry. + // + // The arguments stay `PyObjectRef` and are unwrapped in the body: the + // macro's bare `i32` binding is a raw `w_int_get_value` cast, which + // would read a non-int argument's payload instead of reporting it. #[cfg(not(feature = "sandbox"))] - crate::module_ns_store( - ns, - "dup2", - crate::make_builtin_function("dup2", |args| { - if args.len() < 2 { - return Err(crate::PyError::type_error("dup2() requires 2 arguments")); + #[crate::pyre_function] + fn dup2( + fd: pyre_object::PyObjectRef, + fd2: pyre_object::PyObjectRef, + inheritable: Option, + ) -> Result { + // interp_posix.py:733 `@unwrap_spec(fd=c_int, fd2=c_int, inheritable=bool)`. + let fd = crate::baseobjspace::c_int_w(fd)?; + let fd2 = crate::baseobjspace::c_int_w(fd2)?; + let inheritable = match inheritable { + Some(w) => crate::baseobjspace::is_true(w)?, + None => true, + }; + // `os_dup2_impl` asks for a non-inheritable duplicate through + // `dup3` where it exists, so no window opens in which the new + // descriptor is inheritable and an exec could carry it. The + // inheritable case keeps plain `dup2`, which is also the one + // that tolerates `fd == fd2`. + let n = if inheritable { + crate::builtins::crt_call!(libc::dup2(fd, fd2)) + } else { + #[cfg(any(target_os = "android", target_os = "linux", target_os = "freebsd"))] + { + crate::builtins::crt_call!(libc::dup3(fd, fd2, libc::O_CLOEXEC)) } - // interp_posix.py:733 `@unwrap_spec(fd=c_int, fd2=c_int, inheritable=bool)`. - let fd = crate::baseobjspace::c_int_w(args[0])?; - let fd2 = crate::baseobjspace::c_int_w(args[1])?; - let inheritable = match args.get(2) { - Some(&w) => crate::baseobjspace::is_true(w)?, - None => true, - }; - // `os_dup2_impl` asks for a non-inheritable duplicate through - // `dup3` where it exists, so no window opens in which the new - // descriptor is inheritable and an exec could carry it. The - // inheritable case keeps plain `dup2`, which is also the one - // that tolerates `fd == fd2`. - let n = if inheritable { - crate::builtins::crt_call!(libc::dup2(fd, fd2)) - } else { - #[cfg(any(target_os = "android", target_os = "linux", target_os = "freebsd"))] - { - crate::builtins::crt_call!(libc::dup3(fd, fd2, libc::O_CLOEXEC)) - } - #[cfg(not(any( - target_os = "android", - target_os = "linux", - target_os = "freebsd" - )))] - { - let n = crate::builtins::crt_call!(libc::dup2(fd, fd2)); - if n >= 0 { - use std::os::fd::BorrowedFd; - let bfd = unsafe { BorrowedFd::borrow_raw(n) }; - host_posix::set_inheritable(bfd, false).map_err(|e| io_err(e, ""))?; - } - n + #[cfg(not(any( + target_os = "android", + target_os = "linux", + target_os = "freebsd" + )))] + { + let n = crate::builtins::crt_call!(libc::dup2(fd, fd2)); + if n >= 0 { + use std::os::fd::BorrowedFd; + let bfd = unsafe { BorrowedFd::borrow_raw(n) }; + host_posix::set_inheritable(bfd, false).map_err(|e| io_err(e, ""))?; } - }; - if n < 0 { - return Err(errno_err(crate::builtins::crt_errno(), "")); + n } - Ok(pyre_object::w_int_new(n as i64)) - }), + }; + if n < 0 { + return Err(errno_err(crate::builtins::crt_errno(), "")); + } + Ok(pyre_object::w_int_new(n as i64)) + } + + #[cfg(not(feature = "sandbox"))] + crate::module_ns_store( + ns, + "dup2", + crate::make_builtin_function_with_arity_and_maybe_sig( + "dup2", + dup2, + dup2_pyre_arity(), + dup2_pyre_sig(), + ), ); // os.fsync(fd) @@ -5833,20 +6460,29 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ); // interp_posix.py:407-412: retry EINTR, propagate every other OSError. + // The retry is `eintr_retry=True`, which runs the pending Python signal + // handlers before going back to the call — a handler that raises ends + // the loop there, and one that disarms the timer stops the interruption + // recurring. Retrying on the bare errno would spin without ever giving + // that handler a turn. + // // Which filename the caller then reports is its own: `os.ftruncate` was // given no name to report, while `os.truncate` names the one it opened. - // The call runs through the call gate so a signal handler gets its turn - // between the retries. #[cfg(all(unix, not(feature = "sandbox")))] - fn ftruncate_retry(fd: libc::c_int, length: libc::off_t) -> Result<(), i32> { + fn ftruncate_retry( + fd: libc::c_int, + length: libc::off_t, + wrap: impl Fn(i32) -> crate::PyError, + ) -> Result<(), crate::PyError> { loop { if crate::builtins::crt_call!(libc::ftruncate(fd, length)) == 0 { return Ok(()); } let errno = crate::builtins::crt_errno(); - if errno != libc::EINTR { - return Err(errno); - } + crate::builtins::eintr_retry_with( + std::io::Error::from_raw_os_error(errno), + |e| wrap(e.raw_os_error().unwrap_or(0)), + )?; } } @@ -5877,7 +6513,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { )?; let length = truncate_length_w(args[1])?; if path.as_fd != -1 { - ftruncate_retry(path.as_fd, length).map_err(|e| errno_err(e, ""))?; + ftruncate_retry(path.as_fd, length, |e| errno_err(e, ""))?; return Ok(pyre_object::w_none()); } let c_path = std::ffi::CString::new(path.as_bytes.as_slice()) @@ -5901,7 +6537,8 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { |e| errno_err_with_filename(e.raw_os_error().unwrap_or(0), path.w_path()), )?; }; - let truncated = ftruncate_retry(fd, length); + let truncated = + ftruncate_retry(fd, length, |e| errno_err_with_filename(e, path.w_path())); // `interp_posix.py:429-431` closes the descriptor it opened // in a `finally`, through the module's own `close` — so a // writeback error the close is the first to see is the @@ -5910,7 +6547,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // both fail, which is the order the `finally` gives them. let closed = unsafe { libc::close(fd) }; let close_errno = (closed < 0).then(crate::builtins::crt_errno); - truncated.map_err(|e| errno_err_with_filename(e, path.w_path()))?; + truncated?; if let Some(errno) = close_errno { return Err(errno_err_with_filename(errno, path.w_path())); } @@ -5955,7 +6592,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::PyError::overflow_error("Python int too large to convert to C int") })?; let length = truncate_length_w(args[1])?; - ftruncate_retry(fd, length).map_err(|e| errno_err(e, ""))?; + ftruncate_retry(fd, length, |e| errno_err(e, ""))?; Ok(pyre_object::w_none()) }, 2, @@ -6307,35 +6944,40 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ), ); - // os.cpu_count() -> int | None + // os.cpu_count() -> int | None — `interp_posix.py:2910-2914`, which + // answers None for a count of 0 or less. Both names read the processor + // count through `host_cpu_count`; the syscalls it makes are the reason + // the pair is left to the sandbox stubs below. + #[cfg(not(feature = "sandbox"))] crate::module_ns_store( ns, "cpu_count", crate::make_builtin_function_with_arity( "cpu_count", |_| { - let n = host_posix::get_number_of_os_threads(); + let n = host_cpu_count(); if n <= 0 { Ok(pyre_object::w_none()) } else { - Ok(pyre_object::w_int_new(n as i64)) + Ok(pyre_object::w_int_new(n)) } }, 0, ), ); // _cpu_count alias — newer CPython exposes both. + #[cfg(not(feature = "sandbox"))] crate::module_ns_store( ns, "_cpu_count", crate::make_builtin_function_with_arity( "_cpu_count", |_| { - let n = host_posix::get_number_of_os_threads(); + let n = host_cpu_count(); if n <= 0 { Ok(pyre_object::w_none()) } else { - Ok(pyre_object::w_int_new(n as i64)) + Ok(pyre_object::w_int_new(n)) } }, 0, @@ -6348,11 +6990,35 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ns, "symlink", crate::make_builtin_function("symlink", |args| { - if args.len() < 2 { - return Err(crate::PyError::type_error("symlink() requires 2 arguments")); - } - let src = crate::gateway::fsencode_path_w(args[0])?; - let dst = crate::gateway::fsencode_path_w(args[1])?; + let (bound, kwargs) = bind_path_args( + args, + "symlink", + &["src", "dst", "target_is_directory"], + 2, + &["dir_fd"], + )?; + // `target_is_directory` selects between the two Windows link + // kinds and is ignored everywhere else (`os_symlink_impl`). + // Bound rather than dropped so a fourth positional is the + // `dir_fd` error it is, not a silently created link. + let _target_is_directory = match bound[2] { + Some(value) => crate::baseobjspace::is_true(value)?, + None => false, + }; + // `symlink` types `dir_fd` as `DirFD(rposix.HAVE_SYMLINKAT)`; + // the body below calls `libc::symlink`, which has no + // descriptor arm. + let _dir_fd = dir_fd_kwarg(kwargs, false)?; + let src = crate::gateway::fsencode_path_named_w( + bound[0].expect("src is required"), + "symlink", + "src", + )?; + let dst = crate::gateway::fsencode_path_named_w( + bound[1].expect("dst is required"), + "symlink", + "dst", + )?; let c_src = std::ffi::CString::new(src.as_bytes.as_slice()) .map_err(|_| crate::PyError::value_error("embedded null in src"))?; let c_dst = std::ffi::CString::new(dst.as_bytes.as_slice()) @@ -6384,8 +7050,8 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "link", )?; link_positional(args)?; - let src = crate::gateway::fsencode_path_w(args[0])?; - let dst = crate::gateway::fsencode_path_w(args[1])?; + let src = crate::gateway::fsencode_path_named_w(args[0], "link", "src")?; + let dst = crate::gateway::fsencode_path_named_w(args[1], "link", "dst")?; let c_src = std::ffi::CString::new(src.as_bytes.as_slice()) .map_err(|_| crate::PyError::value_error("embedded null in src"))?; let c_dst = std::ffi::CString::new(dst.as_bytes.as_slice()) @@ -6542,9 +7208,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { if !follow_symlinks { let errno = crate::builtins::io_error_posix_errno(&err, 0); if errno == libc::ENOTSUP || errno == libc::EOPNOTSUPP { - return Err(crate::PyError::not_implemented(format!( - "{name}: follow_symlinks unavailable on this platform" - ))); + return Err(argument_unavailable(name, "follow_symlinks")); } } return Err(io_err_with_filename(err, path.w_path())); @@ -6823,36 +7487,100 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ), ); - // os.access(path, mode) -> bool + // os.access(path, mode, *, dir_fd=None, effective_ids=False, + // follow_symlinks=True) -> bool crate::module_ns_store( ns, "access", crate::make_builtin_function("access", |args| { - if args.len() < 2 { - return Err(crate::PyError::type_error("access() requires 2 arguments")); - } - let path = extract_path(args[0])?; + // `access` names three keyword-only modifiers, so a third + // positional is an error rather than a `dir_fd`. + let (bound, kwargs) = bind_path_args( + args, + "access", + &["path", "mode"], + 2, + &["dir_fd", "effective_ids", "follow_symlinks"], + )?; + // The parameters convert in declaration order, and every one of + // them can raise, so the order is observable: `path` reports + // before `mode`, `mode` before `dir_fd`, and both before either + // flag's `__bool__` is called at all. + let path = crate::gateway::fsencode_path_named_w( + bound[0].expect("path is required"), + "access", + "path", + )? + .as_bytes; // interp_posix.py:744 `@unwrap_spec(mode=c_int, ...)`. - let mode = crate::baseobjspace::c_int_w(args[1])?; + let mode = crate::baseobjspace::c_int_w(bound[1].expect("mode is required"))?; + // interp_posix.py:745 types `dir_fd` as + // `DirFD(rposix.HAVE_FACCESSAT)`, so a host with no `faccessat` + // turns the descriptor away instead of resolving the name + // against the working directory as though none had been given. + let dir_fd = dir_fd_kwarg(kwargs, HAVE_FACCESSAT)?; + let effective_ids = match crate::builtins::kwarg_get(kwargs, "effective_ids") { + Some(v) => crate::baseobjspace::is_true(v)?, + None => false, + }; + let follow_symlinks = match crate::builtins::kwarg_get(kwargs, "follow_symlinks") { + Some(v) => crate::baseobjspace::is_true(v)?, + None => true, + }; + // interp_posix.py:771-775 — the two flag modifiers have no other + // call to reach, so without `faccessat` they are refused rather + // than answered as though they had been applied. + if !HAVE_FACCESSAT { + if !follow_symlinks { + return Err(argument_unavailable("access", "follow_symlinks")); + } + if effective_ids { + return Err(argument_unavailable("access", "effective_ids")); + } + } #[cfg(feature = "sandbox")] { + // `HAVE_FACCESSAT` is false here, so the three modifiers + // have already been turned away and only the plain form is + // left to serve. + let _ = dir_fd; return Ok(pyre_object::w_bool_from( crate::host_seam::ops::access(&path, mode).unwrap_or(false), )); } #[cfg(not(feature = "sandbox"))] { - // `check_access` takes the mask as a `u8` and rejects any - // bit outside `R_OK | W_OK | X_OK`. Narrowing before that - // check would fold a mode like 256 onto `F_OK` and answer - // "exists" for a mode `access(2)` rejects with EINVAL. - let Ok(mode) = u8::try_from(mode) else { - return Ok(pyre_object::w_bool_from(false)); + let c_path = std::ffi::CString::new(path.as_slice()) + .map_err(|_| crate::PyError::value_error("embedded null character"))?; + // interp_posix.py:778-786 keeps the plain `access` for the + // unmodified call and reaches for `faccessat` only where the + // name resolves against a descriptor, the final symlink must + // not be followed, or the effective ids are the ones to ask + // about. `rposix.py:2551-2560` is the flag mapping. + let ret = if dir_fd.is_some() || !follow_symlinks || effective_ids { + let mut flags = 0; + if !follow_symlinks { + flags |= libc::AT_SYMLINK_NOFOLLOW; + } + if effective_ids { + flags |= libc::AT_EACCESS; + } + unsafe { + libc::faccessat( + dir_fd.unwrap_or(libc::AT_FDCWD), + c_path.as_ptr(), + mode, + flags, + ) + } + } else { + unsafe { libc::access(c_path.as_ptr(), mode) } }; - match host_posix::check_access(path_from_bytes(&path).as_ref(), mode) { - Ok(ok) => Ok(pyre_object::w_bool_from(ok)), - Err(_) => Ok(pyre_object::w_bool_from(false)), - } + // `rposix.access` and `rposix.faccessat` both answer + // `error == 0` without `handle_posix_error`, so a refused + // call is False and not an `OSError` — including the EINVAL + // a mode outside `R_OK | W_OK | X_OK` can draw. + return Ok(pyre_object::w_bool_from(ret == 0)); } }), ); @@ -6867,7 +7595,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { if args.is_empty() { return Err(crate::PyError::type_error("chroot() requires 1 argument")); } - let path = crate::gateway::fsencode_path_w(args[0])?; + let path = crate::gateway::fsencode_path_named_w(args[0], "chroot", "path")?; host_posix::chroot(path_from_bytes(&path.as_bytes).as_ref()) .map_err(|e| io_err_with_filename(e, path.w_path()))?; Ok(pyre_object::w_none()) @@ -6987,10 +7715,19 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // wrapper (macos). // * Returns bytes-sent as int (PyPy: space.newint(res)). // - // EINTR retry loop intentionally omitted — pyre's other os-syscall - // wrappers don't do manual retry (relies on PEP 475 OS-level retry), - // matching pyre-wide convention rather than introducing a single - // outlier. + // Both arms of `interp_posix.py:2958-2974` sit in a + // `while True: ... except OSError: wrap_oserror(..., eintr_retry=True)`, + // so an interrupted transfer runs the pending Python signal handlers and + // then goes back to the call. The three below do the same through + // `builtins::eintr_retry_with`. + // + // The BSD arm discards a partial `sbytes` on EINTR rather than reporting + // it: `rposix.py:3086-3095` rescues a partial transfer for `EAGAIN` and + // `EBUSY` alone, and EINTR falls through to `handle_posix_error`, which + // raises. The loop then re-runs the whole call with the same `offset` and + // `count` — both are loop-invariant in `interp_posix.py`, and `rposix` + // never sees the retry — so the transfer restarts from the range the + // caller asked for, not from where it had got to. #[cfg(all( any(target_os = "linux", target_os = "macos"), not(feature = "sandbox") @@ -7000,19 +7737,28 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "sendfile", crate::make_builtin_function("sendfile", |args| { use std::os::fd::BorrowedFd; - if args.len() < 4 { - return Err(crate::PyError::type_error( - "sendfile() requires 4 arguments", - )); - } + // Every parameter is positional-or-keyword. `headers`, + // `trailers` and `flags` are the BSD `sendfile(2)` tail, which + // neither arm below passes on; they are named here so a + // caller that supplies them is bound rather than truncated, + // and so an unknown keyword is an error. + let (bound, _kwargs) = bind_path_args( + args, + "sendfile", + &[ + "out_fd", "in_fd", "offset", "count", "headers", "trailers", "flags", + ], + 4, + &[], + )?; // interp_posix.py:2946 `@unwrap_spec(out_fd=c_int, count=int)`, // with `in_ = space.c_int_w(w_in_fd)` in the body (:2955). The // spec runs in the gateway, so the count is converted before // the descriptor argument that follows it here. - let out_fd = crate::baseobjspace::c_int_w(args[0])?; - let count_raw = crate::baseobjspace::int_w(args[3])?; - let in_fd = crate::baseobjspace::c_int_w(args[1])?; - let w_offset = args[2]; + let out_fd = crate::baseobjspace::c_int_w(bound[0].expect("out_fd is required"))?; + let count_raw = crate::baseobjspace::int_w(bound[3].expect("count is required"))?; + let in_fd = crate::baseobjspace::c_int_w(bound[1].expect("in_fd is required"))?; + let w_offset = bound[2].expect("offset is required"); if unsafe { pyre_object::is_none(w_offset) } { // linux-only no-offset path; non-linux raises TypeError // matching interp_posix.py:2946. @@ -7029,14 +7775,19 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // libc::sendfile directly with a null pointer, matching // rposix.sendfile_no_offset (rposix.py:3066-3069). let count = count_raw as libc::size_t; - let (res, errno) = - crate::module::thread::call_external_function(|| unsafe { - libc::sendfile(out_fd, in_fd, core::ptr::null_mut(), count) - }); - if res < 0 { - return Err(io_err(std::io::Error::from_raw_os_error(errno), "")); + loop { + let (res, errno) = + crate::module::thread::call_external_function(|| unsafe { + libc::sendfile(out_fd, in_fd, core::ptr::null_mut(), count) + }); + if res >= 0 { + return Ok(pyre_object::w_int_new(res as i64)); + } + crate::builtins::eintr_retry_with( + std::io::Error::from_raw_os_error(errno), + |e| io_err(e, ""), + )?; } - return Ok(pyre_object::w_int_new(res as i64)); } } // interp_posix.py:2968 `space.gateway_r_longlong_w(w_offset)`. @@ -7046,42 +7797,61 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { #[cfg(target_os = "linux")] { let count = count_raw as usize; - let mut offset: rustpython_host_env::crt_fd::Offset = offset_i64 as _; - let n = { - let _blocked = crate::module::thread::before_external_block(); - host_posix::sendfile(out_b, in_b, &mut offset, count) + loop { + // Seeded from the caller's value on every attempt. + // `rposix.sendfile` (`rposix.py:3061-3065`) writes the + // offset into a fresh cell each call from the argument it + // was passed, and the retry sits above it holding that + // argument unchanged, so what a failed call left behind + // here is not what the next one starts from. + let mut offset: rustpython_host_env::crt_fd::Offset = offset_i64 as _; + let result = { + let _blocked = crate::module::thread::before_external_block(); + host_posix::sendfile(out_b, in_b, &mut offset, count) + }; + match result { + Ok(n) => return Ok(pyre_object::w_int_new(n as i64)), + Err(e) => crate::builtins::eintr_retry_with(e, |e| io_err(e, ""))?, + } } - .map_err(|e| io_err(e, ""))?; - return Ok(pyre_object::w_int_new(n as i64)); } #[cfg(target_os = "macos")] { - let (res, written) = { - let _blocked = crate::module::thread::before_external_block(); - host_posix::sendfile( - in_b, - out_b, - offset_i64 as rustpython_host_env::crt_fd::Offset, - count_raw, - None, - None, - ) - }; - // rposix.py:3086-3095: BSD sendfile reports a partial - // transfer through sbytes even when the syscall result is - // EAGAIN/EBUSY. Return that progress so asyncio advances - // its file offset instead of resending the same range. - if let Err(error) = res { - if written == 0 - || !matches!( - error.raw_os_error(), - Some(libc::EAGAIN) | Some(libc::EBUSY) + loop { + let (res, written) = { + let _blocked = crate::module::thread::before_external_block(); + host_posix::sendfile( + in_b, + out_b, + offset_i64 as rustpython_host_env::crt_fd::Offset, + count_raw, + None, + None, ) - { - return Err(io_err(error, "")); + }; + match res { + Ok(_) => return Ok(pyre_object::w_int_new(written)), + Err(error) => { + // rposix.py:3086-3095: BSD sendfile reports a + // partial transfer through sbytes even when the + // syscall result is EAGAIN/EBUSY. Return that + // progress so asyncio advances its file offset + // instead of resending the same range. EINTR is + // not in that set, so a partial transfer a signal + // interrupted goes to the retry below, which asks + // for the caller's original range again. + if written != 0 + && matches!( + error.raw_os_error(), + Some(libc::EAGAIN) | Some(libc::EBUSY) + ) + { + return Ok(pyre_object::w_int_new(written)); + } + crate::builtins::eintr_retry_with(error, |e| io_err(e, ""))?; + } } } - return Ok(pyre_object::w_int_new(written)); } }), ); @@ -7099,13 +7869,36 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { args: &[pyre_object::PyObjectRef], spawnp: bool, ) -> Result { - let (positional, kwargs) = crate::builtins::split_builtin_kwargs(args); - if positional.len() < 3 { - return Err(crate::PyError::type_error( - "posix_spawn() requires path, argv, env", - )); - } - let path = crate::gateway::fsencode_path_w(positional[0])?; + // The two entry points share this body and have an argument + // clinic declaration each, so the one the caller reached is the + // name its rejected path reports. + let func = if spawnp { "posix_spawnp" } else { "posix_spawn" }; + // `(path, argv, env, /, *, file_actions=(), ...)` — the three + // names are positional-only and everything else is + // keyword-only, so there is no positional-or-keyword slot at + // all. + let (bound, kwargs) = bind_posonly_args( + args, + func, + func, + 3, + 3, + &[ + "file_actions", + "setpgroup", + "resetids", + "setsid", + "setsigmask", + "setsigdef", + "scheduler", + ], + )?; + let positional = [ + bound[0].expect("path is required"), + bound[1].expect("argv is required"), + bound[2].expect("env is required"), + ]; + let path = crate::gateway::fsencode_path_named_w(positional[0], func, "path")?; let c_path = std::ffi::CString::new(path.as_bytes.as_slice()).map_err(|_| { crate::PyError::value_error("posix_spawn: embedded null in path") })?; @@ -8069,25 +8862,36 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ), ); - // os.dup2(fd, fd2, inheritable=True) -> fd2 + // os.dup2(fd, fd2, inheritable=True) -> fd2 — the `Signature`-bearing + // twin of the unix registration, and defective in the same way while + // it was registered raw. + #[crate::pyre_function] + fn dup2( + fd: pyre_object::PyObjectRef, + fd2: pyre_object::PyObjectRef, + inheritable: Option, + ) -> Result { + let fd = crate::baseobjspace::c_int_w(fd)?; + let fd2 = crate::baseobjspace::c_int_w(fd2)?; + let inheritable = match inheritable { + Some(w) => crate::baseobjspace::is_true(w)?, + None => true, + }; + match host_nt::dup2(fd, fd2, inheritable) { + Ok(n) => Ok(pyre_object::w_int_new(n as i64)), + Err(e) => Err(errno_err(crt_errno_of(&e), "")), + } + } + crate::module_ns_store( ns, "dup2", - crate::make_builtin_function("dup2", |args| { - if args.len() < 2 { - return Err(crate::PyError::type_error("dup2() requires 2 arguments")); - } - let fd = crate::baseobjspace::c_int_w(args[0])?; - let fd2 = crate::baseobjspace::c_int_w(args[1])?; - let inheritable = match args.get(2) { - Some(&w) => crate::baseobjspace::is_true(w)?, - None => true, - }; - match host_nt::dup2(fd, fd2, inheritable) { - Ok(n) => Ok(pyre_object::w_int_new(n as i64)), - Err(e) => Err(errno_err(crt_errno_of(&e), "")), - } - }), + crate::make_builtin_function_with_arity_and_maybe_sig( + "dup2", + dup2, + dup2_pyre_arity(), + dup2_pyre_sig(), + ), ); // os.fsync(fd) — `_commit`, the runtime's flush-to-disk. @@ -8170,7 +8974,10 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { if args.is_empty() { return Err(crate::PyError::type_error("chdir() requires 1 argument")); } - let path = crate::gateway::fsencode_path_w(args[0])?; + // The POSIX `chdir` names `integer` in its allowed types + // because it can `fchdir`; there is none here, so the list + // this one shows is the path-only one. + let path = crate::gateway::fsencode_path_named_w(args[0], "chdir", "path")?; std::env::set_current_dir(path_from_bytes(&path.as_bytes).as_ref()) .map_err(|e| fs_err_with_filename(e, path.w_path()))?; Ok(pyre_object::w_none()) @@ -8189,7 +8996,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { if args.len() < 2 { return Err(crate::PyError::type_error("access() requires 2 arguments")); } - let path = crate::gateway::fsencode_path_w(args[0])?; + let path = crate::gateway::fsencode_path_named_w(args[0], "access", "path")?; // Only `W_OK` is read, so the byte holding it is the whole of // the mode as far as the answer goes. let mode = crate::baseobjspace::c_int_w(args[1])? as u8; @@ -8332,8 +9139,8 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { )); } link_positional(args)?; - let src = crate::gateway::fsencode_path_w(args[0])?; - let dst = crate::gateway::fsencode_path_w(args[1])?; + let src = crate::gateway::fsencode_path_named_w(args[0], "link", "src")?; + let dst = crate::gateway::fsencode_path_named_w(args[1], "link", "dst")?; let (wide_src, wide_dst) = (wide_path(&src.as_bytes)?, wide_path(&dst.as_bytes)?); let ok = unsafe { @@ -8382,8 +9189,8 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { if args.len() < 2 { return Err(crate::PyError::type_error("symlink() requires 2 arguments")); } - let src = crate::gateway::fsencode_path_w(args[0])?; - let dst = crate::gateway::fsencode_path_w(args[1])?; + let src = crate::gateway::fsencode_path_named_w(args[0], "symlink", "src")?; + let dst = crate::gateway::fsencode_path_named_w(args[1], "symlink", "dst")?; let target_is_directory = match args .get(2) .copied() @@ -8509,6 +9316,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // Every optional argument is positional-or-keyword, so the // four of them are looked up either way round. + // + // `filepath` and `cwd` convert through the caller-less form: + // this entry point exists only here, so the wording it should + // name itself with is unmeasured on this host. See the + // follow-up task. let (args, kwargs) = crate::builtins::split_builtin_kwargs(args); crate::builtins::kwarg_reject_unknown( kwargs, @@ -8576,6 +9388,13 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { ); // os.cpu_count() -> int | None + // + // `rposix.py:2978-2986` reads `GetSystemInfo().dwNumberOfProcessors` + // here, which counts the processors in the caller's processor group; + // `available_parallelism` answers the process affinity mask instead, so + // the two part company on a host that has restricted one. Left as it is + // because no Windows oracle is reachable from this host to measure + // which the surface should report — see the follow-up task. crate::module_ns_store( ns, "cpu_count", @@ -8594,6 +9413,12 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // one re-encodes it through the ANSI code page. Neither reports a // failure other than through the status, which `os_system_impl` // returns as it is. + // + // The POSIX `system` converts its command as a filesystem name and so + // reports the caller-less message — measured. This one declares text + // rather than a path, so the message it should report is a different + // shape entirely and is unmeasured here; it keeps the same conversion + // meanwhile. See the follow-up task. crate::module_ns_store( ns, "system", @@ -8704,6 +9529,9 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { 0, ), ); + // `volume` converts through the caller-less form: 3.14 added this entry + // point on Windows alone, so what it names itself with is unmeasured on + // this host. See the follow-up task. crate::module_ns_store( ns, "listmounts", @@ -8837,9 +9665,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // inheritance control (set_inheritable would mutate a real fd). "dup", "dup2", - "dup3", "pipe", - "pipe2", "openpty", "login_tty", "sendfile", @@ -8935,6 +9761,70 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::make_builtin_function(name, sandbox_unavailable), ); } + + // The same, for the names only some hosts have. Listing them above + // would not neutralise anything on a host that never registered them — + // `module_ns_store` writes rather than overwrites, so it would publish + // a `posix.pipe2` where there is no `pipe2` to refuse. + #[cfg(any( + target_os = "android", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd", + target_os = "openbsd" + ))] + crate::module_ns_store( + ns, + "pipe2", + crate::make_builtin_function("pipe2", sandbox_unavailable), + ); + // The policy calls reach the host scheduler; only the setters mutate, + // but a policy read is a host-process leak in the same way `getpriority` + // above is. `sched_param` is left alone — it carries no host access. + #[cfg(any( + target_os = "android", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" + ))] + for name in [ + "sched_getscheduler", + "sched_getparam", + "sched_rr_get_interval", + ] { + crate::module_ns_store( + ns, + name, + crate::make_builtin_function(name, sandbox_unavailable), + ); + } + // The affinity mask is the same kind of host-process leak, and carries + // the narrower gate the pair is published under. + #[cfg(any(target_os = "linux", target_os = "android"))] + for name in ["sched_getaffinity", "sched_setaffinity"] { + crate::module_ns_store( + ns, + name, + crate::make_builtin_function(name, sandbox_unavailable), + ); + } + #[cfg(all( + not(target_env = "musl"), + any( + target_os = "android", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd" + ) + ))] + for name in ["sched_setscheduler", "sched_setparam"] { + crate::module_ns_store( + ns, + name, + crate::make_builtin_function(name, sandbox_unavailable), + ); + } } crate::module_ns_store(ns, "error", crate::typedef::w_object()); diff --git a/pyre/pyre-interpreter/src/module/pyexpat/mod.rs b/pyre/pyre-interpreter/src/module/pyexpat/mod.rs index 4acbce8f81e..a8cc8aefab4 100644 --- a/pyre/pyre-interpreter/src/module/pyexpat/mod.rs +++ b/pyre/pyre-interpreter/src/module/pyexpat/mod.rs @@ -1742,7 +1742,17 @@ fn init_parser_slots(parser: PyObjectRef) { ); } -/// `ParserCreate(encoding=None, namespace_separator=None, intern=None)`. +/// `ParserCreate()`'s spelling for a `str`-or-`None` parameter handed +/// something else. Both parameters report it, so the argument name is the only +/// thing that varies. +fn parser_create_not_str(param: &str, obj: PyObjectRef) -> crate::PyError { + crate::PyError::type_error(format!( + "ParserCreate() argument '{param}' must be str or None, not {}", + crate::type_methods::arg_type_name(obj) + )) +} + +/// `ParserCreate(encoding=None, namespace_separator=None[, intern])`. fn parser_create3( encoding: PyObjectRef, namespace_separator: PyObjectRef, @@ -1752,14 +1762,22 @@ fn parser_create3( init_parser_slots(parser); if unsafe { !is_none(encoding) } { if unsafe { !is_str(encoding) } { - return Err(crate::PyError::type_error( - "ParserCreate() argument 'encoding' must be str or None", - )); - } + return Err(parser_create_not_str("encoding", encoding)); + } + // Both stored names are read back through `w_str_get_value`, which + // panics on a lone surrogate — `declared_or_forced_encoding` for this + // one, `namespace_separator` for the other. A name with no UTF-8 + // spelling is refused here so that read cannot abort the process; + // `space.text_w` has no such reader behind it and so does not need the + // refusal. + crate::baseobjspace::str_utf8_w(encoding)?; crate::baseobjspace::setdictvalue_native(parser, "_pyre_forced_encoding", encoding); } if unsafe { is_none(namespace_separator) } { } else if unsafe { is_str(namespace_separator) } { + // `namespace_separator` reads this back through `w_str_get_value`, so + // the refusal the encoding arm makes applies here too; the length check + // below then runs on a value that has a `&str` view. let value = crate::baseobjspace::str_utf8_w(namespace_separator)?; if value.chars().count() > 1 { return Err(crate::PyError::value_error( @@ -1772,11 +1790,17 @@ fn parser_create3( w_str_new(value), ); } else { - return Err(crate::PyError::type_error( - "ParserCreate() argument 'namespace_separator' must be str or None, not int", + return Err(parser_create_not_str( + "namespace_separator", + namespace_separator, )); } - if unsafe { !is_none(intern) } { + // interp_pyexpat.py:948-952 — "Explicitly passing None means no interning + // is desired. Not passing anything means that a new dictionary is used." + // `init_parser_slots` installed that new dictionary already, so an omitted + // argument has nothing to write and the two named cases write themselves; + // `intern_string` reads `None` back as "do not intern". + if !intern.is_null() { crate::baseobjspace::setdictvalue_native(parser, "intern", intern); } Ok(parser) @@ -1931,7 +1955,10 @@ crate::py_module! { fn ParserCreate( #[default(w_none())] encoding: PyObjectRef, #[default(w_none())] namespace_separator: PyObjectRef, - #[default(w_none())] intern: PyObjectRef, + // Omitted has to be distinguishable from an explicit `None` here — + // they mean different things (`parser_create3`) — so the default is + // the absent marker and not the value. + #[default(pyre_object::PY_NULL)] intern: PyObjectRef, ) -> Result { parser_create3(encoding, namespace_separator, intern) } diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index ea6d22ee897..f6f8a19f508 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -1433,6 +1433,20 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { 0, ), ); + // sys._get_cpu_count_config() — the cpu count the interpreter was + // configured with, or -1 where none was. `os.py:1180` asks before it + // decides whether `process_cpu_count` counts the affinity mask or aliases + // `cpu_count`, so a `posix` that publishes `sched_getaffinity` and a `sys` + // that does not answer this makes `import os` raise. + // + // -1 is the answer here rather than a placeholder: the value is set by + // `-X cpu_count` and `PYTHON_CPU_COUNT`, and this interpreter reads + // neither, so there is no configured count to report. + module_ns_store( + ns, + "_get_cpu_count_config", + make_builtin_function_with_arity("_get_cpu_count_config", |_| Ok(w_int_new(-1)), 0), + ); // sys.getrecursionlimit / setrecursionlimit — pypy/module/sys/vm.py:45. // The runtime stack budget lives in `crate::stack_check`; both // helpers route through it so the interpreter, JIT prologue probe, @@ -2658,25 +2672,54 @@ pub fn audit_hooks_armed() -> bool { !holder.is_null() && unsafe { (*holder).hooks_armed.get() } } +/// `vm.py:474 audit(space, event, args_w)` under `@unwrap_spec(event="text")`. +/// +/// The unwrap in front of the hook dispatch is observable on its own: the +/// parameters are positional-only, and the event name has to be a `str` with a +/// UTF-8 spelling, so a bad event name is reported at the call rather than +/// carried to whichever hook reads it. +/// +/// The unwrap is `str_utf8_w` and not a surrogate-tolerant read because the +/// encode is there for the error it raises: an event name holding a lone +/// surrogate is a `UnicodeEncodeError` at the call. fn sys_audit(args: &[pyre_object::PyObjectRef]) -> crate::PyResult { - let Some(&w_event) = args.first() else { + let (positional, kwargs) = crate::builtins::split_builtin_kwargs(args); + if crate::builtins::has_real_kwargs(kwargs) { + return Err(crate::PyError::type_error( + "sys.audit() takes no keyword arguments", + )); + } + let Some(&w_event) = positional.first() else { return Err(crate::PyError::type_error( - "audit() missing 1 required positional argument: 'event'", + "audit expected at least 1 argument, got 0", )); }; // `@unwrap_spec(event="text")` if !unsafe { pyre_object::is_str(w_event) } { - return Err(crate::PyError::type_error( - "audit() argument 1 must be str, not other", - )); + // `_PyArg_BadArgument` names the `None` singleton itself rather than + // its type. + let given = if unsafe { pyre_object::is_none(w_event) } { + "None".to_string() + } else { + crate::type_methods::arg_type_name(w_event) + }; + return Err(crate::PyError::type_error(format!( + "audit() argument 1 must be str, not {given}" + ))); } - audit_w(w_event, &args[1..])?; + // The `@unwrap_spec` round trip is observable: the hooks are handed the + // `str` the unwrapped name is re-wrapped as, so a `str` subclass reaches + // them flattened to a plain one. + let event = crate::baseobjspace::str_utf8_w(w_event)?; + audit_w(w_str_new(event), &positional[1..])?; Ok(w_none()) } -/// `vm.py:485 addaudithook`. The hooks already installed get a say: a -/// `RuntimeError` out of the `sys.addaudithook` event means the set refused the -/// new hook, and it is dropped rather than added. Anything else propagates. +/// `vm.py:486 addaudithook`. The hooks already installed get a say: an +/// `Exception` out of the `sys.addaudithook` event means the set refused the +/// new hook, and it is dropped rather than added. A `BaseException` outside +/// `Exception` propagates. The new hook is not installed yet when the event +/// fires, so it never gets to refuse its own installation. fn sys_addaudithook(args: &[pyre_object::PyObjectRef]) -> crate::PyResult { let Some(&w_hook) = args.first() else { return Err(crate::PyError::type_error( @@ -2684,7 +2727,7 @@ fn sys_addaudithook(args: &[pyre_object::PyObjectRef]) -> crate::PyResult { )); }; if let Err(err) = audit("sys.addaudithook", &[]) { - if !error_is_runtime_error(&err) { + if !error_is_exception(&err) { return Err(err); } return Ok(w_none()); @@ -2713,22 +2756,34 @@ fn sys_addaudithook(args: &[pyre_object::PyObjectRef]) -> crate::PyResult { Ok(w_none()) } -/// `e.match(space, space.w_RuntimeError)` for a `PyError` that may carry either -/// an interpreter-level kind or a materialised exception object. -fn error_is_runtime_error(err: &crate::PyError) -> bool { - // A hook may raise a RuntimeError SUBCLASS, so the interpreter-level kind - // alone is not the test; it is the fallback for an error that never - // materialised an exception object. - let w_runtime_error = pyre_object::interp_exceptions::lookup_exc_class_for_kind( - pyre_object::interp_exceptions::ExcKind::RuntimeError, +/// `e.match(space, space.w_Exception)` for a `PyError` that may carry either an +/// interpreter-level kind or a materialised exception object. +/// +/// The C-level `PySys_AddAuditHook` reads a refusal as `RuntimeError`, but the +/// `sys.addaudithook` a Python caller reaches widens it to `Exception`. +/// Measured against 3.14 with a refusing hook already installed: `RuntimeError`, +/// `ValueError` and `Exception` all leave the call returning `None`, and only a +/// `BaseException` outside `Exception` — `KeyboardInterrupt`, bare +/// `BaseException` — comes back out. +fn error_is_exception(err: &crate::PyError) -> bool { + // A hook raises a real exception object, so the class test is the live one; + // the interpreter-level kind is the fallback for an error that never + // materialised an object. + let w_exception = pyre_object::interp_exceptions::lookup_exc_class_for_kind( + pyre_object::interp_exceptions::ExcKind::Exception, ); - if !err.exc_object.is_null() && !w_runtime_error.is_null() { + if !err.exc_object.is_null() && !w_exception.is_null() { let _roots = pyre_object::gc_roots::push_roots(); pyre_object::gc_roots::pin_root(err.exc_object); - pyre_object::gc_roots::pin_root(w_runtime_error); - return crate::baseobjspace::isinstance(err.exc_object, w_runtime_error).unwrap_or(false); - } - matches!(err.kind, crate::PyErrorKind::RuntimeError) + pyre_object::gc_roots::pin_root(w_exception); + return crate::baseobjspace::isinstance(err.exc_object, w_exception).unwrap_or(false); + } + // `exc_kind_matches(kind, "Exception")` spelled over `PyErrorKind`: every + // variant descends from `Exception` except the two `BaseException` ones. + !matches!( + err.kind, + crate::PyErrorKind::GeneratorExit | crate::PyErrorKind::SystemExit + ) } /// `sysmodule.c sys._clear_type_descriptors`: remove the instance-dict and weakref diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 2d4ec61dd6b..41addc5bbb9 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -392,45 +392,40 @@ pub(crate) fn try_walker_specialize_unary_positive_int( dst: usize, dst_bank: char, ) -> Result, DispatchError> { - let Some(obj) = walker_concrete_ref_object(ctx, operand) else { - return Ok(None); - }; // `+x` is identity only for an EXACT builtin int. A bool shares the - // `intval` but `+True` is int `1`, not identity. A numeric subclass keeps - // the builtin `ob_type` (its Python class lives in `w_class`), so the - // `GUARD_CLASS INT` below reads `ob_type` and would NOT catch it at - // runtime — forwarding the operand would return the subclass instead of - // the plain int its `__pos__` yields. Both decline to the generic - // residual. Mirrors the `is_exact_builtin_instance` gate in - // `walker_int_specialization_operands`. - // SAFETY: `obj` is a live concrete `PyObjectRef` from the walker shadow. - if unsafe { - !pyre_object::is_int(obj) - || pyre_object::is_bool(obj) - || !pyre_object::is_exact_builtin_instance(obj) - } { + // `intval` but `+True` is int `1`, not identity, and a numeric subclass + // must reach its own `__pos__` rather than have the operand forwarded. + let Some((_, x_class)) = walker_unary_int_operand(ctx, operand) else { return Ok(None); - } + }; let int_type_addr = &pyre_object::pyobject::INT_TYPE as *const _ as i64; // Emit the guard prefix (`GUARD_CLASS INT` / tag test) so a later non-int // arrival deopts; the returned raw is unused because the result is the // operand box itself. let _ = walker_unbox_int(ctx, op_pc, operand, int_type_addr)?; + walker_guard_exact_w_class(ctx, op_pc, operand, x_class)?; write_residual_call_result_to_dst(ctx, op_pc, dst, dst_bank, operand)?; Ok(Some(())) } -/// Shared gate for the `UNARY_NEGATIVE` / `UNARY_INVERT` int folds: the operand -/// must be a concrete EXACT builtin non-bool `W_IntObject`. A bool unboxes -/// through its own `&BOOL_TYPE` guard (declined here for simplicity — `-True` / -/// `~True` stay on the residual), and a numeric subclass keeps the builtin -/// `ob_type` so the `GUARD_CLASS INT` the fold emits would not catch it at -/// runtime. Returns the concrete `intval` on success. Mirrors the -/// `is_exact_builtin_instance` gate in `walker_int_specialization_operands`. +/// Shared gate for the `UNARY_POSITIVE` / `UNARY_NEGATIVE` / `UNARY_INVERT` int +/// folds: the operand must be a concrete EXACT builtin non-bool `W_IntObject`. +/// A bool unboxes through its own `&BOOL_TYPE` guard (declined here for +/// simplicity — `+True` / `-True` / `~True` stay on the residual). +/// +/// Returns the concrete `intval` and the canonical `int` type object the caller +/// must pin with [`walker_guard_exact_w_class`]. `is_exact_builtin_instance` +/// only settles the operand the trace RECORDED; a numeric subclass keeps the +/// builtin `ob_type`, so the `GUARD_CLASS INT` the fold emits does not stop one +/// from entering the trace later and being answered by the fold instead of its +/// own `__neg__` / `__invert__` / `__pos__`. Pinning `w_class` is what makes +/// that arrival side-exit, and the operand that carries the null spelling of +/// "exact builtin" has no value to pin, so it declines — the same shape the +/// long folds use (`walker_exact_builtin_class` + guard). fn walker_unary_int_operand( ctx: &mut WalkContext<'_, '_, Sym>, operand: OpRef, -) -> Option { +) -> Option<(i64, pyre_object::PyObjectRef)> { let obj = walker_concrete_ref_object(ctx, operand)?; // SAFETY: `obj` is a live concrete `PyObjectRef` from the walker shadow. unsafe { @@ -440,8 +435,102 @@ fn walker_unary_int_operand( { return None; } - Some(pyre_object::w_int_get_value(obj)) + let class = walker_exact_builtin_class(obj)?; + Some((pyre_object::w_int_get_value(obj), class)) + } +} + +/// The `W_LongObject.value` payload of a concrete long, read the way the folds +/// that pass a payload to an `rbigint` helper need it. +/// +/// # Safety +/// `obj` must be a live concrete `W_LongObject` from the walker shadow. +unsafe fn long_payload_of(obj: pyre_object::PyObjectRef) -> i64 { + unsafe { *((obj as *const u8).add(pyre_object::longobject::LONG_VALUE_OFFSET) as *const i64) } +} + +/// Record the `getfield_gc_r` that reads a long operand's `value` payload. +/// A box the same trace built with [`crate::helpers::emit_box_long_inline`] +/// answers this out of the heap cache, so the read costs nothing and the box +/// keeps no reason to escape. +fn walker_read_long_payload( + ctx: &mut WalkContext<'_, '_, Sym>, + boxed: OpRef, + concrete_payload: i64, +) -> OpRef { + let payload = ctx.trace_ctx.record_op_with_descr( + OpCode::GetfieldGcR, + &[boxed], + crate::descr::long_value_descr(), + ); + ctx.trace_ctx.set_opref_concrete( + payload, + majit_ir::Value::Ref(majit_ir::GcRef(concrete_payload as usize)), + ); + payload +} + +/// `intobject.py:494 _make_ovf2long`: the tail every int arithmetic fold shares +/// once its own guard has pinned the promoting branch — `GUARD_OVERFLOW` for +/// the `BINARY_OP` arm, `GUARD_VALUE` on the operand for unary negate. The tail +/// is the elidable raw-int bigint helper (`rbigint.py:717/788/873`) under +/// `EF_ELIDABLE_OR_MEMORYERROR`, then the inline `W_LongObject` box around the +/// payload it returns. `payload_fn` takes the two machine ints in +/// `(raw, concrete)` pairs, which is also the shape the concrete-args vector +/// wants. +/// +/// The box needs no preceding fits_int guard. `newlong_from_rbigint` +/// (objspace.py:316-320) demotes through `rbigint.toint()`, whose +/// `numdigits() > MAX_DIGITS_THAT_CAN_FIT_IN_INT` test (rbigint.py:470) that +/// guard already answers: the helper is the *exact* int-pair sum / difference / +/// product, so a value that just overflowed a machine int cannot fit one back. +/// The same fold is what lets `try_walker_specialize_binary_op_long_int_pow` +/// skip its result-fits guard. +fn walker_emit_ovf2long_box( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + payload_fn: *const (), + lhs: (OpRef, i64), + rhs: (OpRef, i64), + boxed_result_i64: i64, +) -> Result { + let (lhs_raw, la) = lhs; + let (rhs_raw, rb) = rhs; + let payload_concrete = + unsafe { long_payload_of(boxed_result_i64 as usize as pyre_object::PyObjectRef) }; + let concrete_args = [ + majit_ir::Value::Int(payload_fn as usize as i64), + majit_ir::Value::Int(la), + majit_ir::Value::Int(rb), + ]; + let payload = ctx.trace_ctx.call_typed_with_effect_pure_can_raise( + OpCode::CallR, + payload_fn, + &[lhs_raw, rhs_raw], + &[majit_ir::Type::Int, majit_ir::Type::Int], + majit_ir::Type::Ref, + majit_metainterp::ELIDABLE_OR_MEMERROR_EFFECT_INFO, + &concrete_args, + majit_ir::Value::Ref(majit_ir::GcRef(payload_concrete as usize)), + ); + ctx.trace_ctx.set_opref_concrete( + payload, + majit_ir::Value::Ref(majit_ir::GcRef(payload_concrete as usize)), + ); + if payload.inline_const_to_value().is_none() { + walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardNoException, &[])?; } + let result = crate::helpers::emit_box_long_inline( + ctx.trace_ctx, + payload, + crate::descr::w_long_size_descr(), + crate::descr::long_value_descr(), + ); + ctx.trace_ctx.set_opref_concrete( + result, + majit_ir::Value::Ref(majit_ir::GcRef(boxed_result_i64 as usize)), + ); + Ok(result) } /// #61: walker-native int specialization for the `UNARY_NEGATIVE` residual @@ -450,14 +539,17 @@ fn walker_unary_int_operand( /// `W_LongObject` (`intobject.py:628` `descr_neg` → `_make_ovf2long`). Since /// majit has no overflow-checked unary negate, the fold expresses `-x` as /// `IntSubOvf(0, x)` behind a `GUARD_CLASS INT`, reusing the binary-sub -/// overflow discipline: a record value of `INT_MIN` declines (the residual -/// builds the `2**63` long), and any other record value emits a -/// `GUARD_NO_OVERFLOW` so an `INT_MIN` arrival on the reused trace deopts to -/// the residual rather than wrapping back to `INT_MIN`. +/// overflow discipline in both directions: a record value other than `INT_MIN` +/// emits `GUARD_NO_OVERFLOW` so an `INT_MIN` arrival on the reused trace deopts +/// rather than wrapping back to `INT_MIN`, and a record value of `INT_MIN` +/// pins the operand with `GUARD_VALUE` and takes the same `_make_ovf2long` tail +/// the `BINARY_OP` overflow arm takes, so the `2**63` long is built from the +/// elidable bigint helper instead of the `CallMayForce` residual. /// /// Returns `Ok(Some(()))` when the fold was emitted (caller returns -/// `Continue`); `Ok(None)` for a bool / subclass / non-int / `INT_MIN` -/// operand, or when the residual result box is unavailable. +/// `Continue`); `Ok(None)` for a bool / subclass / non-int operand, when the +/// residual result box is unavailable, or when an `INT_MIN` operand did not +/// produce the promoted `W_LongObject` the payload read expects. pub(crate) fn try_walker_specialize_unary_negative_int( ctx: &mut WalkContext<'_, '_, Sym>, op_pc: usize, @@ -467,29 +559,57 @@ pub(crate) fn try_walker_specialize_unary_negative_int( dst: usize, dst_bank: char, ) -> Result, DispatchError> { - let Some(x) = walker_unary_int_operand(ctx, operand) else { + let Some((x, x_class)) = walker_unary_int_operand(ctx, operand) else { return Ok(None); }; - // `0 - INT_MIN` overflows i64 → `2**63` long; let the residual build it. - if x == i64::MIN { - return Ok(None); - } let Some(boxed_result_i64) = walker_execute_may_force_boxed(ctx, allboxes, call_descr) else { return Ok(None); }; + // `0 - INT_MIN` is the one operand `descr_neg` promotes. + let overflows = x == i64::MIN; + if overflows { + let boxed_result_obj = boxed_result_i64 as usize as pyre_object::PyObjectRef; + if boxed_result_obj == pyre_object::PY_NULL + || !unsafe { pyre_object::is_long(boxed_result_obj) } + { + return Ok(None); + } + } let int_type_addr = &pyre_object::pyobject::INT_TYPE as *const _ as i64; let x_raw = walker_unbox_int(ctx, op_pc, operand, int_type_addr)?; + walker_guard_exact_w_class(ctx, op_pc, operand, x_class)?; let zero_raw = ctx.trace_ctx.const_int(0); - let result_value = 0i64.wrapping_sub(x); - let raw_result = ctx - .trace_ctx - .record_op(OpCode::IntSubOvf, &[zero_raw, x_raw]); - ctx.trace_ctx - .set_opref_concrete(raw_result, majit_ir::Value::Int(result_value)); - walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardNoOverflow, &[])?; - let boxed = walker_box_int(ctx, op_pc, raw_result, result_value)?; - ctx.trace_ctx - .set_opref_concrete(boxed, box_int_concrete(result_value, boxed_result_i64)); + let boxed = if overflows { + // `0 - x` overflows an i64 for exactly one operand, so "the negate + // promoted" and "the operand is INT_MIN" name the same set: guarding + // the value admits what `GUARD_OVERFLOW` would and nothing more. The + // value form is the one the tail can use — with `x_raw` constant the + // elidable bigint call folds to the `2**63` payload it returned while + // recording instead of running once per iteration. This is the + // `guard_value` spelling the version-tag promotes already use. + let int_min = ctx.trace_ctx.const_int(i64::MIN); + walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardValue, &[x_raw, int_min])?; + walker_emit_ovf2long_box( + ctx, + op_pc, + pyre_object::longobject::jit_bigint_sub_int_int as *const (), + (zero_raw, 0), + (x_raw, x), + boxed_result_i64, + )? + } else { + let result_value = 0i64.wrapping_sub(x); + let raw_result = ctx + .trace_ctx + .record_op(OpCode::IntSubOvf, &[zero_raw, x_raw]); + ctx.trace_ctx + .set_opref_concrete(raw_result, majit_ir::Value::Int(result_value)); + walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardNoOverflow, &[])?; + let boxed = walker_box_int(ctx, op_pc, raw_result, result_value)?; + ctx.trace_ctx + .set_opref_concrete(boxed, box_int_concrete(result_value, boxed_result_i64)); + boxed + }; write_residual_call_result_to_dst(ctx, op_pc, dst, dst_bank, boxed)?; Ok(Some(())) } @@ -512,7 +632,7 @@ pub(crate) fn try_walker_specialize_unary_invert_int( dst: usize, dst_bank: char, ) -> Result, DispatchError> { - let Some(x) = walker_unary_int_operand(ctx, operand) else { + let Some((x, x_class)) = walker_unary_int_operand(ctx, operand) else { return Ok(None); }; let Some(boxed_result_i64) = walker_execute_may_force_boxed(ctx, allboxes, call_descr) else { @@ -520,6 +640,7 @@ pub(crate) fn try_walker_specialize_unary_invert_int( }; let int_type_addr = &pyre_object::pyobject::INT_TYPE as *const _ as i64; let x_raw = walker_unbox_int(ctx, op_pc, operand, int_type_addr)?; + walker_guard_exact_w_class(ctx, op_pc, operand, x_class)?; let result_value = !x; let raw_result = ctx.trace_ctx.record_op(OpCode::IntInvert, &[x_raw]); ctx.trace_ctx @@ -684,7 +805,6 @@ pub(crate) fn try_walker_specialize_binary_op_int( let lhs_raw = walker_unbox_int_typed(ctx, op_pc, lhs, lhs_type, lhs_descr)?; let rhs_raw = walker_unbox_int_typed(ctx, op_pc, rhs, rhs_type, rhs_descr)?; if overflows { - let boxed_result_obj = boxed_result_i64 as usize as pyre_object::PyObjectRef; let concrete_value = match op_code { OpCode::IntAddOvf => la.wrapping_add(rb), OpCode::IntSubOvf => la.wrapping_sub(rb), @@ -696,57 +816,20 @@ pub(crate) fn try_walker_specialize_binary_op_int( .set_opref_concrete(raw_result, majit_ir::Value::Int(concrete_value)); walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardOverflow, &[])?; - let payload_concrete = unsafe { - *((boxed_result_obj as *const u8).add(pyre_object::longobject::LONG_VALUE_OFFSET) - as *const i64) - }; let payload_fn = match op_code { OpCode::IntAddOvf => pyre_object::longobject::jit_bigint_add_int_int as *const (), OpCode::IntSubOvf => pyre_object::longobject::jit_bigint_sub_int_int as *const (), OpCode::IntMulOvf => pyre_object::longobject::jit_bigint_mul_int_int as *const (), _ => unreachable!("overflow arm requires Add/Sub/Mul"), }; - let concrete_args = [ - majit_ir::Value::Int(payload_fn as usize as i64), - majit_ir::Value::Int(la), - majit_ir::Value::Int(rb), - ]; - let payload = ctx.trace_ctx.call_typed_with_effect_pure_can_raise( - OpCode::CallR, + let result = walker_emit_ovf2long_box( + ctx, + op_pc, payload_fn, - &[lhs_raw, rhs_raw], - &[majit_ir::Type::Int, majit_ir::Type::Int], - majit_ir::Type::Ref, - majit_metainterp::ELIDABLE_OR_MEMERROR_EFFECT_INFO, - &concrete_args, - majit_ir::Value::Ref(majit_ir::GcRef(payload_concrete as usize)), - ); - ctx.trace_ctx.set_opref_concrete( - payload, - majit_ir::Value::Ref(majit_ir::GcRef(payload_concrete as usize)), - ); - if payload.inline_const_to_value().is_none() { - walker_emit_guard_with_snapshot(ctx, op_pc, OpCode::GuardNoException, &[])?; - } - - // The box needs no preceding fits_int guard. `newlong_from_rbigint` - // (objspace.py:316-320) demotes through `rbigint.toint()`, whose - // `numdigits() > MAX_DIGITS_THAT_CAN_FIT_IN_INT` test (rbigint.py:470) - // the GuardOverflow above already answers: the helper is the *exact* - // int-pair sum / difference / product, so a value that just overflowed - // a machine int cannot fit one back. The same fold is what lets - // `try_walker_specialize_binary_op_long_int_pow` skip its result-fits - // guard. - let result = crate::helpers::emit_box_long_inline( - ctx.trace_ctx, - payload, - crate::descr::w_long_size_descr(), - crate::descr::long_value_descr(), - ); - ctx.trace_ctx.set_opref_concrete( - result, - majit_ir::Value::Ref(majit_ir::GcRef(boxed_result_i64 as usize)), - ); + (lhs_raw, la), + (rhs_raw, rb), + boxed_result_i64, + )?; write_residual_call_result_to_dst(ctx, op_pc, dst, dst_bank, result)?; return Ok(Some(())); } @@ -5369,8 +5452,9 @@ pub(crate) fn try_walker_specialize_compare_op_long_int( /// W_LongObject (bigint) COMPARE_OP specialization — the long analogue of /// [`try_walker_specialize_compare_op_int`]. Both operands are `int`-typed but -/// bigint-stored: guard each against `LONG_TYPE`, then `CallPure_I` the pure -/// `jit_w_long_cmp` (sign of `a <=> b` in {-1,0,1}; a comparison neither +/// bigint-stored: guard each against `LONG_TYPE`, read each `value` payload, +/// then `CallPure_I` the pure +/// `jit_bigint_cmp` (sign of `a <=> b` in {-1,0,1}; a comparison neither /// allocates nor raises, so `EF_ELIDABLE_CANNOT_RAISE` and NO trailing guard) /// and turn the sign into the requested truth with `int_(sign, 0)` before /// boxing to a `W_Bool` (same #62 dead-box elision as the int path). Same gate @@ -5431,19 +5515,29 @@ pub(crate) fn try_walker_specialize_compare_op_long( walker_guard_class(ctx, op_pc, rhs, long_type_addr)?; walker_guard_exact_w_class(ctx, op_pc, lhs, lhs_class)?; walker_guard_exact_w_class(ctx, op_pc, rhs, rhs_class)?; + // `_make_descr_cmp` (longobject.py:383-391) compares `self.num` against + // `w_other.num`, so the two payload reads are trace ops rather than work + // hidden inside the callee. Spelling them out is also what keeps a + // `W_LongObject` this same trace built from having to escape into the + // comparison: the read hits the heap cache entry `emit_box_long_inline` + // filed and the box stays virtual. + let lhs_payload = unsafe { long_payload_of(lhs_obj) }; + let rhs_payload = unsafe { long_payload_of(rhs_obj) }; + let lhs_pl = walker_read_long_payload(ctx, lhs, lhs_payload); + let rhs_pl = walker_read_long_payload(ctx, rhs, rhs_payload); // Pure `rbigint` comparison → sign in {-1,0,1}. Dead after the `int_` // below and never spans a guard, so it needs no blackhole reconstruction. - let cmp_fn = pyre_object::longobject::jit_w_long_cmp as *const (); - let sign_concrete = pyre_object::longobject::jit_w_long_cmp(lhs_obj as i64, rhs_obj as i64); + let cmp_fn = pyre_object::longobject::jit_bigint_cmp as *const (); + let sign_concrete = pyre_object::longobject::jit_bigint_cmp(lhs_payload, rhs_payload); let concrete_args = [ majit_ir::Value::Int(cmp_fn as usize as i64), - majit_ir::Value::Ref(majit_ir::GcRef(lhs_obj as usize)), - majit_ir::Value::Ref(majit_ir::GcRef(rhs_obj as usize)), + majit_ir::Value::Ref(majit_ir::GcRef(lhs_payload as usize)), + majit_ir::Value::Ref(majit_ir::GcRef(rhs_payload as usize)), ]; let sign = ctx.trace_ctx.call_typed_with_effect_pure( OpCode::CallI, cmp_fn, - &[lhs, rhs], + &[lhs_pl, rhs_pl], &[majit_ir::Type::Ref, majit_ir::Type::Ref], majit_ir::Type::Int, majit_metainterp::ELIDABLE_CANNOT_RAISE_EFFECT_INFO, diff --git a/pyre/pyre-object/src/longobject.rs b/pyre/pyre-object/src/longobject.rs index 4ebe4769d71..2aa3490cb16 100644 --- a/pyre/pyre-object/src/longobject.rs +++ b/pyre/pyre-object/src/longobject.rs @@ -661,22 +661,27 @@ pub extern "C" fn jit_bigint_xor(a: i64, b: i64) -> i64 { unsafe { alloc_bigint_nursery_collecting(&*a ^ &*b) as i64 } } -/// `rbigint` comparison payload for `W_LongObject` — returns the sign of -/// `a <=> b` as `-1` / `0` / `1`. RPython exposes the comparison as six methods -/// (`lt`/`le`/`eq`/`ne`/`gt`/`ge`, the latter built as `other.lt(self)` -/// wrappers, `rbigint.py:573/664`); Rust's total `Ord::cmp` collapses them into -/// one three-way result, and the caller recovers each relation with a plain -/// `int_(sign, 0)` (e.g. `a < b` ⟺ `sign < 0`, `a == b` ⟺ `sign == 0`). -/// A comparison neither allocates nor raises, so this is -/// `EF_ELIDABLE_CANNOT_RAISE` and the fast path records `CallPure*` with NO -/// trailing guard. +/// `rbigint` comparison — returns the sign of `a <=> b` as `-1` / `0` / `1`. +/// RPython exposes the comparison as six methods (`lt`/`le`/`eq`/`ne`/`gt`/`ge`, +/// the latter built as `other.lt(self)` wrappers, `rbigint.py:573/664`); Rust's +/// total `Ord::cmp` collapses them into one three-way result, and the caller +/// recovers each relation with a plain `int_(sign, 0)` (e.g. `a < b` ⟺ +/// `sign < 0`, `a == b` ⟺ `sign == 0`). A comparison neither allocates nor +/// raises, so this is `EF_ELIDABLE_CANNOT_RAISE` and the fast path records +/// `CallPure*` with NO trailing guard. +/// +/// The arguments are the bare payloads, not the `W_LongObject` boxes: +/// `_make_descr_cmp` (longobject.py:383-391) compares `self.num` against +/// `w_other.num`, so the two field reads belong in the trace and not inside the +/// callee. With them spelled out, a `W_LongObject` the same trace has just +/// built is read back through the heap cache and can stay virtual instead of +/// being forced into a real allocation just to be handed to a comparison. #[majit_macros::elidable_cannot_raise] -pub extern "C" fn jit_w_long_cmp(a: i64, b: i64) -> i64 { +pub extern "C" fn jit_bigint_cmp(a: i64, b: i64) -> i64 { use core::cmp::Ordering; - let a = a as PyObjectRef; - let b = b as PyObjectRef; + let (a, b) = (a as *const BigInt, b as *const BigInt); unsafe { - match w_long_get_value(a).cmp(w_long_get_value(b)) { + match (*a).cmp(&*b) { Ordering::Less => -1, Ordering::Equal => 0, Ordering::Greater => 1,