From 1bce179c31fc4603577365b1c6d0c7bf4689040a Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 01:09:33 +0900 Subject: [PATCH 01/16] dict: keep the borrowed-str probe in w_dict_getitem_wtf8_checked 41bac542c18 routed every name through `w_dict_lookup_checked`, so a valid-UTF-8 attribute name wrapped a throwaway `W_UnicodeObject` per lookup on the hot mapdict read path (`mapdict.rs:3143`). `w_dict_getitem_str_object_strategy` exists to avoid exactly that (dictmultiobject.rs:3387-3389 "A borrowed `&str` probe avoids the per-lookup throwaway `W_UnicodeObject` (`getitem_str` parity)"). The flag-swallowing that made the pre-41bac542c18 post-hoc drain inert is in the object-lookup leaf -- `w_dict_lookup_object_strategy` (:2668) is `..._checked(..).unwrap_or(None)` -- not in `w_dict_getitem_str`, whose leaf `dict_entries_get_str` (:451) clears the flag before the probe and leaves whatever the probe sets. So the `&str` arm can use `w_dict_getitem_str_checked` and lose no error; a raising comparison is reachable only from a bucket holding a non-string key, i.e. only under the object strategy, whose `getitem_str` (:6219) is the borrowed probe. The devolved-terminator reproducer from b62b1be9dba still raises `ValueError: boom` where it printed `CLASSVALUE` before that commit. Assisted-by: Claude --- pyre/pyre-object/src/dictmultiobject.rs | 27 +++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/pyre/pyre-object/src/dictmultiobject.rs b/pyre/pyre-object/src/dictmultiobject.rs index ddd9f72347a..143af9f03bd 100644 --- a/pyre/pyre-object/src/dictmultiobject.rs +++ b/pyre/pyre-object/src/dictmultiobject.rs @@ -3453,12 +3453,21 @@ pub unsafe fn w_dict_getitem_wtf8( /// surfaces it, and the caller recovers the concrete exception from the /// interpreter-side error slot. /// -/// Routes through [`w_dict_lookup_checked`] rather than draining the flag -/// after [`w_dict_getitem_wtf8`]: the strategy leaf -/// (`w_dict_lookup_object_strategy`) is itself -/// `..._checked(..).unwrap_or(None)`, so it has already taken the flag by the -/// time the unchecked spelling returns and a post-hoc `take_dict_key_error` -/// always reads `false`. +/// Mirrors the unchecked spelling's dispatch so a valid-UTF-8 key keeps the +/// borrowed-`&str` probe and its `getitem_str` parity — routing every name +/// through [`w_dict_lookup_checked`] instead would wrap a throwaway +/// `W_UnicodeObject` per lookup, which is exactly what +/// `w_dict_getitem_str_object_strategy` exists to avoid. +/// +/// The two arms need different checked leaves because only one of them +/// swallows: `w_dict_lookup_object_strategy` (:2668) is +/// `..._checked(..).unwrap_or(None)`, so draining the flag after the +/// unchecked `w_dict_lookup` always reads `false`, while +/// `dict_entries_get_str` (:451) leaves the probe's flag standing for +/// [`w_dict_getitem_str_checked`] to take. A raising comparison is reachable +/// only from a bucket holding a non-string key, i.e. only under the object +/// strategy — whose `getitem_str` (:6219) is the borrowed probe — so the +/// `&str` arm loses no error. /// /// # Safety /// `obj` must point to a valid `W_DictObject`. @@ -3466,8 +3475,10 @@ pub unsafe fn w_dict_getitem_wtf8_checked( obj: PyObjectRef, key: &rustpython_wtf8::Wtf8, ) -> Result, DictKeyError> { - let w_key = crate::w_str_from_wtf8(key.to_wtf8_buf()); - w_dict_lookup_checked(obj, w_key) + match key.as_str() { + Ok(s) => w_dict_getitem_str_checked(obj, s), + Err(_) => w_dict_lookup_checked(obj, crate::w_str_from_wtf8(key.to_wtf8_buf())), + } } /// WTF-8 keyed equivalent of `space.setitem_str` — `setitem_str` is itself From 3faf25fc259bc69c029845c39e459c852e6d731c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 01:09:45 +0900 Subject: [PATCH 02/16] error: answer no suggestion for a name that has no UTF-8 view `exception_suggestion` read the failing name with `w_str_get_value`, which panics on a lone surrogate, and `dict_string_keys` did the same for every candidate. An uncaught AttributeError whose attribute name carried an unpaired surrogate therefore killed the interpreter while rendering the traceback: S = "z\udcffz" class C: pass getattr(C(), S) thread '' panicked at pyre/pyre-object/src/unicodeobject.rs:579: w_str_get_value: backing Wtf8Buf is not valid UTF-8 (lone surrogate) python3.14 prints `AttributeError: 'C' object has no attribute 'z\udcffz'` with no suggestion suffix. Take `w_str_get_value_opt` at the three name reads and answer `None` when there is no `&str` view -- `suggestion_distance` is computed over `char`s, so such a name has nothing to compare against -- and skip non-UTF-8 candidates instead of pushing them. The `__module__` read in `exc_object_class_name` is left alone: it is not on this path and rendering it needs the WTF-8 spelling, not a bail. Assisted-by: Claude --- pyre/pyre-interpreter/src/error.rs | 63 ++++++++++++++++++------------ 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/pyre/pyre-interpreter/src/error.rs b/pyre/pyre-interpreter/src/error.rs index 1b8e121127f..02a70109595 100644 --- a/pyre/pyre-interpreter/src/error.rs +++ b/pyre/pyre-interpreter/src/error.rs @@ -2423,7 +2423,12 @@ fn dict_string_keys(dict: PyObjectRef, names: &mut Vec) { } for (key, _) in unsafe { pyre_object::w_dict_items(dict) } { if unsafe { pyre_object::is_str(key) } { - names.push(unsafe { pyre_object::w_str_get_value(key) }.to_string()); + // A name carrying a lone surrogate has no `&str` view, and the + // distance below is computed over `char`s. Drop it from the + // candidate set rather than reading it as UTF-8. + if let Some(name) = unsafe { pyre_object::w_str_get_value_opt(key) } { + names.push(name.to_string()); + } } } } @@ -2615,7 +2620,10 @@ fn exception_suggestion(exc_slot: usize) -> Option { { return None; } - let wrong_name = unsafe { pyre_object::w_str_get_value(wrong) }.to_string(); + // The distance below is computed over `char`s, so a name carrying a lone + // surrogate has nothing to compare; answer no suggestion instead of + // reading it as UTF-8. + let wrong_name = unsafe { pyre_object::w_str_get_value_opt(wrong) }?.to_string(); let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); let tb = unsafe { pyre_object::interp_exceptions::w_exception_get_traceback(exc) }; let tb_slot = if tb.is_null() { @@ -2646,28 +2654,35 @@ fn exception_suggestion(exc_slot: usize) -> Option { ExcKind::ImportError => { let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); let module_name = unsafe { pyre_object::interp_exceptions::w_exception_get_name(exc) }; - if module_name.is_null() || !unsafe { pyre_object::is_str(module_name) } { - None - } else { - let module_name = unsafe { pyre_object::w_str_get_value(module_name) }.to_string(); - crate::importing::get_sys_module(&module_name) - .or_else(|| { - crate::importing::importhook( - &module_name, - pyre_object::w_none(), - pyre_object::w_none(), - 0, - crate::call::take_last_exec_ctx(), - ) - .ok() - }) - .and_then(object_dir_strings) - .and_then(|mut names| { - if !wrong_name.starts_with('_') { - names.retain(|name| !name.starts_with('_')); - } - best_suggestion(&names, &wrong_name) - }) + let module_name = + if module_name.is_null() || !unsafe { pyre_object::is_str(module_name) } { + None + } else { + unsafe { pyre_object::w_str_get_value_opt(module_name) } + }; + match module_name { + None => None, + Some(module_name) => { + let module_name = module_name.to_string(); + crate::importing::get_sys_module(&module_name) + .or_else(|| { + crate::importing::importhook( + &module_name, + pyre_object::w_none(), + pyre_object::w_none(), + 0, + crate::call::take_last_exec_ctx(), + ) + .ok() + }) + .and_then(object_dir_strings) + .and_then(|mut names| { + if !wrong_name.starts_with('_') { + names.retain(|name| !name.starts_with('_')); + } + best_suggestion(&names, &wrong_name) + }) + } } } ExcKind::NameError => { From c9ea8fb52ca2c2ca0afec8f0d8c0491d7b41bccf Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 01:10:08 +0900 Subject: [PATCH 03/16] baseobjspace: read the instance dict through finditem on the surrogate getattr path `object_getattribute_surrogate` probed the module dict and the instance dict with `w_dict_lookup`, whose object-strategy leaf is `..._checked(..).unwrap_or(None)`. A stored non-string key whose hash collides with the name can run a user `__eq__`; the raising comparison was swallowed and the attribute read back as absent. b62b1be9dba closed that for names with a `&str` view; the lone-surrogate arm kept the swallowing spelling. S = "z\udcffz" class R: def __hash__(self): return hash(S) def __eq__(self, o): raise ValueError("boom") o = C(); o.__dict__[R()] = 1 getattr(o, S) AttributeError before ValueError: boom after, matching python3.14 Take `finditem` -- `space.finditem`, the same helper `finditem_str` and the devolved terminator already use -- so the pending key error becomes the raised exception. Both the devolved and the small instance-dict shapes were affected. Add `pyre/extra_tests/parity_tests/mapdict_devolved_raising_eq.py`, which covers the devolved probe for both name kinds, the non-raising colliding-key control, and the plain lone-surrogate attribute round trip. b62b1be9dba and 41bac542c18 landed without a fixture; this is the one that would have caught both of the above. check.py: dynasm 391/391, cranelift 391/391, wasm 387/387. `cargo test --all --no-default-features --features dynasm`: 0 failed. parity_tests: all pass, new file cpython=OK dynasm=OK cranelift=OK. Assisted-by: Claude --- .../mapdict_devolved_raising_eq.py | 137 ++++++++++++++++++ pyre/pyre-interpreter/src/baseobjspace.rs | 8 +- 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/mapdict_devolved_raising_eq.py diff --git a/pyre/extra_tests/parity_tests/mapdict_devolved_raising_eq.py b/pyre/extra_tests/parity_tests/mapdict_devolved_raising_eq.py new file mode 100644 index 00000000000..b90dd29ce55 --- /dev/null +++ b/pyre/extra_tests/parity_tests/mapdict_devolved_raising_eq.py @@ -0,0 +1,137 @@ +"""A devolved instance dictionary probed by attribute name. + +The probe compares the name against whatever each colliding bucket holds, so a +stored non-string key can reach a user `__eq__`. When that raises, the read +must propagate the exception; reporting it as a miss would silently let a class +attribute of the same name answer instead. + +The lone-surrogate blocks exercise the other half of the name dispatch: a name +that is not valid UTF-8 has no borrowed-str view, so it wraps before probing. +""" + + +class Colliding: + """Hashes as `"zz"` and refuses to compare.""" + + def __hash__(self): + return hash("zz") + + def __eq__(self, other): + raise ValueError("boom") + + +class Quiet: + """Hashes as `"zz"` and compares unequal without raising.""" + + def __hash__(self): + return hash("zz") + + def __eq__(self, other): + return NotImplemented + + +def devolve(obj): + """Grow the instance dict past the mapdict limit, then return it.""" + for i in range(200): + setattr(obj, "a%d" % i, i) + return obj.__dict__ + + +class R: + zz = "CLASSVALUE" + + +# A raising comparison in the probe surfaces, and the class attribute does not +# win by default. +r = R() +devolve(r)[Colliding()] = 1 +try: + r.zz +except ValueError as exc: + assert str(exc) == "boom", str(exc) +else: + raise AssertionError("a raising __eq__ in the probe was reported as a miss") + +# getattr and __getattribute__ reach the same probe. +for read in (lambda o: getattr(o, "zz"), lambda o: type(o).__getattribute__(o, "zz")): + try: + read(r) + except ValueError: + pass + else: + raise AssertionError("raising __eq__ swallowed on an alternate read path") + +# The dict subscript itself agrees. +try: + r.__dict__["zz"] +except ValueError: + pass +except KeyError: + raise AssertionError("raising __eq__ reported as a missing key") + +# A colliding key that compares unequal without raising is an ordinary miss, so +# the class attribute answers. +q = R() +devolve(q)[Quiet()] = 1 +assert q.zz == "CLASSVALUE" + +# An instance attribute still wins over the class attribute after devolving. +own = R() +devolve(own) +own.zz = "OWN" +assert own.zz == "OWN" +own.__dict__[Quiet()] = 1 +assert own.zz == "OWN" + +# A builtin subclass reaches the same terminator. +import _random # noqa: E402 + + +class Rand(_random.Random): + zz = "CLASSVALUE" + + +rand = Rand() +devolve(rand)[Colliding()] = 1 +try: + rand.zz +except ValueError: + pass +else: + raise AssertionError("raising __eq__ swallowed on a builtin subclass") + +# A lone-surrogate attribute name takes the wrapping arm of the name dispatch. +SURROGATE = "z\udcffz" + + +class S: + pass + + +s = S() +devolve(s) +setattr(s, SURROGATE, "SURR") +assert getattr(s, SURROGATE) == "SURR" +assert s.__dict__[SURROGATE] == "SURR" + +# ... and it propagates a raising comparison too. `Colliding` hashes as "zz", +# so pick a colliding key for this name instead. +class CollidingSurrogate: + def __hash__(self): + return hash(SURROGATE) + + def __eq__(self, other): + raise ValueError("boom") + + +s2 = S() +devolve(s2) +s2.__dict__[CollidingSurrogate()] = 1 +try: + getattr(s2, SURROGATE) +except ValueError: + pass +except AttributeError: + raise AssertionError("raising __eq__ reported as a missing attribute") + +print("OK") diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 988f023cc76..f78d69d233f 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -6136,7 +6136,7 @@ pub(crate) unsafe fn object_getattribute_surrogate( if is_module(obj) { let w_dict = pyre_object::w_module_get_w_dict(obj); if !w_dict.is_null() { - if let Some(v) = pyre_object::w_dict_lookup(w_dict, w_name) { + if let Some(v) = finditem(w_dict, w_name)? { if !v.is_null() { return Ok(v); } @@ -6206,9 +6206,13 @@ pub(crate) unsafe fn object_getattribute_surrogate( } } } + // `space.finditem`, not the raw lookup: the probe compares the name + // against whatever each colliding bucket holds, so a stored non-string + // key can run a user `__eq__` that raises, and the swallowing spelling + // would read that back as an absent attribute. let w_dict = getdict_backing(obj)?; if !w_dict.is_null() { - if let Some(v) = pyre_object::w_dict_lookup(w_dict, w_name) { + if let Some(v) = finditem(w_dict, w_name)? { if !v.is_null() { return Ok(v); } From 71ea0acde2412901b4585fc69f77868bbcc89988 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 01:35:03 +0900 Subject: [PATCH 04/16] majit: fold a virtual's never-stored field read to the zero constant optimize_getfield_gc answered GETFIELD_GC on a virtual only when the trace had already stored that field; an unset field fell through to PassOn and the load was emitted, surviving to the arg-forcing pass, which materialized the very virtual it read. virtualize.py:184-193 substitutes `optimizer.new_const(fielddescr)` when `opinfo.getfield` returns None, with optimizer.py:528-534 choosing CONST_NULL / CONST_ZERO_FLOAT / CONST_0 by field kind. Port that fallback. typeptr, w_class and the GETFIELD_RAW_* opcodes stay out of it: the first two are header fields the same function already resolves from class identity, and upstream defines this handler for GETFIELD_GC_{I,R,F} only. The fold does not add an assumption the rest of the optimizer lacks -- virtualstate.py:171-174 tolerates a None fieldstate and info.py:216-226 `_force_elements` emits no SETFIELD for a None field, so upstream already depends on the allocation being zeroed. pytraceback.rs reads an exception's traceback slot before writing it, so every raise emitted that load and dragged the exception, its args list and the traceback node out of virtual state. Measured on pyre/bench/synth/type_immutable_reject.py (MAJIT_LOG=1, same tree, rebuilt both ways, `git checkout HEAD -- virtualize.rs` for the control and restored by sha256): compiled loop 108 -> 128 ops ==> 108 -> 42 ops forced virtuals 12 ==> 0 NewWithVtable/NewArrayClear 16 ==> 0 CallMallocNursery 9 ==> 0 SetfieldGc 66 ==> 4 CallR 4 ==> 0 Both print 400000. check.py exec time on the exception family, dynasm / cranelift / wasm, from the two full runs: type_immutable_reject 0.08/0.08/0.11 -> 0.05/0.06/0.07 exception_value_op_caught 0.11/0.13/0.14 -> 0.06/0.06/0.07 exception_escape_hot_callee_tb_node_once 0.34/0.42/0.47 -> 0.25/0.29/0.34 exception_const_operand_resume 0.12/0.13/0.15 -> 0.09/0.10/0.12 Single runs of the same harness on the same machine, not min-of-rounds; the load-independent signal is the allocation count above. No .jitstats baseline moved: check.py dynasm 391/391, cranelift 391/391, wasm 387/387. `cargo test --all --no-default-features --features dynasm`: 0 failed. Assisted-by: Claude --- majit/majit-metainterp/src/optimizeopt/virtualize.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/majit/majit-metainterp/src/optimizeopt/virtualize.rs b/majit/majit-metainterp/src/optimizeopt/virtualize.rs index 118c61f4bfc..ace0a657de8 100644 --- a/majit/majit-metainterp/src/optimizeopt/virtualize.rs +++ b/majit/majit-metainterp/src/optimizeopt/virtualize.rs @@ -983,6 +983,13 @@ impl OptVirtualize { // (`pytraceback.rs:462`) escapes with its args list and the // traceback node behind it. // + // Reaching here means `field_val` was `None` and neither header + // arm answered. `virtualstate.py:171-174` tolerates a `None` + // fieldstate and `info.py:216-226 _force_elements` emits no + // SETFIELD for a `None` field, so upstream itself depends on the + // allocation being zeroed -- the fold does not add an assumption + // the rest of the optimizer lacks. + // // `w_class` and `typeptr` are excluded: both are header fields // resolved from class identity above, and neither is ever zero on // a live object, so folding them to null/0 would answer a read From 1117c33df88137b85a8abb4a2ed3c952ceda13d2 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 10:22:43 +0900 Subject: [PATCH 05/16] majit: resolve a virtual's getfield slot against the PtrInfo descr `optimize_getfield_gc` addressed a virtual's field list with `field_descr.index_in_parent()` and never checked that the slot it landed on describes the field being read. `optimize_setfield_gc` has checked the same pairing since #1072, but only on the write side, so a descr spelling that appears solely on reads never reached it. `PYFRAME_VABLE_TOKEN_FIELD_DESCR` is such a spelling: it describes `PyFrame.vable_token` at offset 80 with a placeholder `index_in_parent: 0`, because the positional census that assigns the indices does not list the field -- upstream carries it as `rvirtualizable.py:29`'s appended `('vable_token', llmemory.GCREF)` and pyre registers it only as an extra GC edge. Slot 0 of that layout is `PyFrame.locals_cells_stack_w`, so the read returned the locals array pointer as the frame's token. Add `field_slot_identifies`, the release-live half of the existing `field_slot_disagreement` (both now share `slot_holds_field`). A read whose slot does not hold the field takes the `virtualize.py:188` zeroed-allocation fold instead of the slot's value; the write side keeps its panic. Logged as `[jit][getfield-slot-unlisted]` under `MAJIT_LOG=1`. Also port `info.py:212-213`: upstream's `getfield` opens with the same `init_fields(fielddescr.get_parent_descr(), fielddescr.get_index())` that `setfield` does, which is what upgrades `vinfo.descr` to a more precise subclass descr. pyre's read side omitted the call. A release build with `-C debug-assertions=on` over the check.py corpus reported 66 hits across 33 benches, all of them this one descr, all from `compile_loop_body`. `MAJIT_LOG=1` confirmed the aliased slot was populated. Assisted-by: Claude --- .../src/optimizeopt/virtualize.rs | 144 ++++++++++++++++-- 1 file changed, 129 insertions(+), 15 deletions(-) diff --git a/majit/majit-metainterp/src/optimizeopt/virtualize.rs b/majit/majit-metainterp/src/optimizeopt/virtualize.rs index ace0a657de8..12101395fb2 100644 --- a/majit/majit-metainterp/src/optimizeopt/virtualize.rs +++ b/majit/majit-metainterp/src/optimizeopt/virtualize.rs @@ -771,7 +771,7 @@ impl OptVirtualize { if let Some(err) = field_slot_disagreement(&vinfo.descr, field_idx, field_descr) { - panic!("Virtual {err}"); + panic!("Virtual setfield: {err}"); } Some(OptimizationResult::Remove) } @@ -780,7 +780,7 @@ impl OptVirtualize { if let Some(err) = field_slot_disagreement(&vinfo.descr, field_idx, field_descr) { - panic!("VirtualStruct {err}"); + panic!("VirtualStruct setfield: {err}"); } Some(OptimizationResult::Remove) } @@ -848,6 +848,34 @@ impl OptVirtualize { return OptimizationResult::PassOn; } + // info.py:212-213 `getfield` opens with the same + // `init_fields(fielddescr.get_parent_descr(), fielddescr.get_index())` + // that `setfield` does, so upstream's read is what grows `_fields` and + // swaps in the more precise descr (info.py:184-188) when the index + // belongs to a subclass the allocation's descr does not cover. pyre + // keys fields by slot instead of indexing an array, so the read needed + // nothing to answer and the call was dropped; `vinfo.descr` then stayed + // at whatever the allocation set. That descr is what + // `field_slot_disagreement` below reads, so the upgrade has to happen + // for the slot it checks to be the slot upstream would have used. + // + // Only for a virtual: `virtualize.py:185-186` reaches `opinfo.getfield` + // under `opinfo.is_virtual()`, and a non-virtual info's descr is + // `OptHeap`'s to move (`optimizer.py:484`). The header reads are + // excluded for the reason the arms below give -- they do not resolve + // through the field list at all. + if !is_raw_op && !is_typeptr && !field_descr.is_w_class() { + if let (Some(b), Some(parent_descr)) = + (struct_box.as_ref(), field_descr.get_parent_descr()) + { + ctx.with_ptr_info_mut(b, |info| { + if info.is_virtual() { + info.init_fields(parent_descr, field_idx as usize); + } + }); + } + } + if let Some(info) = struct_box.as_ref().and_then(|b| ctx.peek_ptr_info(b)) { // info.py:212-214 getfield: return _fields[fielddescr.get_index()]. // For Virtual, ob_type (typeptr) is not in fields — fold from @@ -929,7 +957,64 @@ impl OptVirtualize { // slots by `field_slot_index`, so the two do not meet. Removing // that split is the prerequisite for folding this read at all. } + // `optimize_setfield_gc` panics on a slot its descr does not + // identify, but a spelling that only ever appears on reads never + // reaches that check, and this is the side that resolves it. + // + // It does reach here. `PYFRAME_VABLE_TOKEN_FIELD_DESCR` + // (`pyre-jit-trace descr.rs`) describes `PyFrame.vable_token` at + // its byte offset with a placeholder `index_in_parent: 0` and no + // parent, because the positional census that assigns the real + // indices deliberately does not list the field -- upstream carries + // it as `rvirtualizable.py:29`'s appended `('vable_token', + // llmemory.GCREF)` and pyre registers it as an extra GC edge so + // `clear_gc_fields` zeroes it. Slot 0 of that layout is + // `PyFrame.locals_cells_stack_w`, so `field_idx` addressed the + // locals array and `get_field` forwarded a live array pointer as + // the frame's token; `emit_force_virtualizable` reads that token + // with GETFIELD_GC_R to decide whether the frame is JIT-owned, and + // a non-null pointer reads as owned on a frame that has no token. + // + // A field the positional list does not hold cannot have been + // stored under its own identity either, so this is exactly + // `virtualize.py:188`'s state: the trace never stored it and the + // read answers the zeroed allocation. Skip the slot lookup and + // take the zero fold below -- which for `vable_token` is the + // correct value, a virtual frame having never been forced. + // + // Not gated on `debug_assertions`: the resolution it guards runs in + // release, so the guard has to. `Virtualizable` is not covered -- + // its fields come from the state-field JIT's own descr set, which + // `vstate.descr` does not index. + let slot_identifies_field = match &info { + PtrInfo::Virtual(vinfo) => { + field_slot_identifies(&vinfo.descr, field_idx, field_descr) + } + PtrInfo::VirtualStruct(vinfo) => { + field_slot_identifies(&vinfo.descr, field_idx, field_descr) + } + _ => true, + }; + let slot_resolvable = + slot_identifies_field || is_raw_op || is_typeptr || field_descr.is_w_class(); + if !slot_resolvable && crate::majit_log_enabled() { + // What the skip is worth: a populated slot is the value + // `get_field` would have forwarded for a field not in it. + let populated = match &info { + PtrInfo::Virtual(vinfo) => get_field(&vinfo.fields, field_idx).is_some(), + PtrInfo::VirtualStruct(vinfo) => get_field(&vinfo.fields, field_idx).is_some(), + _ => false, + }; + eprintln!( + "[jit][getfield-slot-unlisted] field {:?} at offset {} does not hold slot \ + {field_idx} of the virtual's descr (slot populated: {populated}); folding \ + to the zeroed allocation", + field_descr.field_name(), + field_descr.offset(), + ); + } let field_val = match &info { + _ if !slot_resolvable => None, PtrInfo::Virtual(vinfo) => get_field(&vinfo.fields, field_idx), PtrInfo::VirtualStruct(vinfo) => get_field(&vinfo.fields, field_idx), PtrInfo::Virtualizable(vstate) => vstate @@ -2417,7 +2502,8 @@ impl Optimization for OptVirtualize { /// Returns the disagreement as a message so the caller's panic names both the /// slot and the field. Compiled out of release builds: it is a debug assertion, /// written as a function only because the message needs the same walk the -/// predicate does. +/// predicate does. [`field_slot_identifies`] is the same walk without the +/// message, for the read side, which has to answer in release too. fn field_slot_disagreement( descr: &DescrRef, field_idx: u32, @@ -2430,22 +2516,12 @@ fn field_slot_disagreement( let Some(slot) = fields.get(field_idx as usize) else { return Some(format!( "field slot {field_idx} is outside its own descr's field list (len {}, descr \ - index {}); `set_field` just wrote past the struct this PtrInfo describes", + index {}); the slot is past the end of the struct this PtrInfo describes", fields.len(), descr.index(), )); }; - // Both halves must agree. The name is the better key but is not always - // carried — the flattened inline aggregates (`ob_header`, an enum's - // `__pos_0`) reach here under the documented empty-name fallback — so the - // name is compared only when both sides have one, and the offset is - // compared always. Neither alone is sufficient: a name can be absent, and a - // flattened layout puts an aggregate and its first leaf at one address - // (`heaptracker.py:68-69`). - let named_apart = !field.field_name().is_empty() - && !slot.field_name().is_empty() - && slot.field_name() != field.field_name(); - if named_apart || slot.offset() != field.offset() { + if !slot_holds_field(slot.as_ref(), field) { return Some(format!( "field {:?} at offset {} claims slot {field_idx} of descr index {}, but that \ slot holds {:?} at offset {}", @@ -2459,6 +2535,44 @@ fn field_slot_disagreement( None } +/// Whether `slot` and `field` name the same field. +/// +/// Both halves must agree. The name is the better key but is not always +/// carried — the flattened inline aggregates (`ob_header`, an enum's `__pos_0`) +/// reach here under the documented empty-name fallback — so the name is +/// compared only when both sides have one, and the offset is compared always. +/// Neither alone is sufficient: a name can be absent, and a flattened layout +/// puts an aggregate and its first leaf at one address +/// (`heaptracker.py:68-69`). +fn slot_holds_field(slot: &dyn FieldDescr, field: &dyn FieldDescr) -> bool { + let named_apart = !field.field_name().is_empty() + && !slot.field_name().is_empty() + && slot.field_name() != field.field_name(); + !named_apart && slot.offset() == field.offset() +} + +/// Whether `field_idx` addresses `field` in the struct `descr` describes. +/// +/// The read side's release-live half of [`field_slot_disagreement`]. The write +/// side can panic on a disagreement because a wrong store is unrecoverable; a +/// read has a correct answer available — a field the slot list does not hold +/// was never stored under its own identity, so `virtualize.py:188`'s zeroed +/// allocation is what it reads — so it answers that instead of aborting, and +/// has to be able to answer it in a release build. +/// +/// A descr that is not a size descr answers `true`: the caller has no field +/// list to check against, which is the state every pre-existing read was +/// resolved in and not a disagreement this can see. +fn field_slot_identifies(descr: &DescrRef, field_idx: u32, field: &dyn FieldDescr) -> bool { + let Some(size_descr) = descr.as_size_descr() else { + return true; + }; + size_descr + .all_fielddescrs() + .get(field_idx as usize) + .is_some_and(|slot| slot_holds_field(slot.as_ref(), field)) +} + fn set_field(fields: &mut Vec<(u32, Operand)>, field_idx: u32, value: Operand) { for entry in fields.iter_mut() { if entry.0 == field_idx { From 0da31b812df199ef4a2022512d2221f332beca1d Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 10:22:51 +0900 Subject: [PATCH 06/16] jit: classify the walker's cross-loop close through classify_compile_outcome The walker's loop-header close read its `CompileOutcome` as `Compiled` / not-`Compiled` and recorded every non-`Compiled` result as a declined close, which is latched in `TraceCtx::declined_cross_loop_closes` and never retried for that header. `MetaInterp::classify_compile_outcome` keeps three states apart there, and the sibling close in `jitdriver.rs:2947` already goes through it. Route this close through it too: `RetraceNeeded` no longer latches, since the attempt armed `partial_trace` and the next visit of the header takes the `has_partial` arm regardless. Track whether an attempt was made: the give-up arm that returns `Cancelled` without calling `compile_trace` costs no optimizer pass and is excluded from the latch. `classify_compile_outcome` becomes `pub` and `BridgeCompileResult` is re-exported from `majit_metainterp`. Assisted-by: Claude --- majit/majit-metainterp/src/lib.rs | 12 +-- majit/majit-metainterp/src/pyjitpl.rs | 2 +- .../src/jitcode_dispatch/mod.rs | 77 +++++++++++++------ 3 files changed, 62 insertions(+), 29 deletions(-) diff --git a/majit/majit-metainterp/src/lib.rs b/majit/majit-metainterp/src/lib.rs index 33c206b3b4c..248c8633b4c 100644 --- a/majit/majit-metainterp/src/lib.rs +++ b/majit/majit-metainterp/src/lib.rs @@ -147,12 +147,12 @@ pub use parity::{TraceParityCase, assert_trace_parity, normalize_ops, normalize_ /// pool-side resolution without re-deriving a second copy of the logic. pub use pyjitpl::dispatch::field_descr_ref_from_bh; pub use pyjitpl::{ - BackEdgeAction, BridgeRetraceResult, ClosureRuntime, ClosureRuntimeWithResolver, - CompileOutcome, CompiledExitLayout, CompiledTerminalExitLayout, CompiledTraceLayout, - DeadFrameArtifacts, DetailedDriverRunOutcome, InlineDecision, JitCodeMachine, JitCodeRuntime, - JitCodeSym, JitHooks, JitStats, MIFrame, MIFrameStack, MetaInterp, MetaInterpGlobalData, - MetaInterpStaticData, RawCompileResult, StandaloneFrameStack, build_state_field_snapshot, - call_int_function, call_ref_function, call_void_function, counters, + BackEdgeAction, BridgeCompileResult, BridgeRetraceResult, ClosureRuntime, + ClosureRuntimeWithResolver, CompileOutcome, CompiledExitLayout, CompiledTerminalExitLayout, + CompiledTraceLayout, DeadFrameArtifacts, DetailedDriverRunOutcome, InlineDecision, + JitCodeMachine, JitCodeRuntime, JitCodeSym, JitHooks, JitStats, MIFrame, MIFrameStack, + MetaInterp, MetaInterpGlobalData, MetaInterpStaticData, RawCompileResult, StandaloneFrameStack, + build_state_field_snapshot, call_int_function, call_ref_function, call_void_function, counters, record_application_traceback_for_recording, record_application_traceback_hook_address, record_discarded_level_traceback_for_recording, record_discarded_level_traceback_hook_address, record_inline_application_traceback_for_recording, diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 64c785c7684..3b15fca26d1 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -11083,7 +11083,7 @@ impl MetaInterp { /// bridge (`close_bridge`) and the interp-origin entry bridge /// (`compile_trace_from_interp`) alike, since `retrace_after_bridge` is armed /// inside the shared compile path rather than per origin. - pub(crate) fn classify_compile_outcome(&self, outcome: CompileOutcome) -> BridgeCompileResult { + pub fn classify_compile_outcome(&self, outcome: CompileOutcome) -> BridgeCompileResult { match outcome { CompileOutcome::Compiled { .. } => BridgeCompileResult::Compiled, _ if self.retrace_after_bridge => { diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 786cedadca3..9603b0de005 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -11262,15 +11262,22 @@ fn handle( .bridge_info() .map(|b| (b.trace_id, b.fail_index)); let has_targets = driver.meta_interp().has_compiled_targets(key); - // A close that did not compile is not retried on a later + // A close an attempt *rejected* is not retried on a later // crossing of the same header: the attempt runs the optimizer // over the whole trace-so-far, and the decline is deterministic, // so an inner loop crossed N times would pay N optimizer passes // over a growing trace (see - // `TraceCtx::declined_cross_loop_closes`). + // `TraceCtx::declined_cross_loop_closes`). Which outcomes count + // as rejected is the `classify_compile_outcome` match below. let already_declined = ctx.trace_ctx.cross_loop_close_declined(key); if !has_partial && has_targets { if !already_declined { + // jitdriver.rs:2981-2988 states the rule the latch runs + // under: only latch what an attempt actually rejected. + // The give-up below returns without calling into + // `compile_trace` at all, so it costs no optimizer pass + // and has nothing to report about this header. + let mut attempted = true; let outcome = match bridge_origin { // Guard-origin: existing bridge path. Some(_) => driver.meta_interp_mut().compile_trace( @@ -11321,31 +11328,57 @@ fn handle( // halves, so this is unreachable from a jd0 walk; only // `MetaInterp::force_start_tracing` opens a tracer with // no session envelope. - None => majit_metainterp::CompileOutcome::Cancelled, + None => { + attempted = false; + majit_metainterp::CompileOutcome::Cancelled + } }, }; - if matches!(outcome, majit_metainterp::CompileOutcome::Compiled { .. }) { - if majit_metainterp::majit_log_enabled() { - eprintln!( - "[jit][walker-reached-loop-header] compile_trace success: \ + // pyjitpl.py:2982-2983 `classify_compile_outcome` is + // the shared reading of a close's outcome; the sibling + // close in `jitdriver.rs:2947` already goes through it. + // Reading `Compiled` here and treating every other + // outcome as one declined close collapses three states + // the classifier keeps apart, and latches the one that + // is explicitly retryable. + match driver.meta_interp().classify_compile_outcome(outcome) { + majit_metainterp::BridgeCompileResult::Compiled => { + if majit_metainterp::majit_log_enabled() { + eprintln!( + "[jit][walker-reached-loop-header] compile_trace success: \ key={} pc={} bridge={:?}", - key, next_instr, bridge_origin - ); + key, next_instr, bridge_origin + ); + } + // pyjitpl.py raise_if_successful() — the + // successful compile_trace ends tracing; surface + // the dedicated outcome so the driver maps it to + // `TraceAction::CompileTrace` (no further compile + // or abort on this session). + driver.note_compile_trace_success(); + return Ok(( + DispatchOutcome::CompileTracePending { + loop_header_pc: next_instr, + }, + op.next_pc, + )); + } + // pyjitpl.py:3000, jitdriver.rs:2967-2974: the + // attempt armed `partial_trace`, so the next visit + // of this header takes the `has_partial` arm of the + // gate above and runs no optimizer pass either way. + // Once the retrace lands, the state this close was + // refused against is gone -- latching it would + // refuse the close forever for a reason that has + // already stopped holding. + majit_metainterp::BridgeCompileResult::RetraceNeeded => {} + majit_metainterp::BridgeCompileResult::Declined + | majit_metainterp::BridgeCompileResult::Failed => { + if attempted { + ctx.trace_ctx.note_cross_loop_close_declined(key); + } } - // pyjitpl.py raise_if_successful() — the - // successful compile_trace ends tracing; surface - // the dedicated outcome so the driver maps it to - // `TraceAction::CompileTrace` (no further compile - // or abort on this session). - driver.note_compile_trace_success(); - return Ok(( - DispatchOutcome::CompileTracePending { - loop_header_pc: next_instr, - }, - op.next_pc, - )); } - ctx.trace_ctx.note_cross_loop_close_declined(key); } // The jump did not take (`compile.compile_trace` returns // None when none of the existing loop tokens match). Fall From d4e3d7f834802c056c5a97c4fc0a1ced6302e7e9 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 10:23:00 +0900 Subject: [PATCH 07/16] exceptions: keep lone surrogates out of the lossy UTF-8 path Two sites turned an unpaired surrogate into U+FFFD. `error.rs` `write_plain_exception_object` and `write_exception_group` wrote `render_rooted_exc_object_wtf8`'s buffer straight into their byte buffers. `sys.stderr` carries `errors='backslashreplace'`, so the surrogate owes the six-character `\udcXX` escape on the way out; the raw WTF-8 bytes behind it are not valid UTF-8. `render_exc_object` already spent that encode. Extract it as `display::wtf8_display_string` and add `error.rs::render_rooted_exc_object_display` for the two writers. `builtins.rs` `exception_group_str` built its result as `format!("{} ({count} sub-exception{suffix})", message.to_string_lossy())`. `app_group.py:88-90` interpolates `self.message` with no encode, so `str(group)` disagreed with `group.message` -- a loss visible from Python, not only on stderr. Build the result as a `Wtf8Buf` instead. Add `surrogate_traceback_render.py`. The parity harness checks only exit 0 and a final `OK`, so the two stderr-shape checks self-spawn through `subprocess.run(capture_output=True)` and assert on bytes; the other two assert the exception values keep the code point. Assisted-by: Claude --- .../surrogate_traceback_render.py | 90 +++++++++++++++++++ pyre/pyre-interpreter/src/builtins.rs | 13 ++- pyre/pyre-interpreter/src/display.rs | 28 ++++-- pyre/pyre-interpreter/src/error.rs | 27 +++--- 4 files changed, 135 insertions(+), 23 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/surrogate_traceback_render.py diff --git a/pyre/extra_tests/parity_tests/surrogate_traceback_render.py b/pyre/extra_tests/parity_tests/surrogate_traceback_render.py new file mode 100644 index 00000000000..c02d8e34b11 --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_traceback_render.py @@ -0,0 +1,90 @@ +"""An unpaired surrogate in a traceback reaches stderr as a backslash escape. + +`sys.stderr` is opened with `errors='backslashreplace'`, so an attribute name +carrying a lone surrogate prints as the six-character `\\udcff` escape. Writing +the three raw WTF-8 bytes behind that code point instead is not the same output +and is not valid UTF-8, so the difference is visible to any consumer that +decodes the stream. + +The exception value itself keeps the surrogate: `str(e)` holds the real code +point, and only the encode on the way out spends the escape. +""" + +import subprocess +import sys + +SCRIPT = r''' +S = "z\udcffz" + + +class C: + pass + + +getattr(C(), S) +''' + +EXPECTED = rb"AttributeError: 'C' object has no attribute 'z\udcffz'" + + +def check_uncaught_render(): + proc = subprocess.run( + [sys.executable, "-c", SCRIPT], + capture_output=True, + ) + assert proc.returncode == 1, proc.returncode + lines = proc.stderr.splitlines() + assert lines, proc.stderr + assert lines[-1] == EXPECTED, lines[-1] + # The escape is the only thing on that line that is not plain ASCII: a raw + # surrogate would have left three bytes above 0x7f behind. + assert lines[-1].isascii(), lines[-1] + + +def check_group_render(): + script = ( + 'raise ExceptionGroup("g\\udcffg", [ValueError("v\\udcffv")])\n' + ) + proc = subprocess.run([sys.executable, "-c", script], capture_output=True) + assert proc.returncode == 1, proc.returncode + err = proc.stderr + assert err.isascii(), err + assert rb"g\udcffg" in err, err + assert rb"v\udcffv" in err, err + + +def check_group_str_keeps_the_code_point(): + # `BaseExceptionGroup.__str__` interpolates `self.message`, and a message + # carrying a lone surrogate interpolates as itself. Assembling the result + # through a lossy UTF-8 encode instead substitutes U+FFFD, which makes + # `str(group)` disagree with `group.message` -- a loss visible here, not + # only on the way to stderr. + message = "g\udcffg" + group = ExceptionGroup(message, [ValueError("v")]) + assert group.message == message, ascii(group.message) + assert str(group) == message + " (1 sub-exception)", ascii(str(group)) + assert "�" not in str(group), ascii(str(group)) + + +def check_value_keeps_the_code_point(): + S = "z\udcffz" + + class C: + pass + + try: + getattr(C(), S) + except AttributeError as e: + assert e.name == S, ascii(e.name) + # The real code point, not the six characters that spell its escape. + assert str(e) == "'C' object has no attribute '%s'" % S, ascii(str(e)) + assert "\\udcff" not in str(e), ascii(str(e)) + else: + raise AssertionError("no AttributeError") + + +check_uncaught_render() +check_group_render() +check_group_str_keeps_the_code_point() +check_value_keeps_the_code_point() +print("OK") diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index df5e67614be..53815215edf 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -8634,10 +8634,15 @@ fn exception_group_str(args: &[PyObjectRef]) -> Result Result { diff --git a/pyre/pyre-interpreter/src/display.rs b/pyre/pyre-interpreter/src/display.rs index 6f1a0bc5821..09a34371362 100644 --- a/pyre/pyre-interpreter/src/display.rs +++ b/pyre/pyre-interpreter/src/display.rs @@ -1306,17 +1306,29 @@ pub unsafe fn py_str_display(obj: PyObjectRef) -> String { Ok(w) => w, Err(_) => return "".to_string(), }; - if let Ok(s) = w.as_str() { - return s.to_owned(); - } - let s_obj = pyre_object::w_str_from_wtf8(w); - crate::type_methods::encode_object(s_obj, "utf-8", "backslashreplace") - .ok() - .and_then(|b| String::from_utf8(b).ok()) - .unwrap_or_else(|| "".to_string()) + wtf8_display_string(w, "") } } +/// The text a WTF-8 diagnostic becomes on the way to stderr. +/// +/// `sys.stderr` carries `errors='backslashreplace'`, so an unpaired surrogate +/// leaves as the six characters `\udcXX` rather than as the three WTF-8 bytes +/// behind it — which are not valid UTF-8 and would reach a consumer as +/// replacement characters. Every diagnostic assembled as a `Wtf8Buf` owes that +/// encode before it is written; `fallback` names the caller's placeholder for +/// the encode itself failing. +pub(crate) fn wtf8_display_string(rendered: Wtf8Buf, fallback: &str) -> String { + if let Ok(s) = rendered.as_str() { + return s.to_owned(); + } + let s_obj = pyre_object::w_str_from_wtf8(rendered); + crate::type_methods::encode_object(s_obj, "utf-8", "backslashreplace") + .ok() + .and_then(|b| String::from_utf8(b).ok()) + .unwrap_or_else(|| fallback.to_string()) +} + /// The encoded length of the character a WTF-8 lead byte opens. /// /// A stray continuation byte answers 1: the buffer is malformed, and stepping diff --git a/pyre/pyre-interpreter/src/error.rs b/pyre/pyre-interpreter/src/error.rs index 02a70109595..307f03ea36d 100644 --- a/pyre/pyre-interpreter/src/error.rs +++ b/pyre/pyre-interpreter/src/error.rs @@ -2029,7 +2029,7 @@ fn write_plain_exception_object( { write_syntax_error_object(&mut rendered, exc)?; } else { - let header = render_rooted_exc_object_wtf8(exc_slot); + let header = render_rooted_exc_object_display(exc_slot); rendered.write_all(header.as_bytes())?; rendered.write_all(b"\n")?; } @@ -2096,7 +2096,7 @@ fn write_exception_group( } let mut header = Vec::new(); - let group_header = render_rooted_exc_object_wtf8(exc_slot); + let group_header = render_rooted_exc_object_display(exc_slot); header.write_all(group_header.as_bytes())?; header.write_all(b"\n")?; let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); @@ -2366,15 +2366,20 @@ fn notes_is_abc_sequence(notes_slot: usize) -> bool { /// Compose the `ExcName: msg` header for a W_BaseException — /// equivalent to `traceback.format_exception_only`'s last line. fn render_exc_object(exc: PyObjectRef) -> String { - let rendered = render_exc_object_wtf8(exc); - if let Ok(s) = rendered.as_str() { - return s.to_owned(); - } - let s_obj = pyre_object::w_str_from_wtf8(rendered); - crate::type_methods::encode_object(s_obj, "utf-8", "backslashreplace") - .ok() - .and_then(|b| String::from_utf8(b).ok()) - .unwrap_or_else(|| "".to_string()) + crate::display::wtf8_display_string(render_exc_object_wtf8(exc), "") +} + +/// [`render_rooted_exc_object_wtf8`] as the bytes stderr should receive. +/// +/// The callers that assemble the report into a byte buffer have to spend the +/// `backslashreplace` encode themselves; writing the `Wtf8Buf` straight out +/// puts the raw surrogate bytes on the stream, which is not what +/// `errors='backslashreplace'` produces and is not valid UTF-8. +fn render_rooted_exc_object_display(exc_slot: usize) -> String { + crate::display::wtf8_display_string( + render_rooted_exc_object_wtf8(exc_slot), + "", + ) } fn render_rooted_exc_object_wtf8(exc_slot: usize) -> Wtf8Buf { From 6134a52791e22878255006d66b00e4f0f0857742 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 11:01:11 +0900 Subject: [PATCH 08/16] exceptions, format: rebuild str/repr results as WTF-8, not through a String MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `display::py_str` and `py_repr` are their `_wtf8` twins plus a lossy UTF-8 encode. Five entry points took that spelling and immediately minted a Python `str` from the result, so a lone surrogate in the value became U+FFFD: - `object.__format__` (typedef.rs) and `builtin_value_format` (type_methods.rs) — the empty-spec arm falls through to `str(self)`, which is the `f"{obj}"` / `format(obj)` path for any instance. `builtin_value_format` already had a WTF-8 arm for a `str` receiver; the two branches differed only in the encode, so they collapse into one. - `weakref.proxy.__str__` (interp__weakref.rs) - `mappingproxy.__str__` (typedef.rs) - the `BaseExceptionGroup` constructor's recorded sequence repr (builtins.rs) Widening the last one made `exception_group_repr` panic in `w_str_get_value`, which asserts UTF-8; that function assembled its whole result through `String`, so it moves to WTF-8 with it. `app_group.py:92-93` interpolates the two `!r` results verbatim. `os.getcwd()` returned `String::from_utf8_lossy` / `Path::to_string_lossy` of the directory bytes on both the sandbox and `host_env` arms. `interp_posix.py:906-908` is `space.fsdecode(getcwdb(space))`; use the `gateway` fsdecode helpers, which are what the sibling path entry points already use. Measured against CPython 3.14 with a `__str__` returning `"s\udcffz"`: `f"{obj}"`, `format(obj)` and `str(weakref.proxy(obj))` answered `s�z` before and `s\udcffz` after. `"%s" % obj` and `str(obj)` were already lossless and are kept as controls in the fixture. Add `surrogate_str_roundtrip.py`. A mappingproxy check was written and dropped: a dict's repr escapes surrogates to ASCII, so it cannot fail. Assisted-by: Claude --- .../parity_tests/surrogate_str_roundtrip.py | 66 +++++++++++++++++++ pyre/pyre-interpreter/src/builtins.rs | 29 +++++--- .../src/module/_weakref/interp__weakref.rs | 4 +- .../src/module/posix/interp_posix.rs | 10 +-- pyre/pyre-interpreter/src/type_methods.rs | 14 ++-- pyre/pyre-interpreter/src/typedef.rs | 10 +-- 6 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/surrogate_str_roundtrip.py diff --git a/pyre/extra_tests/parity_tests/surrogate_str_roundtrip.py b/pyre/extra_tests/parity_tests/surrogate_str_roundtrip.py new file mode 100644 index 00000000000..2445dde30cc --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_str_roundtrip.py @@ -0,0 +1,66 @@ +"""A lone surrogate survives str()/repr() round trips through the object protocol. + +A Python `str` may hold an unpaired surrogate -- `surrogateescape` puts one there +for every undecodable filesystem byte -- so any path that rebuilds a `str` from a +`__str__`/`__repr__` result has to carry it. Assembling the result through a +lossy UTF-8 encode instead substitutes U+FFFD, which silently changes the value: +`f"{obj}"` stops equalling `str(obj)`, and two distinct strings compare equal. + +`str()` and `%`-formatting were already lossless here; the entry points below are +the ones that rebuilt the value through a `String`. +""" + +import weakref + +S = "s\udcffz" +FFFD = "�" + + +class Str: + def __str__(self): + return S + + +class Seq: + """A non-list/tuple sequence -- the group constructor saves its repr().""" + + def __repr__(self): + return S + + def __len__(self): + return 1 + + def __getitem__(self, index): + if index == 0: + return ValueError("v") + raise IndexError(index) + + +def check_format(): + # `object.__format__` with an empty spec falls through to `str(self)`. + assert f"{Str()}" == S, ascii(f"{Str()}") + assert format(Str()) == S, ascii(format(Str())) + assert format(Str(), "") == S, ascii(format(Str(), "")) + # The two that were already lossless, kept as controls: if these ever break, + # the cause is upstream of the entry points above. + assert "%s" % Str() == S, ascii("%s" % Str()) + assert str(Str()) == S, ascii(str(Str())) + + +def check_weakref_proxy(): + obj = Str() + proxy = weakref.proxy(obj) + assert str(proxy) == S, ascii(str(proxy)) + + +def check_exception_group_repr(): + group = ExceptionGroup("m", Seq()) + # The constructor records the sequence's repr() and `__repr__` replays it. + assert repr(group) == "ExceptionGroup('m', %s)" % S, ascii(repr(group)) + assert FFFD not in repr(group), ascii(repr(group)) + + +check_format() +check_weakref_proxy() +check_exception_group_repr() +print("OK") diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 53815215edf..ca9d03439e4 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -8153,8 +8153,8 @@ fn exception_group_new(args: &[PyObjectRef]) -> Result Result Result Result { pub fn proxy_str(args: &[PyObjectRef]) -> Result { let w_obj0 = force(args[0])?; - Ok(pyre_object::w_str_new(&unsafe { crate::py_str(w_obj0)? })) + Ok(pyre_object::w_str_from_wtf8(unsafe { + crate::display::py_str_wtf8(w_obj0)? + })) } pub fn proxy_bool(args: &[PyObjectRef]) -> Result { diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index 1b2cb064bfc..cd0a83b8c2c 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -4396,10 +4396,12 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::make_builtin_function_with_arity( "getcwd", |_| { - // interp_posix.py:906 `space.fsdecode(getcwdb(space))`. A - // lossy decode would answer U+FFFD for a byte the surrogate - // escape represents, and the name would no longer name the - // directory it came from. + // `interp_posix.py:906-908` is `space.fsdecode(getcwdb(space))`, + // so the directory's bytes reach Python through the filesystem + // decoder and a byte with no UTF-8 spelling survives as its + // surrogate escape. A lossy decode would fold it to U+FFFD, + // which breaks the `os.fsencode(os.getcwd())` round trip and + // makes two different directories compare equal. #[cfg(feature = "sandbox")] { let cwd = crate::host_seam::ops::getcwd() diff --git a/pyre/pyre-interpreter/src/type_methods.rs b/pyre/pyre-interpreter/src/type_methods.rs index 894f3604dd3..ebdb27498a4 100644 --- a/pyre/pyre-interpreter/src/type_methods.rs +++ b/pyre/pyre-interpreter/src/type_methods.rs @@ -2672,15 +2672,13 @@ pub fn builtin_value_format(args: &[PyObjectRef]) -> Result Date: Fri, 7 Aug 2026 14:24:11 +0900 Subject: [PATCH 09/16] interp: carry text that may hold a lone surrogate as WTF-8 A Python `str` may hold an unpaired surrogate -- `surrogateescape` puts one there for every undecodable filesystem byte -- and `format!` renders a `Wtf8Buf` through `Display`, which substitutes U+FFFD. Every surface below rebuilt a Python-visible value that way. Message channel: - `PyError.message` becomes a `Wtf8Buf` and all 27 constructors take `impl Into`; `message_text()` keeps the display-side reading (backslashreplace) and `message_wtf8()` answers the value. `syntax_error_located` takes a `&Wtf8` filename and pins it verbatim. - `display::wtf8_format!` assembles a message from `str` / `String` / `Wtf8` / `Wtf8Buf` pieces through a `Wtf8Piece` trait. - `py_module!` gains a `Wtf8Buf` return arm (`w_str_from_wtf8`). - `display::py_repr` and `display::py_str`, the `String` wrappers over `py_repr_wtf8` / `py_str_wtf8`, are removed; their callers read the WTF-8 (typedef, _pickle, _csv, _ast, _sre, generic_alias, _contextvars, argument, call, opcode_ops, runtime_ops, type_methods and the rest). OS strings and the import machinery: - `gateway::fsencode_os_str` / `os_string_from_fs_bytes` are the two directions between a host `OsStr` and filesystem bytes. - posix `getcwd` / `getlogin` / `ttyname` / sandbox `getenv` / `strerror`, `create_environ`, `win_nt::arg_path` / `wrap_path` and `_path_splitroot` decode with fsdecode instead of a lossy UTF-8 pass; `split_root` scans the `Wtf8` by code point. - `load_source_module` spells the path once as bytes, so `__file__`, `co_filename` (through `PyCode.filename_bytes`) and the package `__path__` keep the name; `sys.path[0]`, the shadowing hint and `create_sys_path_list` follow. - `decode_source_bytes` takes a `&Wtf8` filename, and a declared codec that yields a surrogate now raises `UnicodeEncodeError` (`typedef::utf8_strict_w`) rather than rewriting the source. - `host_seam::getenv` (non-unix) and `launch_env` keep environment values in the filesystem-bytes spelling. Rendered reports: - A report is assembled as WTF-8 throughout and the sink decides the spelling: `sys.stderr.write` takes the text and its own `errors='backslashreplace'` applies, while `error::emit_report_to_host_stderr` spends that encode for a raw fd. `sys.excepthook` into a `StringIO` now reads the surrogate itself. - `BaseException.__str__`, the SyntaxError writer's filename and msg, `__notes__`, the thread excepthook and `SystemExit`'s printed code follow the same split. Adds `surrogate_name_messages.py`, covering `__qualname__` in `repr(f)` and the binder's three TypeErrors, the ContextVar / Token reprs and the already-used RuntimeError, `sys.excepthook`, and `BaseException.__str__`. Assisted-by: Claude --- Cargo.lock | 1 + .../parity_tests/surrogate_name_messages.py | 117 +++++++ .../src/_pypy_generic_alias.rs | 73 ++-- pyre/pyre-interpreter/src/argument.rs | 46 ++- .../src/astcompiler/validate.rs | 9 +- pyre/pyre-interpreter/src/baseobjspace.rs | 141 +++++--- pyre/pyre-interpreter/src/builtins.rs | 117 ++++--- pyre/pyre-interpreter/src/call.rs | 100 +++--- pyre/pyre-interpreter/src/compile.rs | 38 +- pyre/pyre-interpreter/src/display.rs | 164 ++++++--- pyre/pyre-interpreter/src/error.rs | 328 ++++++++++-------- pyre/pyre-interpreter/src/eval.rs | 15 +- pyre/pyre-interpreter/src/executioncontext.rs | 16 +- pyre/pyre-interpreter/src/function.rs | 24 +- pyre/pyre-interpreter/src/gateway.rs | 46 +++ pyre/pyre-interpreter/src/importing.rs | 172 +++++---- pyre/pyre-interpreter/src/launch_env.rs | 15 +- pyre/pyre-interpreter/src/lib.rs | 9 +- .../src/module/__pypy__/mod.rs | 2 +- .../src/module/_ast/convert.rs | 56 +-- .../src/module/_collections/mod.rs | 6 +- .../src/module/_contextvars/mod.rs | 54 +-- pyre/pyre-interpreter/src/module/_csv/mod.rs | 40 ++- .../src/module/_ctypes/cdata.rs | 6 +- .../src/module/_ctypes/funcptr.rs | 27 +- .../src/module/_ctypes/interp_ctypes.rs | 22 +- .../src/module/_io/buffered.rs | 14 +- .../src/module/_io/buffered_random.rs | 14 +- .../src/module/_io/buffered_writer.rs | 14 +- .../src/module/_io/stringio.rs | 7 +- .../pyre-interpreter/src/module/_io/textio.rs | 16 +- .../src/module/_pickle/mod.rs | 22 +- .../src/module/_pickle/pickler.rs | 208 ++++++++--- .../src/module/_socket/interp_socket.rs | 39 ++- .../src/module/_sre/interp_sre.rs | 48 ++- .../src/module/_symtable/mod.rs | 2 +- .../src/module/_tokenize/mod.rs | 18 +- .../src/module/_weakref/interp__weakref.rs | 7 +- .../src/module/binascii/mod.rs | 15 +- .../src/module/importlib/interp_importlib.rs | 4 +- .../src/module/posix/interp_posix.rs | 136 ++++++-- .../src/module/signal/interp_signal.rs | 2 +- pyre/pyre-interpreter/src/module/sys/vm.rs | 26 +- .../pyre-interpreter/src/module/thread/mod.rs | 14 +- .../src/module/time/interp_time.rs | 28 +- .../src/module/unicodedata/mod.rs | 10 +- .../src/objspace/descroperation.rs | 4 +- pyre/pyre-interpreter/src/opcode_ops.rs | 16 +- pyre/pyre-interpreter/src/pyframe.rs | 15 +- pyre/pyre-interpreter/src/pyopcode.rs | 4 +- pyre/pyre-interpreter/src/runtime_ops.rs | 8 +- pyre/pyre-interpreter/src/type_methods.rs | 23 +- pyre/pyre-interpreter/src/typedef.rs | 174 ++++++---- .../src/jitcode_dispatch/specialize.rs | 2 +- pyre/pyre-jit/tests/gc_stress.rs | 2 +- pyre/pyre-macros/src/lib.rs | 4 + pyre/pyrex/Cargo.toml | 1 + pyre/pyrex/src/lib.rs | 16 +- pyre/pyrex/src/repl.rs | 2 +- 59 files changed, 1672 insertions(+), 887 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/surrogate_name_messages.py diff --git a/Cargo.lock b/Cargo.lock index ceb5038536e..abba1f4e6cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2371,6 +2371,7 @@ dependencies = [ "pyre-object", "pyre-sandbox", "rustpython-compiler", + "rustpython-wtf8", "rustyline 17.0.2", ] diff --git a/pyre/extra_tests/parity_tests/surrogate_name_messages.py b/pyre/extra_tests/parity_tests/surrogate_name_messages.py new file mode 100644 index 00000000000..ad2bb08f09b --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_name_messages.py @@ -0,0 +1,117 @@ +"""A lone surrogate in a name survives into every message that quotes the name. + +`surrogateescape` puts an unpaired surrogate into a `str` for every undecodable +filesystem byte, and a name assigned from such a string keeps it. Every surface +that quotes the name -- a repr, the argument binder's TypeErrors, the text handed +to `sys.stderr` -- rebuilds a `str` from it, and assembling that through a lossy +UTF-8 encode substitutes U+FFFD instead, silently changing the value. + +The reprs below read the surrogate out of a `__repr__` rather than out of a +plain `str`, because `str.__repr__` backslash-escapes a surrogate and would hide +the difference. +""" + +import contextvars +import io +import sys + +S = "q\udcffn" +FFFD = "�" + + +class Repr: + def __repr__(self): + return S + + +def check_function_qualname(): + def f(a): + return a + + f.__qualname__ = S + assert repr(f) == "" % (S, id(f)), ascii(repr(f)) + + try: + f(1, 2) + except TypeError as e: + assert str(e) == S + "() takes 1 positional argument but 2 were given", ascii(str(e)) + else: + raise AssertionError("no TypeError") + + try: + f() + except TypeError as e: + assert str(e) == S + "() missing 1 required positional argument: 'a'", ascii(str(e)) + else: + raise AssertionError("no TypeError") + + try: + f(1, zz=2) + except TypeError as e: + assert str(e) == S + "() got an unexpected keyword argument 'zz'", ascii(str(e)) + else: + raise AssertionError("no TypeError") + + +def check_generator_qualname(): + def g(): + yield 1 + + g.__qualname__ = S + assert g().__qualname__ == S, ascii(g().__qualname__) + + +def check_contextvars(): + var = contextvars.ContextVar("n", default=Repr()) + var_repr = repr(var) + assert var_repr == "" % (S, id(var)), ascii(var_repr) + + token = var.set("x") + assert repr(token) == "" % (var_repr, id(token)), ascii(repr(token)) + var.reset(token) + used_repr = repr(token) + assert used_repr == "" % (var_repr, id(token)), ascii(used_repr) + try: + var.reset(token) + except RuntimeError as e: + assert str(e) == used_repr + " has already been used once", ascii(str(e)) + else: + raise AssertionError("no RuntimeError") + + # A variable with no default names itself by repr in the LookupError, so + # the message carries whatever its own repr does. + unset = contextvars.ContextVar(Repr.__name__) + try: + unset.get() + except LookupError as e: + assert str(e) == repr(unset), ascii(str(e)) + else: + raise AssertionError("no LookupError") + + +def check_excepthook(): + buf = io.StringIO() + saved = sys.stderr + sys.stderr = buf + try: + raise ValueError(S) + except ValueError: + sys.excepthook(*sys.exc_info()) + finally: + sys.stderr = saved + rendered = buf.getvalue() + assert rendered.endswith("ValueError: %s\n" % S), ascii(rendered) + assert FFFD not in rendered, ascii(rendered) + + +def check_base_exception_str(): + assert str(BaseException(S)) == S, ascii(str(BaseException(S))) + assert BaseException.__str__(ValueError(S)) == S, ascii(BaseException.__str__(ValueError(S))) + + +check_function_qualname() +check_generator_qualname() +check_contextvars() +check_excepthook() +check_base_exception_str() +print("OK") diff --git a/pyre/pyre-interpreter/src/_pypy_generic_alias.rs b/pyre/pyre-interpreter/src/_pypy_generic_alias.rs index bc59b261158..8d5906972e4 100644 --- a/pyre/pyre-interpreter/src/_pypy_generic_alias.rs +++ b/pyre/pyre-interpreter/src/_pypy_generic_alias.rs @@ -215,7 +215,7 @@ fn self_alias(args: &[PyObjectRef]) -> Result { /// `GenericAlias.__repr__` (`_pypy_generic_alias.py:57`). fn ga_repr(args: &[PyObjectRef]) -> crate::PyResult { let self_ = self_alias(args)?; - Ok(w_str_new(&unsafe { repr(self_)? })) + Ok(pyre_object::w_str_from_wtf8(unsafe { repr(self_)? })) } /// `GenericAlias.__hash__` (`_pypy_generic_alias.py:82`). @@ -455,9 +455,10 @@ pub(crate) fn subs_parameters( let current_params = || pyre_object::gc_roots::shadow_stack_get(root_base + 2); let nparams = unsafe { w_tuple_len(current_params()) }; if nparams == 0 { - let repr = unsafe { crate::display::py_repr(current_self())? }; - return Err(crate::PyError::type_error(format!( - "{repr} is not a generic class" + let repr = unsafe { crate::display::py_repr_wtf8(current_self())? }; + return Err(crate::PyError::type_error(crate::display::wtf8_format!( + repr, + " is not a generic class" ))); } // Substitution runs arbitrary Python — `__typing_prepare_subst__`, @@ -524,8 +525,9 @@ pub(crate) fn subs_parameters( }; if nparams != nitems { let direction = if nitems > nparams { "many" } else { "few" }; - let s = - unsafe { crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(self_slot))? }; + let s = unsafe { + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(self_slot))? + }; if nitems < nparams { // A parameter carrying a default need not be supplied, so the // shortfall is measured against the required count rather than @@ -1193,7 +1195,8 @@ pub(crate) fn init_generic_alias_type(ns: PyObjectRef) { /// /// # Safety /// `obj` must point to a valid `GenericAlias`. -pub(crate) unsafe fn repr(obj: PyObjectRef) -> Result { +pub(crate) unsafe fn repr(obj: PyObjectRef) -> Result { + use rustpython_wtf8::Wtf8Buf; let _roots = pyre_object::gc_roots::push_roots(); pyre_object::gc_roots::pin_root(obj); let obj_slot = pyre_object::gc_roots::shadow_stack_len() - 1; @@ -1214,13 +1217,13 @@ pub(crate) unsafe fn repr(obj: PyObjectRef) -> Result { let result = w_tuple_getitem(current_args(), (n - 1) as i64).unwrap(); let result_repr = repr_item(result)?; if n == 1 { - format!("[], {result_repr}") + crate::display::wtf8_format!("[], ", result_repr) } else { let first = w_tuple_getitem(current_args(), 0).unwrap(); if is_ellipsis(first) { - format!("..., {result_repr}") + crate::display::wtf8_format!("..., ", result_repr) } else if n == 2 && (is_param_spec(first)? || is_typing_generic_alias(first)?) { - format!("{}, {result_repr}", repr_item(first)?) + crate::display::wtf8_format!(repr_item(first)?, ", ", result_repr) } else { let mut params = Vec::with_capacity(n - 1); for i in 0..n - 1 { @@ -1228,11 +1231,11 @@ pub(crate) unsafe fn repr(obj: PyObjectRef) -> Result { params.push(repr_item(item)?); } } - format!("[{}], {result_repr}", params.join(", ")) + crate::display::wtf8_format!("[", join_wtf8(¶ms, ", "), "], ", result_repr) } } } else if n == 0 { - "()".to_string() + Wtf8Buf::from_string("()".to_string()) } else { let mut parts = Vec::with_capacity(n); for i in 0..n { @@ -1244,24 +1247,40 @@ pub(crate) unsafe fn repr(obj: PyObjectRef) -> Result { }); } } - parts.join(", ") + join_wtf8(&parts, ", ") }; let star = if w_generic_alias_get_unpacked(pyre_object::gc_roots::shadow_stack_get(obj_slot)) { "*" } else { "" }; - Ok(format!( - "{star}{}[{inner}]", - repr_item(pyre_object::gc_roots::shadow_stack_get(origin_slot))? + Ok(crate::display::wtf8_format!( + star, + repr_item(pyre_object::gc_roots::shadow_stack_get(origin_slot))?, + "[", + inner, + "]" )) } +/// `", ".join(parts)` for pieces that may hold a lone surrogate, which +/// `[Wtf8Buf]` has no `join` for. +fn join_wtf8(parts: &[rustpython_wtf8::Wtf8Buf], sep: &str) -> rustpython_wtf8::Wtf8Buf { + let mut out = rustpython_wtf8::Wtf8Buf::new(); + for (index, part) in parts.iter().enumerate() { + if index > 0 { + out.push_str(sep); + } + out.push_wtf8(part); + } + out +} + /// CPython 3.14 `ga_repr_items_list` — ParamSpec substitutions retain a /// list, whose type items use typing-style rendering. Fetch each element /// after its predecessor's repr so mutation during a callback raises /// `IndexError` rather than reading stale storage. -unsafe fn repr_items_list(list: PyObjectRef) -> Result { +unsafe fn repr_items_list(list: PyObjectRef) -> Result { let n = w_list_len(list); let mut parts = Vec::with_capacity(n); for i in 0..n { @@ -1269,19 +1288,25 @@ unsafe fn repr_items_list(list: PyObjectRef) -> Result { .ok_or_else(|| crate::PyError::index_error("list index out of range"))?; parts.push(repr_item(item)?); } - Ok(format!("[{}]", parts.join(", "))) + Ok(crate::display::wtf8_format!( + "[", + join_wtf8(&parts, ", "), + "]" + )) } /// `_repr_item(it)` (`_pypy_generic_alias.py:124`) — a class renders as its /// qualname (prefixed with the module when it is not `builtins`); anything /// else falls back to `repr`. -pub(crate) unsafe fn repr_item(it: PyObjectRef) -> Result { +pub(crate) unsafe fn repr_item( + it: PyObjectRef, +) -> Result { let _roots = pyre_object::gc_roots::push_roots(); pyre_object::gc_roots::pin_root(it); let item_slot = pyre_object::gc_roots::shadow_stack_len() - 1; let current_item = || pyre_object::gc_roots::shadow_stack_get(item_slot); if is_ellipsis(current_item()) { - return Ok("...".to_string()); + return Ok(rustpython_wtf8::Wtf8Buf::from_string("...".to_string())); } if is_generic_alias(current_item()) { return repr(current_item()); @@ -1291,7 +1316,7 @@ pub(crate) unsafe fn repr_item(it: PyObjectRef) -> Result Result format!("{m}.{qualname}"), _ => qualname, - }); + })); } } - crate::display::py_repr(current_item()) + unsafe { crate::display::py_repr_wtf8(current_item()) } } fn is_collections_abc_callable(origin: PyObjectRef) -> Result { diff --git a/pyre/pyre-interpreter/src/argument.rs b/pyre/pyre-interpreter/src/argument.rs index 9ef65e0e1d9..f229d987f20 100644 --- a/pyre/pyre-interpreter/src/argument.rs +++ b/pyre/pyre-interpreter/src/argument.rs @@ -63,12 +63,20 @@ fn type_name_of(w_obj: PyObjectRef) -> String { /// TypeError and propagates as the actual exception PyPy raises. /// Pyre mirrors this by returning the async `PyError` verbatim /// instead of building the TypeError prefix. -pub fn raise_type_error(w_function: PyObjectRef, msg: String) -> crate::PyError { +pub fn raise_type_error( + w_function: PyObjectRef, + msg: impl Into, +) -> crate::PyError { + let msg = msg.into(); if w_function.is_null() { return crate::PyError::type_error(msg); } match crate::baseobjspace::object_functionstr(w_function) { - Ok(prefix) => crate::PyError::type_error(format!("{prefix} {msg}")), + Ok(mut prefix) => { + prefix.push_str(" "); + prefix.push_wtf8(&msg); + crate::PyError::type_error(prefix) + } // Async findattr propagates as the actual raise (matches // PyPy `oefmt`'s argument-evaluation order: side effects of // `%s` formatting fire before the TypeError is constructed). @@ -154,14 +162,18 @@ pub fn check_not_duplicate_kwargs( if contains_w_names(w_key, existingkeywords_w) { let key_repr = unsafe { if pyre_object::is_str(w_key) { - pyre_object::w_str_get_wtf8(w_key).to_string() + pyre_object::w_str_get_wtf8(w_key).to_owned() } else { - crate::display::py_str(w_key)? + crate::display::py_str_wtf8(w_key)? } }; return Err(raise_type_error( w_function, - format!("got multiple values for keyword argument '{key_repr}'"), + crate::display::wtf8_format!( + "got multiple values for keyword argument '", + key_repr, + "'" + ), )); } } @@ -1727,7 +1739,7 @@ mod tests { fn raise_type_error_no_function() { let err = raise_type_error(pyre_object::PY_NULL, "boom".to_string()); assert_eq!(err.kind, crate::PyErrorKind::TypeError); - assert_eq!(err.message, "boom"); + assert_eq!(err.message_text(), "boom"); } /// pypy/interpreter/argument.py:16-17 — function-prefixed arm. @@ -1744,7 +1756,7 @@ mod tests { assert_eq!(err.kind, crate::PyErrorKind::TypeError); // object_functionstr scalar fallback returns the str() of the // value, here "7"; raise_type_error joins it with the message. - assert_eq!(err.message, "7 needs an iterable"); + assert_eq!(err.message_text(), "7 needs an iterable"); } /// pypy/interpreter/argument.py:534-552 single-missing positional case. @@ -1917,7 +1929,7 @@ mod tests { let err = check_not_duplicate_kwargs(&existing, &new, &values, pyre_object::PY_NULL) .expect_err("should raise TypeError on duplicate"); assert_eq!(err.kind, crate::PyErrorKind::TypeError); - assert!(err.message.contains("got multiple values")); + assert!(err.message_text().contains("got multiple values")); } /// pypy/interpreter/argument.py:410-417 `_check_not_duplicate_kwargs` — @@ -1930,8 +1942,8 @@ mod tests { let err = check_not_duplicate_kwargs(&existing, &new, &values, pyre_object::PY_NULL) .expect_err("should raise TypeError on duplicate"); assert_eq!(err.kind, crate::PyErrorKind::TypeError); - assert!(err.message.contains("got multiple values")); - assert!(err.message.contains("'a'")); + assert!(err.message_text().contains("got multiple values")); + assert!(err.message_text().contains("'a'")); } /// `check_not_duplicate_kwargs` accepts disjoint name sets. @@ -1970,7 +1982,10 @@ mod tests { let err = combine_starargs_wrapped(&mut args, stararg, pyre_object::PY_NULL) .expect_err("int star arg should raise TypeError"); assert_eq!(err.kind, crate::PyErrorKind::TypeError); - assert!(err.message.contains("argument after * must be an iterable")); + assert!( + err.message_text() + .contains("argument after * must be an iterable") + ); } /// pypy/interpreter/argument.py:172-338 `match_signature` happy @@ -2187,7 +2202,10 @@ mod tests { Ok(_) => panic!("non-iterable star arg should fail"), Err(err) => { assert_eq!(err.kind, crate::PyErrorKind::TypeError); - assert!(err.message.contains("argument after * must be an iterable")); + assert!( + err.message_text() + .contains("argument after * must be an iterable") + ); } } } @@ -2280,7 +2298,7 @@ mod tests { assert_eq!(err.kind, crate::PyErrorKind::TypeError); // baseobjspace.py:313-315 `_typed_unwrap_error`: // `expected str, got object`. - assert!(err.message.contains("expected str")); + assert!(err.message_text().contains("expected str")); } } } @@ -2352,7 +2370,7 @@ mod tests { ) .expect_err("unknown kwarg should TypeError"); assert_eq!(err.kind, crate::PyErrorKind::TypeError); - assert_eq!(err.message, "myfn() takes no keyword arguments"); + assert_eq!(err.message_text(), "myfn() takes no keyword arguments"); } /// pypy/interpreter/argument.py:385-389 `frompacked` builds diff --git a/pyre/pyre-interpreter/src/astcompiler/validate.rs b/pyre/pyre-interpreter/src/astcompiler/validate.rs index c5a9075f5ab..4ffe655311f 100644 --- a/pyre/pyre-interpreter/src/astcompiler/validate.rs +++ b/pyre/pyre-interpreter/src/astcompiler/validate.rs @@ -13,16 +13,17 @@ use rustpython_compiler::ast; use crate::PyError; +use rustpython_wtf8::Wtf8Buf; type ValidateResult = Result<(), PyError>; /// Seen as a ValueError. -fn validation_error(message: impl Into) -> PyError { +fn validation_error(message: impl Into) -> PyError { PyError::value_error(message) } /// Seen as a TypeError. -fn validation_type_error(message: impl Into) -> PyError { +fn validation_type_error(message: impl Into) -> PyError { PyError::type_error(message) } @@ -823,7 +824,7 @@ mod tests { } fn message(result: ValidateResult) -> String { - result.unwrap_err().message + result.unwrap_err().message_text() } #[test] @@ -1020,7 +1021,7 @@ mod tests { // expression, a ValueError. let error = validate(named(constant(1))).unwrap_err(); assert_eq!(error.kind, crate::error::PyErrorKind::TypeError); - assert_eq!(error.message, "NamedExpr target must be a Name"); + assert_eq!(error.message_text(), "NamedExpr target must be a Name"); // Only the shape is checked here; the context the target carries is // not, so a `Store` name passes the walk. diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index f78d69d233f..234737f811b 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -157,7 +157,7 @@ pub fn wrap_dict_key_hash_error(key: PyObjectRef, err: PyError) -> PyError { PyError::type_error(format!( "cannot use '{}' as a dict key ({})", object_functionstr_type_name(key), - err.message, + err.message_text(), )) } @@ -176,7 +176,7 @@ pub fn wrap_set_element_hash_error(item: PyObjectRef, err: PyError) -> PyError { PyError::type_error(format!( "cannot use '{}' as a set element ({})", object_functionstr_type_name(item), - err.message, + err.message_text(), )) } @@ -2159,9 +2159,9 @@ pub(crate) fn range_index_method(args: &[PyObjectRef]) -> PyResult { if pyre_object::w_range_contains_bigint(obj, &item) { return Ok(pyre_object::w_range_index_of(obj, &item)); } - return Err(PyError::value_error(format!( - "{} is not in range", - crate::display::py_repr(needle)? + return Err(PyError::value_error(crate::display::wtf8_format!( + crate::display::py_repr_wtf8(needle)?, + " is not in range" ))); } } @@ -6640,32 +6640,44 @@ unsafe fn module_getattr_hook_or_err( let (origin, is_shadowing, is_shadowing_stdlib) = crate::importing::module_shadow_info(w_spec, w_name)?; let w_spec = pyre_object::gc_roots::shadow_stack_get(spec_slot); + // The origin is a filename and may hold a surrogate escape, so each + // message that names it is assembled as WTF-8; `format!` would render + // it through `Display` and substitute U+FFFD. + let with_origin = |head: String, tail: String| { + let mut msg = Wtf8Buf::from_string(head); + msg.push_wtf8(origin.as_deref().unwrap_or(Wtf8::new(""))); + msg.push_str(&tail); + msg + }; if is_shadowing_stdlib { - let origin = origin.as_deref().unwrap_or(""); - format!( - "module '{nm}' has no attribute '{name}' (consider renaming \ - '{origin}' since it has the same name as the standard library \ - module named '{nm}' and prevents importing that standard \ - library module)" + with_origin( + format!("module '{nm}' has no attribute '{name}' (consider renaming '"), + format!( + "' since it has the same name as the standard library module \ + named '{nm}' and prevents importing that standard library \ + module)" + ), ) } else if crate::importing::is_spec_initializing(w_spec)? { if is_shadowing { - let origin = origin.as_deref().unwrap_or(""); - format!( - "module '{nm}' has no attribute '{name}' (consider renaming \ - '{origin}' if it has the same name as a library you \ - intended to import)" + with_origin( + format!("module '{nm}' has no attribute '{name}' (consider renaming '"), + "' if it has the same name as a library you intended to import)".to_string(), ) - } else if let Some(origin) = origin.as_deref() { - format!( - "partially initialized module '{nm}' from '{origin}' has no \ - attribute '{name}' (most likely due to a circular import)" + } else if origin.is_some() { + with_origin( + format!("partially initialized module '{nm}' from '"), + format!( + "' has no attribute '{name}' (most likely due to a \ + circular import)" + ), ) } else { format!( "partially initialized module '{nm}' has no attribute \ '{name}' (most likely due to a circular import)" ) + .into() } } else { let w_spec = pyre_object::gc_roots::shadow_stack_get(spec_slot); @@ -6674,12 +6686,13 @@ unsafe fn module_getattr_hook_or_err( "cannot access submodule '{name}' of module '{nm}' \ (most likely due to a circular import)" ) + .into() } else { - format!("module '{nm}' has no attribute '{name}'") + format!("module '{nm}' has no attribute '{name}'").into() } } } else { - format!("module '{nm}' has no attribute '{name}'") + Wtf8Buf::from_string(format!("module '{nm}' has no attribute '{name}'")) }; Err(PyError::new(PyErrorKind::AttributeError, msg)) } @@ -8630,7 +8643,7 @@ impl SimpleBufferBytes { if let Err(mut error) = crate::builtins::memoryview_release(&[view]) { error.write_unraisable( pyre_object::w_none(), - "Exception ignored in __release_buffer__:", + Wtf8::new("Exception ignored in __release_buffer__:"), view, ); } @@ -13667,7 +13680,7 @@ pub fn view_as_kwargs(w_dict: PyObjectRef) -> (Option>, Option< /// functions. Pyre's `Function` does not carry the field directly; /// `crate::function::function_get_qualname` reproduces the same /// precedence (set-attr override → `code.qualname` → `function.name`). -pub fn object_functionstr(w_function: PyObjectRef) -> Result { +pub fn object_functionstr(w_function: PyObjectRef) -> Result { // baseobjspace.py:2108-2120 — Function fast path (also covers // `FunctionWithFixedCode` and `BuiltinFunction`, both subclasses // of `Function` per function.py:783,786). Pyre's `is_function` @@ -13676,15 +13689,22 @@ pub fn object_functionstr(w_function: PyObjectRef) -> Result Result break 'qualname, }; + // The dotted prefix is optional; the qualname and the trailing `()` + // are not, so build the one and prepend the other. + let bare = |qualname: &Wtf8Buf| { + let mut out = qualname.clone(); + out.push_str("()"); + out + }; match w_module { // No `__module__` or `__module__ is None`: bare `qualname()`. - None => return Ok(format!("{qualname}()")), - Some(w_module) if is_w(w_module, w_none()) => return Ok(format!("{qualname}()")), + None => return Ok(bare(&qualname)), + Some(w_module) if is_w(w_module, w_none()) => return Ok(bare(&qualname)), Some(w_module) => { // text_w(w_module) — non-string raises in PyPy → except → // fall through (do NOT return `qualname()` here, which @@ -13727,11 +13754,15 @@ pub fn object_functionstr(w_function: PyObjectRef) -> Result Result Result { +fn object_functionstr_text_w(w_obj: PyObjectRef) -> Result { unsafe { if pyre_object::is_str(w_obj) { - Ok(pyre_object::w_str_get_value(w_obj).to_string()) + // `text_w` is the surrogate-preserving spelling upstream, and + // `w_str_get_value` would panic outright on a `__qualname__` + // carrying a lone surrogate rather than return its text. + Ok(pyre_object::w_str_get_wtf8(w_obj).to_wtf8_buf()) } else { Err(crate::PyError::type_error(format!( "expected str, got {} object", @@ -16675,10 +16711,15 @@ unsafe fn property_no_accessor( } } let msg = if !w_name.is_null() { - let name_repr = crate::display::py_repr(w_name).unwrap_or_else(|_| "".to_string()); - format!("property {name_repr} of '{qualname}' object has no {kind}") + let name_repr = crate::display::py_repr_wtf8(w_name) + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())); + crate::display::wtf8_format!( + "property ", + name_repr, + format!(" of '{qualname}' object has no {kind}") + ) } else { - format!("property of '{qualname}' object has no {kind}") + Wtf8Buf::from_string(format!("property of '{qualname}' object has no {kind}")) }; Ok(crate::PyError::attribute_error(msg)) } @@ -18004,7 +18045,11 @@ pub fn generator_finalize(gen_obj: PyObjectRef) -> PyResult { return match crate::call::call_function_impl_result(finalizer, &[gen_obj]) { Ok(_) => Ok(w_none()), Err(mut err) => { - err.write_unraisable(w_none(), "async generator finalizer", gen_obj); + err.write_unraisable( + w_none(), + Wtf8::new("async generator finalizer"), + gen_obj, + ); Ok(w_none()) } }; @@ -18265,7 +18310,7 @@ mod tests { crate::typedef::init_typeobjects(); let err = super::isinstance(w_int_new(5), w_int_new(6)).unwrap_err(); assert!(matches!(err.kind, PyErrorKind::TypeError)); - assert!(err.message.contains("isinstance() arg 2")); + assert!(err.message_text().contains("isinstance() arg 2")); } /// abstractinst.py:108-114 + 53-72 — when one tuple element is not a @@ -18287,7 +18332,7 @@ mod tests { let int_type = crate::typedef::r#type(w_int_new(0)).unwrap().as_ptr(); let err = super::issubclass(w_int_new(5), int_type).unwrap_err(); assert!(matches!(err.kind, PyErrorKind::TypeError)); - assert!(err.message.contains("issubclass() arg 1")); + assert!(err.message_text().contains("issubclass() arg 1")); } /// abstractinst.py:150-169 — `issubclass(int, 6)` must raise @@ -18298,7 +18343,7 @@ mod tests { let int_type = crate::typedef::r#type(w_int_new(0)).unwrap().as_ptr(); let err = super::issubclass(int_type, w_int_new(6)).unwrap_err(); assert!(matches!(err.kind, PyErrorKind::TypeError)); - assert!(err.message.contains("issubclass() arg 2")); + assert!(err.message_text().contains("issubclass() arg 2")); } /// abstractinst.py:127-147 — `p_abstract_issubclass_w` must walk @@ -18352,7 +18397,7 @@ mod tests { let lst = w_list_new(vec![w_int_new(1)]); let err = unpackiterable(lst, 3).expect_err("expected ValueError"); assert_eq!(err.kind, crate::PyErrorKind::ValueError); - assert!(err.message.contains("not enough values")); + assert!(err.message_text().contains("not enough values")); } /// pypy/interpreter/baseobjspace.py:1043-1046 — `too many values @@ -18363,7 +18408,7 @@ mod tests { let lst = w_list_new(vec![w_int_new(1), w_int_new(2), w_int_new(3), w_int_new(4)]); let err = unpackiterable(lst, 3).expect_err("expected ValueError"); assert_eq!(err.kind, crate::PyErrorKind::ValueError); - assert!(err.message.contains("too many values")); + assert!(err.message_text().contains("too many values")); } /// pypy/interpreter/baseobjspace.py:983-994 — expected_length=-1 @@ -18453,7 +18498,7 @@ mod tests { fn object_functionstr_scalar_fallback() { crate::typedef::init_typeobjects(); let s = object_functionstr(w_int_new(42)).expect("scalar fallback never propagates async"); - assert_eq!(s, "42"); + assert_eq!(s.as_str(), Ok("42")); } } diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index ca9d03439e4..9331bc71186 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -678,7 +678,7 @@ fn w_memoryview_new_with_flags_impl( let tname = crate::typedef::r#type(w_obj) .map(|t| pyre_object::w_type_get_name(t.as_ptr())) .unwrap_or("object"); - return Err(crate::PyError::type_error(&format!( + return Err(crate::PyError::type_error(format!( "memoryview: a bytes-like object is required, not '{tname}'" ))); } @@ -1784,9 +1784,12 @@ unsafe fn memoryview_call_python_release_unraisable( let r_exporter = pyre_object::gc_roots::shadow_stack_get(sp); error.write_unraisable( w_none(), - &format!( - "Exception ignored in __release_buffer__ of {}:", - crate::baseobjspace::object_functionstr_type_name(r_exporter) + rustpython_wtf8::Wtf8::new( + format!( + "Exception ignored in __release_buffer__ of {}:", + crate::baseobjspace::object_functionstr_type_name(r_exporter) + ) + .as_str(), ), r_exporter, ); @@ -3948,8 +3951,13 @@ pub(crate) fn sys_excepthook(args: &[PyObjectRef]) -> Result pyre_object::w_str_from_wtf8(text.to_wtf8_buf()), + None => pyre_object::w_str_new(&String::from_utf8_lossy(&rendered)), + }; let result = crate::baseobjspace::call_method(stderr, "write", &[w_text]); if result.is_null() { let _ = crate::call::take_call_error(); @@ -3957,7 +3965,7 @@ pub(crate) fn sys_excepthook(args: &[PyObjectRef]) -> Result = vec![PY_NULL; names.len()]; let mut filled: Vec = vec![false; names.len()]; - let mut unknown: Option = None; + // `argument.py:616` keys the message off `space.text_w(keyword_names_w[i])`, + // the keyword's own storage, so a name carrying a lone surrogate reaches + // `e.args[0]` intact. Keep the WTF-8 rather than a lossy `String`. + let mut unknown: Option = None; for (i, &v) in positional.iter().enumerate() { scope[i] = v; filled[i] = true; @@ -4528,7 +4539,7 @@ pub(crate) fn bind_builtin_kwargs( // so a call that misses a required argument is reported // against that argument even when it also passed a keyword // the function does not know. - None => unknown = Some(key.to_string_lossy().into_owned()), + None => unknown = Some(key.to_wtf8_buf()), } } } @@ -4542,9 +4553,11 @@ pub(crate) fn bind_builtin_kwargs( } } if let Some(key) = unknown { - return Err(crate::PyError::type_error(format!( - "{fn_name}() got an unexpected keyword argument '{key}'" - ))); + let mut msg = + Wtf8Buf::from_string(format!("{fn_name}() got an unexpected keyword argument '")); + msg.push_wtf8(&key); + msg.push_str("'"); + return Err(crate::PyError::type_error(msg)); } Ok(scope) } @@ -5963,21 +5976,25 @@ fn os_error_build( let w = unsafe { pyre_object::w_str_get_wtf8(args[0]) }; interp_exceptions::w_exception_new_wtf8(kind, w) } else { - let msg: String = if args.is_empty() { - String::new() + let msg: rustpython_wtf8::Wtf8Buf = if args.is_empty() { + rustpython_wtf8::Wtf8Buf::new() } else if args.len() == 1 { // exception construction is non-raising machinery, per the F7 // display policy; a raising __str__/__repr__ on the args degrades // to the empty string rather than propagating. - unsafe { crate::display::py_str(args[0]) }.unwrap_or_default() + unsafe { crate::display::py_str_wtf8(args[0]) }.unwrap_or_default() } else { - let parts: Vec = args - .iter() - .map(|&a| unsafe { crate::display::py_repr(a) }.unwrap_or_default()) - .collect(); - format!("({})", parts.join(", ")) + let mut parts = rustpython_wtf8::Wtf8Buf::from_string("(".to_string()); + for (index, &a) in args.iter().enumerate() { + if index > 0 { + parts.push_str(", "); + } + parts.push_wtf8(&unsafe { crate::display::py_repr_wtf8(a) }.unwrap_or_default()); + } + parts.push_str(")"); + parts }; - interp_exceptions::w_exception_new(kind, &msg) + interp_exceptions::w_exception_new_wtf8(kind, &msg) }; // Seed `args_w` so a deferred-init instance (`_use_init`, no `__new__` // slot fill) still reports the empty tuple until `__init__` runs. @@ -7262,9 +7279,8 @@ fn exc_system_exit_init(args: &[PyObjectRef]) -> crate::PyResult { /// runs even when `exc`'s own class registers a `descr_str` override. fn base_exception_str_method(args: &[PyObjectRef]) -> crate::PyResult { let obj = args[0]; - Ok(pyre_object::w_str_new(&unsafe { - crate::display::base_exception_str(obj)? - })) + let text = unsafe { crate::display::base_exception_str_wtf8(obj)? }; + Ok(pyre_object::w_str_from_wtf8(text)) } /// The `descr_str` of the class that registers one, falling back to @@ -9412,10 +9428,11 @@ fn unicode_to_decimal_w( /// source object, not the whitespace-trimmed or Unicode-normalized buffer the /// number parser consumes internally. fn invalid_int_literal(w_source: PyObjectRef, base: u32) -> crate::PyError { - let source_repr = unsafe { crate::display::py_repr(w_source) } - .unwrap_or_else(|_| "".to_string()); - crate::PyError::value_error(format!( - "invalid literal for int() with base {base}: {source_repr}" + let source_repr = unsafe { crate::display::py_repr_wtf8(w_source) } + .unwrap_or_else(|_| rustpython_wtf8::Wtf8Buf::from_string("".to_string())); + crate::PyError::value_error(crate::display::wtf8_format!( + format!("invalid literal for int() with base {base}: "), + source_repr )) } @@ -9717,9 +9734,10 @@ pub(crate) fn builtin_float(args: &[PyObjectRef]) -> Result Result Result { unsafe { @@ -10911,6 +10930,7 @@ fn builtin_compile(args: &[PyObjectRef]) -> Result // spelling, and reporting it at the first `co_filename` read instead would // blame whoever came to look at the code object. crate::gateway::fsdecode_filename_checked(&filename_bytes)?; + let filename_text = crate::gateway::fsdecode_filename_wtf8(&filename_bytes); let (filename, filename_bytes) = crate::pycode::split_code_filename_bytes(filename_bytes, None); let mode = crate::baseobjspace::text_w(mode_obj)?; // flags / dont_inherit / optimize are positional-or-keyword ints @@ -11000,7 +11020,7 @@ fn builtin_compile(args: &[PyObjectRef]) -> Result source, "compile", "string, bytes or AST", - &filename, + &filename_text, &mut flags, ) .map_err(|error| { @@ -11150,7 +11170,7 @@ fn exec_or_eval( source, if is_eval { "eval" } else { "exec" }, "string, bytes or code", - "", + rustpython_wtf8::Wtf8::new(""), &mut flags, )?; if is_eval { @@ -12057,7 +12077,7 @@ pub fn try_hash_value(obj: PyObjectRef) -> Result { } } } - return Err(crate::PyError::type_error(&format!( + return Err(crate::PyError::type_error(format!( "unhashable type: '{}'", name ))); @@ -13958,14 +13978,16 @@ fn fileio_method_repr(args: &[PyObjectRef]) -> Result"))) } @@ -16862,7 +16884,7 @@ mod tests { let err = parse_int_from_str(source, &text, 10).unwrap_err(); assert_eq!(err.kind, crate::PyErrorKind::ValueError); assert_eq!( - err.message, + err.message_text(), "Exceeds the limit (4300 digits) for integer string conversion: value has 4301 digits; use sys.set_int_max_str_digits() to increase the limit" ); } @@ -17132,6 +17154,9 @@ mod tests { // 2 and 4 share a factor, so 2 has no inverse modulo 4. let err = builtin_pow(&[w_int_new(2), w_int_new(-1), w_int_new(4)]).unwrap_err(); assert_eq!(err.kind, crate::PyErrorKind::ValueError); - assert_eq!(err.message, "base is not invertible for the given modulus"); + assert_eq!( + err.message_text(), + "base is not invertible for the given modulus" + ); } } diff --git a/pyre/pyre-interpreter/src/call.rs b/pyre/pyre-interpreter/src/call.rs index c5483b2b735..d696d6a0809 100644 --- a/pyre/pyre-interpreter/src/call.rs +++ b/pyre/pyre-interpreter/src/call.rs @@ -6,7 +6,7 @@ use std::cell::{Cell, RefCell}; use std::sync::OnceLock; -use rustpython_wtf8::Wtf8Buf; +use rustpython_wtf8::{Wtf8, Wtf8Buf}; use crate::runtime_ops::{CallableKind, classify_callable}; use crate::{ @@ -21,9 +21,12 @@ pub(crate) fn frame_into_generator_for_function( frame: crate::pyframe::FrameBox, function: PyObjectRef, ) -> PyResult { - let name = unsafe { function_get_name(function) }.to_string(); + // `__name__` lives in the `Function`'s Rust `str` field, so it is UTF-8 by + // construction; `__qualname__` is a Python object and may carry a lone + // surrogate, which the generator's own `__qualname__` has to read back. + let name = unsafe { function_get_name(function) }; let qualname = unsafe { function_get_qualname(function) }; - frame.into_generator_named(Some(&name), Some(&qualname)) + frame.into_generator_named(Some(Wtf8::new(name)), Some(&qualname)) } struct FrameLocalsRoot { @@ -526,10 +529,12 @@ fn fill_user_function_args( ) }; let given_str = format!("{} {}", nargs, if nargs != 1 { "were" } else { "was" }); - return Err(crate::PyError::type_error(format!( - "{}() takes {} but {} given", - fname, takes_str, given_str - ))); + // `fname` is WTF-8: `format!` would render it through `Display`, which + // substitutes U+FFFD for a lone surrogate. + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(&fname); + msg.push_str(&format!("() takes {takes_str} but {given_str} given")); + return Err(crate::PyError::type_error(msg)); } // Lay out filled_args as `[positional[0..nparams], kwonly[0..nkwonly]]` @@ -643,7 +648,11 @@ fn fill_user_function_args( } /// `argument.py:534-552` ArgErrMissing.getmsg parity. -fn format_missing_err(fname: &str, missing: &[&str], positional: bool) -> String { +/// +/// `fname` is the function's `__qualname__`, which may carry a lone surrogate, +/// and the message becomes the TypeError's `args[0]` -- so the whole line is +/// assembled as WTF-8 rather than through a `String`. +fn format_missing_err(fname: &Wtf8, missing: &[&str], positional: bool) -> Wtf8Buf { let mut arguments_str = String::new(); for (i, arg) in missing.iter().enumerate() { if i == 0 { @@ -661,9 +670,10 @@ fn format_missing_err(fname: &str, missing: &[&str], positional: bool) -> String arguments_str.push_str(arg); arguments_str.push('\''); } - format!( - "{}() missing {} required {} argument{}: {}", - fname, + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(fname); + msg.push_str(&format!( + "() missing {} required {} argument{}: {arguments_str}", missing.len(), if positional { "positional" @@ -671,36 +681,41 @@ fn format_missing_err(fname: &str, missing: &[&str], positional: bool) -> String "keyword-only" }, if missing.len() != 1 { "s" } else { "" }, - arguments_str - ) + )); + msg } /// `argument.py:620-626` ArgErrUnknownKwds.getmsg parity. -fn format_unknown_kwds_err(fname: &str, unmatched: &[Wtf8Buf]) -> String { +fn format_unknown_kwds_err(fname: &Wtf8, unmatched: &[Wtf8Buf]) -> Wtf8Buf { + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(fname); if unmatched.len() == 1 { - format!( - "{}() got an unexpected keyword argument '{}'", - fname, unmatched[0] - ) + // `argument.py:616` keys this off the keyword's own storage, so a name + // with a lone surrogate reaches `e.args[0]` as itself. + msg.push_str("() got an unexpected keyword argument '"); + msg.push_wtf8(&unmatched[0]); + msg.push_str("'"); } else { - format!( - "{}() got {} unexpected keyword arguments", - fname, + msg.push_str(&format!( + "() got {} unexpected keyword arguments", unmatched.len() - ) + )); } + msg } #[cold] -fn raise_if_posonly_kwds(posonly_kwds: &[String], fname: &str) -> Result<(), PyError> { +fn raise_if_posonly_kwds(posonly_kwds: &[String], fname: &Wtf8) -> Result<(), PyError> { if posonly_kwds.is_empty() { return Ok(()); } - Err(crate::PyError::type_error(format!( - "{}() got some positional-only arguments passed as keyword arguments: '{}'", - fname, + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(fname); + msg.push_str(&format!( + "() got some positional-only arguments passed as keyword arguments: '{}'", posonly_kwds.join(", ") - ))) + )); + Err(crate::PyError::type_error(msg)) } /// Materialize a Python call frame without retaining the by-value `PyFrame` @@ -2290,7 +2305,7 @@ pub(crate) fn bind_kwargs_to_signature( // argument.py:499-500 — ArgErrPosonlyAsKwds, raised after the full // keyword scan and before ArgErrUnknownKwds. - raise_if_posonly_kwds(&posonly_kwds, fname)?; + raise_if_posonly_kwds(&posonly_kwds, Wtf8::new(fname))?; if !unmatched_kw_names.is_empty() { // parse_obj (argument.py:377-380) rewrites the unknown-keyword message @@ -2299,9 +2314,12 @@ pub(crate) fn bind_kwargs_to_signature( // call routes through parse_obj (gateway.py funcrun / funcrun_obj), so // the rewrite applies at any arity, not just the single-argument form. let msg = if !has_varkw && sig.num_kwonlyargnames() == 0 { - format!("{}() takes no keyword arguments", fname) + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(Wtf8::new(fname)); + msg.push_str("() takes no keyword arguments"); + msg } else { - format_unknown_kwds_err(fname, &unmatched_kw_names) + format_unknown_kwds_err(Wtf8::new(fname), &unmatched_kw_names) }; return Err(crate::PyError::type_error(msg)); } @@ -3127,7 +3145,7 @@ pub fn call_function_impl_raw(callable: PyObjectRef, args: &[PyObjectRef]) -> Py match call_function_impl_result(callable, args) { Ok(result) => result, Err(e) => { - log_call_error(&e.message); + log_call_error(&e.message_text()); set_call_error(e); PY_NULL } @@ -4732,17 +4750,21 @@ fn build_class_inner( if w_metaclass.is_some() && unsafe { pyre_object::is_type(w_type) } { let cell_value = unsafe { pyre_object::w_cell_get(classcell) }; if cell_value.is_null() { - let class_str = unsafe { crate::py_str(w_type) }?; - return Err(PyError::runtime_error(format!( - "__class__ not set defining {name} as {class_str}. \ - Was __classcell__ propagated to type.__new__?" + let class_str = unsafe { crate::py_str_wtf8(w_type) }?; + return Err(PyError::runtime_error(crate::display::wtf8_format!( + format!("__class__ not set defining {name} as "), + class_str, + ". Was __classcell__ propagated to type.__new__?", ))); } if !std::ptr::eq(cell_value, w_type) { - let cell_str = unsafe { crate::py_str(cell_value) }?; - let class_str = unsafe { crate::py_str(w_type) }?; - return Err(PyError::type_error(format!( - "__class__ set to {cell_str} defining {name} as {class_str}" + let cell_str = unsafe { crate::py_str_wtf8(cell_value) }?; + let class_str = unsafe { crate::py_str_wtf8(w_type) }?; + return Err(PyError::type_error(crate::display::wtf8_format!( + "__class__ set to ", + cell_str, + format!(" defining {name} as "), + class_str, ))); } } else { diff --git a/pyre/pyre-interpreter/src/compile.rs b/pyre/pyre-interpreter/src/compile.rs index 515bb0ba9c9..e6a2fba7769 100644 --- a/pyre/pyre-interpreter/src/compile.rs +++ b/pyre/pyre-interpreter/src/compile.rs @@ -150,7 +150,7 @@ fn is_utf8_encoding(name: &str) -> bool { /// lossily replaced. pub fn decode_source_bytes( source: &[u8], - filename: &str, + filename: &rustpython_wtf8::Wtf8, ignore_cookie: bool, ) -> Result { let has_bom = source.starts_with(b"\xef\xbb\xbf"); @@ -178,16 +178,27 @@ pub fn decode_source_bytes( .filter(|&&b| b == b'\n') .count() + 1; - let (file_context, location_suffix) = if filename == "" { - (String::new(), format!(" ({filename}, line {line})")) - } else { - (format!(" in file {filename}"), String::new()) - }; - Err(crate::PyError::syntax_error(format!( - "Non-UTF-8 code starting with '\\x{bad_byte:02x}'{file_context} \ - on line {line}, but no encoding declared; \ - see https://peps.python.org/pep-0263/ for details{location_suffix}" - ))) + // The name may hold the surrogate escape an undecodable path + // byte becomes, so the message is assembled as WTF-8; + // `format!` would render it through `Display` as U+FFFD. + let named_string = filename.as_str() == Ok(""); + let mut message = rustpython_wtf8::Wtf8Buf::from_string(format!( + "Non-UTF-8 code starting with '\\x{bad_byte:02x}'" + )); + if !named_string { + message.push_str(" in file "); + message.push_wtf8(filename); + } + message.push_str(&format!( + " on line {line}, but no encoding declared; \ + see https://peps.python.org/pep-0263/ for details" + )); + if named_string { + message.push_str(" ("); + message.push_wtf8(filename); + message.push_str(&format!(", line {line})")); + } + Err(crate::PyError::syntax_error(message)) } } } else { @@ -205,7 +216,10 @@ pub fn decode_source_bytes( }; crate::PyError::syntax_error_located(message, filename, 0, 0, 0, 0, None) })?; - Ok(decoded.to_string_lossy().into_owned()) + // `pyparse.py:9-15 recode_to_utf8` re-encodes the decoded text to + // UTF-8, so a declared codec that yields a surrogate rejects the + // source rather than silently rewriting it. + crate::typedef::utf8_strict_w(decoded) } } diff --git a/pyre/pyre-interpreter/src/display.rs b/pyre/pyre-interpreter/src/display.rs index 09a34371362..045011dca6c 100644 --- a/pyre/pyre-interpreter/src/display.rs +++ b/pyre/pyre-interpreter/src/display.rs @@ -550,8 +550,8 @@ unsafe fn module_user_dunder_obj( /// `listobject.py:206-225 _listrepr_inner` assembles a container's repr in a /// `rutf8.Utf8StringBuilder` from each item's `space.utf8_len_w(space.repr(...))`, /// so a lone surrogate an item wrote survives being nested. A `Wtf8Buf` is the -/// buffer that can hold the same thing here; a Rust `String` cannot, which is -/// why [`py_repr`] is a lossy view of this rather than the other way round. +/// buffer that can hold the same thing here; a Rust `String` cannot, so every +/// caller reads the WTF-8 rather than a `String` round trip of it. pub unsafe fn py_repr_wtf8(obj: PyObjectRef) -> Result { // A tagged immediate must be formatted before `ob_type` touches it as a // pointer; `repr` of a plain `int` is its @@ -751,8 +751,12 @@ pub unsafe fn py_repr_wtf8(obj: PyObjectRef) -> Result // builtin values take this fast path instead of dispatching // through the `__repr__` the type registers in `typedef.rs`, so it // must produce the same address-bearing text. - let name = function_get_qualname(obj); - format!("") + // `format!` would render the WTF-8 qualname through `Display`, + // which substitutes U+FFFD for a lone surrogate. + let mut repr = Wtf8Buf::from_string("")); + return Ok(repr); } else if unsafe { pyre_object::is_exception(obj) } { // A user subclass that overrides `__repr__` shadows the builtin // `W_BaseException.descr_repr`; dispatch it before the native @@ -837,16 +841,23 @@ pub unsafe fn py_repr_wtf8(obj: PyObjectRef) -> Result crate::typedef::gettypeobject(&pyre_object::NONE_TYPE), ) { - parts.push("None".to_string()); + parts.push(Wtf8Buf::from_string("None".to_string())); } else { parts.push(crate::_pypy_generic_alias::repr_item(item)?); } } } - parts.join(" | ") + let mut joined = Wtf8Buf::new(); + for (index, part) in parts.iter().enumerate() { + if index > 0 { + joined.push_str(" | "); + } + joined.push_wtf8(part); + } + return Ok(joined); } else if std::ptr::eq(tp, &pyre_object::GENERIC_ALIAS_TYPE as *const PyType) { // GenericAlias.__repr__ (`_pypy_generic_alias.py:57`). - return Ok(Wtf8Buf::from_string(crate::_pypy_generic_alias::repr(obj)?)); + return crate::_pypy_generic_alias::repr(obj); } else if std::ptr::eq(tp, &MODULE_TYPE as *const PyType) { // A `types.ModuleType` subclass carries its class in `w_class`; a // subclass `__repr__` override wins over the native module @@ -854,7 +865,7 @@ pub unsafe fn py_repr_wtf8(obj: PyObjectRef) -> Result if let Some(r) = module_user_dunder_obj(obj, "__repr__")? { return Ok(pyre_object::w_str_get_wtf8(r).to_wtf8_buf()); } else { - crate::typedef::module_repr_string(obj)? + return crate::typedef::module_repr_string(obj); } } else if std::ptr::eq( tp, @@ -933,10 +944,10 @@ pub unsafe fn py_repr_wtf8(obj: PyObjectRef) -> Result return Ok(out); } else if pyre_object::interp_sre::is_sre_pattern(obj) { // `pypy/module/_sre/interp_sre.py:153 W_SRE_Pattern.repr_w`. - crate::module::_sre::interp_sre::sre_pattern_repr_str(obj)? + return crate::module::_sre::interp_sre::sre_pattern_repr_str(obj); } else if pyre_object::interp_sre::is_sre_match(obj) { // `pypy/module/_sre/interp_sre.py:684 W_SRE_Match.repr_w`. - crate::module::_sre::interp_sre::sre_match_repr_str(obj)? + return crate::module::_sre::interp_sre::sre_match_repr_str(obj); } else if pyre_object::memoryview::is_w_memoryview(obj) { // `memoryobject.py descr_repr` — ``, or // `` once the view is released. @@ -982,10 +993,61 @@ pub unsafe fn py_repr_wtf8(obj: PyObjectRef) -> Result } } -pub unsafe fn py_repr(obj: PyObjectRef) -> Result { - Ok(unsafe { py_repr_wtf8(obj) }?.to_string_lossy().into_owned()) +/// One piece of a [`wtf8_format!`] message. +/// +/// Text a Rust literal or `format!` produced is already `str`; text that came +/// from a Python object — a `repr`, a name, a filename — may hold a lone +/// surrogate and is carried as WTF-8. Both push their own bytes, so nothing on +/// the way to the message goes through `Display`. +pub trait Wtf8Piece { + fn push_onto(&self, out: &mut Wtf8Buf); +} + +impl Wtf8Piece for str { + fn push_onto(&self, out: &mut Wtf8Buf) { + out.push_str(self); + } +} + +impl Wtf8Piece for String { + fn push_onto(&self, out: &mut Wtf8Buf) { + out.push_str(self); + } +} + +impl Wtf8Piece for Wtf8 { + fn push_onto(&self, out: &mut Wtf8Buf) { + out.push_wtf8(self); + } +} + +impl Wtf8Piece for Wtf8Buf { + fn push_onto(&self, out: &mut Wtf8Buf) { + out.push_wtf8(self); + } +} + +impl Wtf8Piece for &T { + fn push_onto(&self, out: &mut Wtf8Buf) { + (**self).push_onto(out); + } } +/// `format!` for a message that interpolates text with no `str` spelling. +/// +/// `format!` renders every argument through `Display`, and `Display for Wtf8` +/// substitutes U+FFFD for a lone surrogate — so a message naming a `repr`, a +/// `__qualname__` or a filename silently loses it. Here the literal chunks stay +/// `format!` calls and the WTF-8 pieces are concatenated as themselves. +macro_rules! wtf8_format { + ($($piece:expr),+ $(,)?) => {{ + let mut buf = rustpython_wtf8::Wtf8Buf::new(); + $($crate::display::Wtf8Piece::push_onto(&$piece, &mut buf);)+ + buf + }}; +} +pub(crate) use wtf8_format; + /// Format for str() — tries __str__ first, then __repr__. pub unsafe fn py_str_wtf8(obj: PyObjectRef) -> Result { unsafe { @@ -1067,10 +1129,6 @@ pub unsafe fn py_str_wtf8(obj: PyObjectRef) -> Result { } } -pub unsafe fn py_str(obj: PyObjectRef) -> Result { - Ok(unsafe { py_str_wtf8(obj) }?.to_string_lossy().into_owned()) -} - /// `pypy/module/exceptions/interp_exceptions.py:126-133 /// W_BaseException.descr_str`: /// @@ -1090,12 +1148,6 @@ pub unsafe fn py_str(obj: PyObjectRef) -> Result { /// /// # Safety /// `obj` must be a live `W_BaseException`. -pub(crate) unsafe fn base_exception_str(obj: PyObjectRef) -> Result { - Ok(unsafe { base_exception_str_wtf8(obj) }? - .to_string_lossy() - .into_owned()) -} - pub(crate) unsafe fn base_exception_str_wtf8(obj: PyObjectRef) -> Result { // `space.str(self.args_w[0])` re-enters `py_str_wtf8` on the element, which // for `e.args = (e,)` lands back here. The re-entry pushes no Python frame @@ -1527,14 +1579,14 @@ unsafe fn exception_descr_str_wtf8(obj: PyObjectRef) -> Result, /// (used for `end - 1` arithmetic and the `end == start + 1` shape /// check); `Err(String)` carries the pre-formatted str-coerced /// fallback for direct interpolation into the message. -unsafe fn unicode_err_int_slot(stored: PyObjectRef) -> Result { +unsafe fn unicode_err_int_slot(stored: PyObjectRef) -> Result { unsafe { if stored.is_null() || pyre_object::is_none(stored) { // Never set / explicit None — PyPy class-default `w_start // = None`. `"%d" % None` raises, but in pyre py_str // cannot raise; surface "None" so the bad state is at // least visible. - return Err("None".to_string()); + return Err(Wtf8Buf::from_string("None".to_string())); } // `int_w` walks the __int__/__index__ protocol, so int // subclasses with stored intval (`class MyInt(int): pass`, @@ -1546,7 +1598,7 @@ unsafe fn unicode_err_int_slot(stored: PyObjectRef) -> Result { } // `descr_str` deliberately str-coerces rather than raising; a raising // `__str__` on the mutated slot degrades to empty here. - Err(py_str(stored).unwrap_or_default()) + Err(py_str_wtf8(stored).unwrap_or_default()) } } @@ -1573,9 +1625,9 @@ unsafe fn unicode_err_str_slot(stored: PyObjectRef) -> Result) -> String { +fn unicode_err_int_repr(slot: &Result) -> Wtf8Buf { match slot { - Ok(v) => v.to_string(), + Ok(v) => Wtf8Buf::from_string(v.to_string()), Err(s) => s.clone(), } } @@ -1584,9 +1636,9 @@ fn unicode_err_int_repr(slot: &Result) -> String { /// On an int slot, arithmetic; on the str-coerced fallback, the /// value is embedded verbatim so the message still reflects what the /// user actually stored. -fn unicode_err_end_minus_one_repr(slot: &Result) -> String { +fn unicode_err_end_minus_one_repr(slot: &Result) -> Wtf8Buf { match slot { - Ok(v) => (v - 1).to_string(), + Ok(v) => Wtf8Buf::from_string((v - 1).to_string()), Err(s) => s.clone(), } } @@ -1684,19 +1736,24 @@ unsafe fn unicode_translate_error_str(obj: PyObjectRef) -> Result".to_string()), + let mut out = wtf8_format!( + format!( + "can't translate character {} in position ", + badchar_repr.unwrap_or_else(|| "".to_string()), + ), start_repr, - )); + ": ", + ); out.push_wtf8(&reason); return Ok(out); } - let mut out = Wtf8Buf::from_string(format!( - "can't translate characters in position {}-{}: ", + let mut out = wtf8_format!( + "can't translate characters in position ", start_repr, + "-", unicode_err_end_minus_one_repr(&end_slot), - )); + ": ", + ); out.push_wtf8(&reason); Ok(out) } @@ -1772,21 +1829,22 @@ unsafe fn unicode_decode_error_str(obj: PyObjectRef) -> Result Result".to_string()), - start_repr, )); + out.push_wtf8(&start_repr); + out.push_str(": "); out.push_wtf8(&reason); return Ok(out); } let mut out = Wtf8Buf::new(); out.push_str("'"); out.push_wtf8(&encoding); - out.push_str(&format!( - "' codec can't encode characters in position {}-{}: ", - start_repr, - unicode_err_end_minus_one_repr(&end_slot), - )); + out.push_str("' codec can't encode characters in position "); + out.push_wtf8(&start_repr); + out.push_str("-"); + out.push_wtf8(&unicode_err_end_minus_one_repr(&end_slot)); + out.push_str(": "); out.push_wtf8(&reason); Ok(out) } @@ -1894,8 +1953,9 @@ impl fmt::Display for PyDisplay { // diagnostic output context degrades to a placeholder rather than // propagating (the user-facing `print()`/`str()` paths thread the // error through `py_str`). - let s = - unsafe { py_str(self.0) }.unwrap_or_else(|_| "".to_string()); + let s = unsafe { py_str_wtf8(self.0) } + .map(|w| wtf8_display_string(w, "")) + .unwrap_or_else(|_| "".to_string()); write!(f, "{s}") } } diff --git a/pyre/pyre-interpreter/src/error.rs b/pyre/pyre-interpreter/src/error.rs index 307f03ea36d..76c6ae772a7 100644 --- a/pyre/pyre-interpreter/src/error.rs +++ b/pyre/pyre-interpreter/src/error.rs @@ -1,5 +1,7 @@ use pyre_object::PyObjectRef; -use pyre_object::interp_exceptions::{ExcKind, exc_kind_name, w_exception_new}; +use pyre_object::interp_exceptions::{ + ExcKind, exc_kind_name, w_exception_new, w_exception_new_wtf8, +}; use ruff_text_size::Ranged; use rustpython_compiler::{ast, parser}; use rustpython_wtf8::Wtf8Buf; @@ -243,16 +245,20 @@ impl OperationError { /// A direct `raise non_exception`, which never calls a constructor, keeps /// the ordinary "exceptions must derive from BaseException" wording. pub fn exception_from_call_type_error(w_constructor: PyObjectRef, w_inst: PyObjectRef) -> PyError { - let constructor = unsafe { crate::display::py_repr(w_constructor) } - .unwrap_or_else(|_| "".to_string()); + let constructor = unsafe { crate::display::py_repr_wtf8(w_constructor) } + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())); let w_type = crate::baseobjspace::exception_getclass(w_inst); + let unknown = || Wtf8Buf::from_string("".to_string()); let returned_type = if w_type.is_null() { - "".to_string() + unknown() } else { - unsafe { crate::display::py_repr(w_type) }.unwrap_or_else(|_| "".to_string()) + unsafe { crate::display::py_repr_wtf8(w_type) }.unwrap_or_else(|_| unknown()) }; - PyError::type_error(format!( - "calling {constructor} should have returned an instance of BaseException, not {returned_type}" + PyError::type_error(crate::display::wtf8_format!( + "calling ", + constructor, + " should have returned an instance of BaseException, not ", + returned_type )) } @@ -334,9 +340,9 @@ pub fn chain_context(exc: PyObjectRef, active: PyObjectRef) { impl From for PyError { fn from(value: OperationError) -> Self { let message = if value.w_value.is_null() { - String::new() + Wtf8Buf::new() } else { - "operation error".to_string() + Wtf8Buf::from_string("operation error".to_string()) }; PyError { kind: PyErrorKind::RuntimeError, @@ -363,7 +369,7 @@ pub type PyResult = Result; #[derive(Debug, Clone)] pub struct PyError { pub kind: PyErrorKind, - pub message: String, + pub message: Wtf8Buf, /// Cached W_BaseException pointer — reused by to_exc_object() /// to avoid re-allocating an exception object that already exists. pub exc_object: PyObjectRef, @@ -532,7 +538,7 @@ pub enum PyErrorKind { } impl PyError { - pub fn new(kind: PyErrorKind, message: impl Into) -> Self { + pub fn new(kind: PyErrorKind, message: impl Into) -> Self { PyError { kind, message: message.into(), @@ -544,11 +550,11 @@ impl PyError { } } - pub fn type_error(msg: impl Into) -> Self { + pub fn type_error(msg: impl Into) -> Self { Self::new(PyErrorKind::TypeError, msg) } - pub fn attribute_error(msg: impl Into) -> Self { + pub fn attribute_error(msg: impl Into) -> Self { Self::new(PyErrorKind::AttributeError, msg) } @@ -557,7 +563,7 @@ impl PyError { /// and the `obj` it was looked up on so `e.name` / `e.obj` read back /// once the instance is materialised (Python 3.10+). pub fn attribute_error_with_context( - msg: impl Into, + msg: impl Into, w_obj: PyObjectRef, name: &str, ) -> Self { @@ -567,7 +573,7 @@ impl PyError { err } - pub fn value_error(msg: impl Into) -> Self { + pub fn value_error(msg: impl Into) -> Self { Self::new(PyErrorKind::ValueError, msg) } @@ -587,7 +593,7 @@ impl PyError { } } - pub fn syntax_error(msg: impl Into) -> Self { + pub fn syntax_error(msg: impl Into) -> Self { Self::new(PyErrorKind::SyntaxError, msg) } @@ -601,8 +607,8 @@ impl PyError { /// offending source line (`None` → `text` reads back as `None`). #[allow(clippy::too_many_arguments)] pub fn syntax_error_located( - msg: impl Into, - filename: &str, + msg: impl Into, + filename: &rustpython_wtf8::Wtf8, lineno: i64, offset: i64, end_lineno: i64, @@ -615,7 +621,7 @@ impl PyError { // `w_tuple_new` / `w_list_new` run, so a collection there could sweep // the unrooted exception before `w_exception_set_args` writes it. let _roots = pyre_object::gc_roots::push_roots(); - let exc = w_exception_new(ExcKind::SyntaxError, &message); + let exc = w_exception_new_wtf8(ExcKind::SyntaxError, &message); let exc_slot = pyre_object::gc_roots::shadow_stack_len(); pyre_object::gc_roots::pin_root(exc); // Build the `(filename, lineno, offset, text, end_lineno, end_offset)` @@ -625,7 +631,7 @@ impl PyError { // young element (the filename or text string, or an uncached line/col // int), leaving its raw local pointing at the old address. let filename_slot = pyre_object::gc_roots::shadow_stack_len(); - pyre_object::gc_roots::pin_root(pyre_object::w_str_new(filename)); + pyre_object::gc_roots::pin_root(pyre_object::w_str_from_wtf8(filename.to_wtf8_buf())); let lineno_slot = pyre_object::gc_roots::shadow_stack_len(); pyre_object::gc_roots::pin_root(pyre_object::w_int_new(lineno)); let offset_slot = pyre_object::gc_roots::shadow_stack_len(); @@ -678,7 +684,7 @@ impl PyError { // becomes `args[1]`. let details_slot = pyre_object::gc_roots::shadow_stack_len(); pyre_object::gc_roots::pin_root(details); - let w_msg = pyre_object::w_str_new(&message); + let w_msg = pyre_object::w_str_from_wtf8(message.clone()); let msg_slot = pyre_object::gc_roots::shadow_stack_len(); pyre_object::gc_roots::pin_root(w_msg); unsafe { @@ -698,7 +704,7 @@ impl PyError { // Leave the display message empty so `message_text` derives it from // `exc_object` via the SyntaxError `descr_str`, which appends the // `(filename, line N)` suffix. - message: String::new(), + message: Wtf8Buf::new(), exc_object: exc, attach_tb: true, reraise_lasti: -1, @@ -774,30 +780,30 @@ impl PyError { }; } - pub fn zero_division(msg: impl Into) -> Self { + pub fn zero_division(msg: impl Into) -> Self { Self::new(PyErrorKind::ZeroDivisionError, msg) } - pub fn overflow_error(msg: impl Into) -> Self { + pub fn overflow_error(msg: impl Into) -> Self { Self::new(PyErrorKind::OverflowError, msg) } - pub fn runtime_error(msg: impl Into) -> Self { + pub fn runtime_error(msg: impl Into) -> Self { Self::new(PyErrorKind::RuntimeError, msg) } - pub fn not_implemented(msg: impl Into) -> Self { + pub fn not_implemented(msg: impl Into) -> Self { Self::new(PyErrorKind::NotImplementedError, msg) } - pub fn system_error(msg: impl Into) -> Self { + pub fn system_error(msg: impl Into) -> Self { Self::new(PyErrorKind::SystemError, msg) } /// `_PyEval_FormatExcCheckArg` parity — a NameError carrying the /// undefined `name` so `e.name` reads back once the instance is /// materialised (Python 3.10+). - pub fn name_error_with_name(msg: impl Into, name: &str) -> Self { + pub fn name_error_with_name(msg: impl Into, name: &str) -> Self { let mut err = Self::new(PyErrorKind::NameError, msg); err.w_name_context = pyre_object::w_str_new(name); err @@ -807,7 +813,7 @@ impl PyError { /// rides only in the message: `format_exc_check_arg` stamps the `name` /// slot when the raised class is exactly `NameError`, so an /// `UnboundLocalError` reaches Python with `name` still `None`. - pub fn unbound_local_error(msg: impl Into) -> Self { + pub fn unbound_local_error(msg: impl Into) -> Self { Self::new(PyErrorKind::UnboundLocalError, msg) } @@ -816,14 +822,14 @@ impl PyError { /// machinery raises `ModuleNotFoundError(msg, name=fullname)`. The /// `name` rides the shared `w_n` slot (ImportError / NameError / /// AttributeError), stamped by `to_exc_object`. - pub fn module_not_found_with_name(msg: impl Into, name: &str) -> Self { + pub fn module_not_found_with_name(msg: impl Into, name: &str) -> Self { Self::module_not_found_with_name_obj(msg, pyre_object::w_str_new(name)) } /// `module_not_found_with_name` for a name with no `&str` spelling, so the /// caller supplies the name object itself. pub fn module_not_found_with_name_obj( - msg: impl Into, + msg: impl Into, w_name: pyre_object::PyObjectRef, ) -> Self { let mut err = Self::new(PyErrorKind::ModuleNotFoundError, msg); @@ -831,13 +837,13 @@ impl PyError { err } - pub fn internal_trace_abort(reason: impl Into) -> Self { + pub fn internal_trace_abort(reason: impl Into) -> Self { let mut err = Self::new(PyErrorKind::TraceAbort, reason); err.attach_tb = false; err } - pub fn key_error(msg: impl Into) -> Self { + pub fn key_error(msg: impl Into) -> Self { Self::new(PyErrorKind::KeyError, msg) } @@ -860,12 +866,12 @@ impl PyError { pyre_object::gc_roots::pin_root(key); } let message = if key.is_null() { - "".to_string() + Wtf8Buf::from_string("".to_string()) } else { - unsafe { crate::display::py_repr(key) } - .unwrap_or_else(|_| "".to_string()) + unsafe { crate::display::py_repr_wtf8(key) } + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())) }; - let exc = pyre_object::interp_exceptions::w_exception_new(ExcKind::KeyError, &message); + let exc = w_exception_new_wtf8(ExcKind::KeyError, &message); pyre_object::gc_roots::pin_root(exc); if !key.is_null() { // Reload the key after the repr / exception allocations: the pin @@ -886,15 +892,15 @@ impl PyError { } } - pub fn index_error(msg: impl Into) -> Self { + pub fn index_error(msg: impl Into) -> Self { Self::new(PyErrorKind::IndexError, msg) } - pub fn lookup_error(msg: impl Into) -> Self { + pub fn lookup_error(msg: impl Into) -> Self { Self::new(PyErrorKind::LookupError, msg) } - pub fn os_error(msg: impl Into) -> Self { + pub fn os_error(msg: impl Into) -> Self { Self::new(PyErrorKind::OSError, msg) } @@ -1065,7 +1071,7 @@ impl PyError { // `exc_object` via `W_OSError.descr_str`, which appends the // `: 'filename'` suffix; the bare "[Errno N] strerror" would bypass // it and the uncaught-traceback header would drop the filename. - message: String::new(), + message: Wtf8Buf::new(), exc_object: exc, attach_tb: true, reraise_lasti: -1, @@ -1076,7 +1082,7 @@ impl PyError { /// Raise the structured OSError for `errno` (no filename). The caller's /// platform message is superseded by the errno-derived strerror. - pub fn os_error_with_errno(errno: i32, _msg: impl Into) -> Self { + pub fn os_error_with_errno(errno: i32, _msg: impl Into) -> Self { Self::os_error_syscall(errno, pyre_object::PY_NULL) } @@ -1106,7 +1112,7 @@ impl PyError { // Leave the display message empty so `message_text` derives it // from `exc_object`, whose `descr_str` renders the two-element // `args` as a tuple repr rather than as a bare string. - message: String::new(), + message: Wtf8Buf::new(), exc_object: exc, attach_tb: true, reraise_lasti: -1, @@ -1121,7 +1127,7 @@ impl PyError { /// `e.errno` / `e.strerror` read back the two values. Like /// `OSError(errno, strerror)`, every errno in CPython's/PyPy's errno map /// selects its concrete subclass (EAGAIN → BlockingIOError, etc.). - pub fn os_error_errno_strerror(errno: i32, strerror: impl Into) -> Self { + pub fn os_error_errno_strerror(errno: i32, strerror: impl Into) -> Self { let strerror = strerror.into(); let subclass = crate::builtins::os_error_errno_subclass(errno as i64); let kind = if matches!(subclass, Some("FileNotFoundError")) { @@ -1134,13 +1140,16 @@ impl PyError { } else { ExcKind::OSError }; - let message = format!("[Errno {errno}] {strerror}"); + // `strerror` reaches Python as `e.strerror` and as `args[1]`, so it is + // WTF-8 and the assembled `[Errno N] ...` line has to be too. + let mut message = Wtf8Buf::from_string(format!("[Errno {errno}] ")); + message.push_wtf8(&strerror); // Root the fresh exception across the args allocation below: `exc` // lives only in this Rust local while `w_int_new` / `w_str_new` / // `w_list_new` run, so a collection there could sweep the unrooted // exception before `w_exception_set_args` writes through it. let _roots = pyre_object::gc_roots::push_roots(); - let exc = w_exception_new(exc_kind, &message); + let exc = w_exception_new_wtf8(exc_kind, &message); pyre_object::gc_roots::pin_root(exc); if let Some(w_target) = subclass.and_then(crate::builtins::lookup_exc_class) { unsafe { @@ -1149,7 +1158,7 @@ impl PyError { } let args_list = pyre_object::interp_exceptions::w_exception_args_new(vec![ pyre_object::w_int_new(errno as i64), - pyre_object::w_str_new(&strerror), + pyre_object::w_str_from_wtf8(strerror.clone()), ]); unsafe { pyre_object::interp_exceptions::w_exception_set_args(exc, args_list); @@ -1159,7 +1168,7 @@ impl PyError { ); pyre_object::interp_exceptions::w_exception_set_strerror( exc, - pyre_object::w_str_new(&strerror), + pyre_object::w_str_from_wtf8(strerror.clone()), ); } PyError { @@ -1178,7 +1187,7 @@ impl PyError { /// this so `.name` (the package) and `.path` (its file) are readable, /// which `PyError::new(ImportError, msg)` cannot carry. pub fn import_error_name_path( - msg: impl Into, + msg: impl Into, w_name: PyObjectRef, w_path: PyObjectRef, ) -> Self { @@ -1197,14 +1206,14 @@ impl PyError { if !w_path.is_null() { pyre_object::gc_roots::pin_root(w_path); } - let exc = w_exception_new(ExcKind::ImportError, &message); + let exc = w_exception_new_wtf8(ExcKind::ImportError, &message); pyre_object::gc_roots::pin_root(exc); // `ImportError.__init__` mirrors args[0] into the dedicated `msg` // slot; the prebuilt-instance path bypasses it, so stamp it here. let w_msg = if message.is_empty() { pyre_object::w_none() } else { - pyre_object::w_str_new(&message) + pyre_object::w_str_from_wtf8(message.clone()) }; // Reload name/path after the message allocation: the pins keep them // alive, but a minor collection may have relocated the young objects, @@ -1238,11 +1247,11 @@ impl PyError { /// pypy/module/_weakref/interp__weakref.py:347 — raised by `force()` /// when the referent of a proxy is no longer alive. - pub fn reference_error(msg: impl Into) -> Self { + pub fn reference_error(msg: impl Into) -> Self { Self::new(PyErrorKind::ReferenceError, msg) } - pub fn recursion_error(msg: impl Into) -> Self { + pub fn recursion_error(msg: impl Into) -> Self { Self::new(PyErrorKind::RecursionError, msg) } @@ -1250,7 +1259,7 @@ impl PyError { /// — module-level singleton instance the JIT raises through /// `PropagateExceptionDescr.handle_fail` when a malloc helper /// returns NULL. - pub fn memory_error(msg: impl Into) -> Self { + pub fn memory_error(msg: impl Into) -> Self { Self::new(PyErrorKind::MemoryError, msg) } @@ -1307,7 +1316,7 @@ impl PyError { pyre_object::gc_roots::pin_root(exc); if !self.message.is_empty() { let msg_slot = pyre_object::gc_roots::shadow_stack_len(); - let msg = pyre_object::w_str_new(&self.message); + let msg = pyre_object::w_str_from_wtf8(self.message.clone()); pyre_object::gc_roots::pin_root(msg); let args_list = pyre_object::interp_exceptions::w_exception_args_new(vec![msg]); unsafe { pyre_object::interp_exceptions::w_exception_set_args(exc, args_list) }; @@ -1369,7 +1378,7 @@ impl PyError { pub fn write_unraisable( &mut self, space: PyObjectRef, - where_desc: &str, + where_desc: &rustpython_wtf8::Wtf8, w_object: PyObjectRef, ) { let w_value = self @@ -1395,12 +1404,14 @@ impl PyError { } else { w_object }; + // The description names an object — a thread's callable, a ctypes + // callback — so it may hold a lone surrogate and is carried as WTF-8. let mut first_line = if where_desc.is_empty() { - String::new() - } else if where_desc.starts_with("Exception ignored ") { - where_desc.to_string() + Wtf8Buf::new() + } else if where_desc.as_bytes().starts_with(b"Exception ignored ") { + where_desc.to_wtf8_buf() } else { - format!("Exception ignored in: {where_desc}") + crate::display::wtf8_format!("Exception ignored in: ", where_desc) }; // vm.py:19-25 also carries `extra_line`; pyre's hook-args // structseq targets the 5-field shape, so only the default printer @@ -1414,7 +1425,7 @@ impl PyError { if first_line.is_empty() { pyre_object::w_none() } else { - pyre_object::w_str_new(&first_line) + pyre_object::w_str_from_wtf8(first_line.clone()) }, w_object, ], @@ -1425,7 +1436,9 @@ impl PyError { match crate::call::call_function_impl_result(w_hook, &[hook_args]) { Ok(_) => return, Err(mut hook_err) => { - first_line = "Exception ignored in sys.unraisablehook".to_string(); + first_line = Wtf8Buf::from_string( + "Exception ignored in sys.unraisablehook".to_string(), + ); w_object = w_hook; let hook_value = hook_err .normalize_exception(space) @@ -1477,8 +1490,13 @@ impl PyError { Ok(w) if !w.is_null() && !unsafe { pyre_object::is_none(w) } => w, _ => return false, }; - let text = String::from_utf8_lossy(buf).into_owned(); - let w_text = pyre_object::w_str_new(&text); + // The buffer is assembled from text the printer already escaped, so + // it is well-formed; a decode failure would mean a byte no writer put + // there, and the lossy read is the last resort for it. + let w_text = match rustpython_wtf8::Wtf8::from_bytes(buf) { + Some(text) => pyre_object::w_str_from_wtf8(text.to_wtf8_buf()), + None => pyre_object::w_str_new(&String::from_utf8_lossy(buf)), + }; let result = crate::baseobjspace::call_method(stderr, "write", &[w_text]); if result.is_null() { let _ = crate::call::take_call_error(); @@ -1493,20 +1511,21 @@ impl PyError { w_type: PyObjectRef, w_value: PyObjectRef, w_tb: PyObjectRef, - first_line: &str, + first_line: &rustpython_wtf8::Wtf8, w_object: PyObjectRef, extra_line: &str, ) { let _ = (space, w_type); let mut first_line = if first_line.is_empty() { - "Exception ignored in:".to_string() + Wtf8Buf::from_string("Exception ignored in:".to_string()) } else { - first_line.to_string() + first_line.to_wtf8_buf() }; if !w_object.is_null() && !unsafe { pyre_object::is_none(w_object) } { - let objrepr = unsafe { crate::display::py_repr(w_object) } - .unwrap_or_else(|_| "".to_string()); - first_line = format!("{first_line} {objrepr}"); + let objrepr = unsafe { crate::display::py_repr_wtf8(w_object) } + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())); + first_line.push_str(" "); + first_line.push_wtf8(&objrepr); } let extra_line = if extra_line.is_empty() { "\n".to_string() @@ -1514,7 +1533,9 @@ impl PyError { format!("{extra_line}:\n") }; let mut buf = Vec::new(); - let _ = write!(&mut buf, "{first_line}"); + // The stream takes text, and the printer spells a lone surrogate as + // the escape it prints elsewhere rather than folding it to U+FFFD. + let _ = buf.write_all(first_line.as_bytes()); if !extra_line.is_empty() { let _ = write!(&mut buf, "{extra_line}"); } @@ -1526,7 +1547,7 @@ impl PyError { let _ = write_exception(&mut buf, &err, true); } if !Self::write_unraisable_to_sys_stderr(&buf) { - crate::host_seam::emit_stderr(&buf); + emit_report_to_host_stderr(&buf); } } @@ -1591,7 +1612,7 @@ impl PyError { // display time. PyError { kind: Self::kind_from_exc(kind), - message: String::new(), + message: Wtf8Buf::new(), exc_object: obj, attach_tb: true, reraise_lasti: -1, @@ -1657,7 +1678,11 @@ impl PyError { /// `__str__` runs at display time, not at raise time. pub fn message_text(&self) -> String { if !self.message.is_empty() || self.exc_object.is_null() { - return self.message.clone(); + // Display-side only: `message` is WTF-8 and may hold a lone + // surrogate, so spend the escape the stream would spend rather + // than folding it to U+FFFD. A caller that needs the value + // itself wants [`message_wtf8`]. + return crate::display::wtf8_display_string(self.message.clone(), ""); } // Infallible Display-side context: a raising `__str__` degrades to // the placeholder rather than propagating, and a lone surrogate is @@ -1665,15 +1690,33 @@ impl PyError { unsafe { crate::display::py_str_display(self.exc_object) } } + /// [`message_text`] without the display encode — the text as a value. + /// + /// The exception's own `args[0]` is minted from this, so an unpaired + /// surrogate in a message that names a user string has to survive here; + /// `message_text` is for the diagnostic streams only. + pub fn message_wtf8(&self) -> Wtf8Buf { + if !self.message.is_empty() || self.exc_object.is_null() { + return self.message.clone(); + } + unsafe { crate::display::py_str_wtf8(self.exc_object) } + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())) + } + pub fn render_exception(&self) -> String { + crate::display::wtf8_display_string(self.render_exception_wtf8(), "") + } + + /// [`render_exception`](Self::render_exception) as the report's own WTF-8, + /// for a writer assembling a buffer whose sink spends the encode. + pub(crate) fn render_exception_wtf8(&self) -> Wtf8Buf { let name = exc_object_class_name(self.exc_object) .unwrap_or_else(|| exc_kind_name(self.to_exc_kind()).to_string()); - let message = self.message_text(); + let message = self.message_wtf8(); if message.is_empty() { - name - } else { - format!("{name}: {message}") + return Wtf8Buf::from_string(name); } + crate::display::wtf8_format!(name, ": ", message) } } @@ -1748,7 +1791,8 @@ pub fn write_exception( include_traceback: bool, ) -> std::io::Result<()> { if !include_traceback { - return writeln!(writer, "{}", err.render_exception()); + writer.write_all(err.render_exception_wtf8().as_bytes())?; + return writer.write_all(b"\n"); } if !err.exc_object.is_null() && unsafe { pyre_object::is_exception(err.exc_object) } { // The instance carries the whole report: the cause/context chain, the @@ -1766,7 +1810,8 @@ pub fn write_exception( // no group and no frame list — only the header. writeln!(writer, "Traceback (most recent call last):")?; write_traceback_chain(writer, err)?; - writeln!(writer, "{}", err.render_exception()) + writer.write_all(err.render_exception_wtf8().as_bytes())?; + writer.write_all(b"\n") } /// CPython 3.14 `_PyErr_Display(file, exc_type, exc_value, exc_tb)` shape used @@ -1831,7 +1876,8 @@ pub fn write_exception_from_parts( pub fn write_syntax_error(writer: &mut W, err: &PyError) -> std::io::Result<()> { let exc = err.exc_object; if exc.is_null() || !unsafe { pyre_object::is_exception(exc) } { - return writeln!(writer, "{}", err.render_exception()); + writer.write_all(err.render_exception_wtf8().as_bytes())?; + return writer.write_all(b"\n"); } write_syntax_error_object(writer, exc) } @@ -1845,31 +1891,31 @@ fn write_syntax_error_object(writer: &mut W, exc: PyObjectRef) -> std: crate::baseobjspace::syntax_error_attr(exc, name) }; // `filename` is the compiled file's own name, so it carries whatever - // surrogate escapes the filesystem encoding produced, and `text` is a line - // of that file. Printing a traceback must not abort for want of a UTF-8 - // view: escape what has no spelling, the way stderr already does. + // surrogate escapes the filesystem encoding produced, and `msg` may quote + // it. Both go out as the report's own WTF-8; the sink spends the + // `backslashreplace` encode. + let wtf8_of = |w: PyObjectRef| -> Option { + (!w.is_null() && unsafe { pyre_object::is_str(w) }) + .then(|| unsafe { pyre_object::w_str_get_wtf8(w) }.to_owned()) + }; + // `text` is a line of source, which the compiler only accepted as UTF-8, so + // the caret arithmetic below can work on a `str`. let str_of = |w: PyObjectRef| -> Option { - (!w.is_null() && unsafe { pyre_object::is_str(w) }).then(|| { - let wtf8 = unsafe { pyre_object::w_str_get_wtf8(w) }; - match wtf8.as_str() { - Ok(s) => s.to_owned(), - Err(_) => { - let s_obj = pyre_object::w_str_from_wtf8(wtf8.to_owned()); - crate::type_methods::encode_object(s_obj, "utf-8", "backslashreplace") - .map(|b| String::from_utf8_lossy(&b).into_owned()) - .unwrap_or_else(|_| "".to_string()) - } - } + wtf8_of(w).map(|wtf8| match wtf8.as_str() { + Ok(s) => s.to_owned(), + Err(_) => "".to_string(), }) }; let int_of = |w: PyObjectRef| -> Option { (!w.is_null() && unsafe { pyre_object::is_int(w) }) .then(|| unsafe { pyre_object::intobject::w_int_get_value(w) }) }; - let filename = str_of(attr("filename")); + let filename = wtf8_of(attr("filename")); let lineno = int_of(attr("lineno")); if let (Some(fname), Some(lineno)) = (filename.as_ref(), lineno) { - writeln!(writer, " File \"{fname}\", line {lineno}")?; + writer.write_all(b" File \"")?; + writer.write_all(fname.as_bytes())?; + writeln!(writer, "\", line {lineno}")?; } if let Some(text) = str_of(attr("text")) { let raw = text.trim_end_matches(['\n', '\r']); @@ -1898,8 +1944,12 @@ fn write_syntax_error_object(writer: &mut W, exc: PyObjectRef) -> std: } let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); let name = exc_object_class_name(exc).unwrap_or_else(|| "SyntaxError".to_string()); - match str_of(attr("msg")) { - Some(msg) if !msg.is_empty() => writeln!(writer, "{name}: {msg}"), + match wtf8_of(attr("msg")) { + Some(msg) if !msg.is_empty() => { + write!(writer, "{name}: ")?; + writer.write_all(msg.as_bytes())?; + writer.write_all(b"\n") + } _ => writeln!(writer, "{name}"), } } @@ -1909,7 +1959,7 @@ fn write_syntax_error_object(writer: &mut W, exc: PyObjectRef) -> std: pub fn eprint_syntax_error(err: &PyError) { let mut buf: Vec = Vec::new(); let _ = write_syntax_error(&mut buf, err); - crate::host_seam::emit_stderr(&buf); + emit_report_to_host_stderr(&buf); } /// `lib-python/3/traceback.py:980-1000 _ExceptionPrintContext` and @@ -2029,8 +2079,7 @@ fn write_plain_exception_object( { write_syntax_error_object(&mut rendered, exc)?; } else { - let header = render_rooted_exc_object_display(exc_slot); - rendered.write_all(header.as_bytes())?; + rendered.write_all(render_rooted_exc_object_wtf8(exc_slot).as_bytes())?; rendered.write_all(b"\n")?; } let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); @@ -2096,8 +2145,7 @@ fn write_exception_group( } let mut header = Vec::new(); - let group_header = render_rooted_exc_object_display(exc_slot); - header.write_all(group_header.as_bytes())?; + header.write_all(render_rooted_exc_object_wtf8(exc_slot).as_bytes())?; header.write_all(b"\n")?; let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); write_exception_notes(&mut header, exc)?; @@ -2240,8 +2288,8 @@ fn write_chained_context_inner( fn write_single_exception(writer: &mut W, exc: PyObjectRef) -> std::io::Result<()> { writeln!(writer, "Traceback (most recent call last):")?; write_traceback_chain_from_exc(writer, exc)?; - let render = render_exc_object(exc); - writeln!(writer, "{}", render)?; + writer.write_all(render_exc_object_wtf8(exc).as_bytes())?; + writer.write_all(b"\n")?; write_exception_notes(writer, exc) } @@ -2270,9 +2318,11 @@ fn write_exception_notes(writer: &mut W, exc: PyObjectRef) -> std::io: let err_obj = pyre_object::gc_roots::shadow_stack_get( pyre_object::gc_roots::shadow_stack_len() - 1, ); - let rendered = unsafe { crate::display::py_repr(err_obj) } - .unwrap_or_else(|_| "".to_string()); - return writeln!(writer, "Ignored error getting __notes__: {rendered}"); + let rendered = unsafe { crate::display::py_repr_wtf8(err_obj) } + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())); + writer.write_all(b"Ignored error getting __notes__: ")?; + writer.write_all(rendered.as_bytes())?; + return writer.write_all(b"\n"); } }; let notes_slot = pyre_object::gc_roots::shadow_stack_len(); @@ -2333,9 +2383,10 @@ fn write_exception_notes(writer: &mut W, exc: PyObjectRef) -> std::io: return Ok(()); } let notes = pyre_object::gc_roots::shadow_stack_get(notes_slot); - let rendered = unsafe { crate::display::py_repr(notes) } - .unwrap_or_else(|_| "".to_string()); - writeln!(writer, "{rendered}") + let rendered = unsafe { crate::display::py_repr_wtf8(notes) } + .unwrap_or_else(|_| Wtf8Buf::from_string("".to_string())); + writer.write_all(rendered.as_bytes())?; + writer.write_all(b"\n") } fn notes_is_abc_sequence(notes_slot: usize) -> bool { @@ -2363,25 +2414,6 @@ fn notes_is_abc_sequence(notes_slot: usize) -> bool { crate::baseobjspace::isinstance(notes, sequence).unwrap_or(false) } -/// Compose the `ExcName: msg` header for a W_BaseException — -/// equivalent to `traceback.format_exception_only`'s last line. -fn render_exc_object(exc: PyObjectRef) -> String { - crate::display::wtf8_display_string(render_exc_object_wtf8(exc), "") -} - -/// [`render_rooted_exc_object_wtf8`] as the bytes stderr should receive. -/// -/// The callers that assemble the report into a byte buffer have to spend the -/// `backslashreplace` encode themselves; writing the `Wtf8Buf` straight out -/// puts the raw surrogate bytes on the stream, which is not what -/// `errors='backslashreplace'` produces and is not valid UTF-8. -fn render_rooted_exc_object_display(exc_slot: usize) -> String { - crate::display::wtf8_display_string( - render_rooted_exc_object_wtf8(exc_slot), - "", - ) -} - fn render_rooted_exc_object_wtf8(exc_slot: usize) -> Wtf8Buf { let exc = pyre_object::gc_roots::shadow_stack_get(exc_slot); if exc.is_null() || !unsafe { pyre_object::is_exception(exc) } { @@ -3072,9 +3104,12 @@ fn read_source_line(filename: &[u8], lineno: i64) -> Option { // for a file declaring anything else. The name reaches the decode only // to report a failure this caller discards, so the lossy spelling of a // path with no UTF-8 form is enough there. - let content = - crate::compile::decode_source_bytes(&bytes, &String::from_utf8_lossy(filename), false) - .ok()?; + let content = crate::compile::decode_source_bytes( + &bytes, + &crate::gateway::fsdecode_filename_wtf8(filename), + false, + ) + .ok()?; content .lines() .nth((lineno - 1) as usize) @@ -3092,13 +3127,31 @@ fn read_source_line(filename: &[u8], lineno: i64) -> Option { } } +/// Write a rendered report to the host's stderr, spending the encode the +/// stream object would have. +/// +/// A report is assembled as WTF-8 throughout, which is what `sys.stderr.write` +/// takes: a lone surrogate reaches the stream as itself and its +/// `errors='backslashreplace'` decides how it is spelled, exactly as +/// `PyErr_Display` leaves that to the stream. A raw fd has no codec behind it, +/// so a caller writing to one owes that encode itself -- putting the WTF-8 +/// bytes straight on the stream would emit a sequence that is not valid UTF-8. +pub(crate) fn emit_report_to_host_stderr(buf: &[u8]) { + let text = match rustpython_wtf8::Wtf8::from_bytes(buf) { + Some(report) => crate::display::wtf8_display_string(report.to_wtf8_buf(), ""), + // A byte no writer put there; the lossy read is the last resort for it. + None => String::from_utf8_lossy(buf).into_owned(), + }; + crate::host_seam::emit_stderr(text.as_bytes()); +} + pub fn eprint_exception(err: &PyError, include_traceback: bool) { // Buffer then emit through the host_seam so the traceback rides the same // mediated stderr as sys.stderr under sandbox (raw fd 2 would bypass the // controller / corrupt nothing but escape the seam). let mut buf: Vec = Vec::new(); let _ = write_exception(&mut buf, err, include_traceback); - crate::host_seam::emit_stderr(&buf); + emit_report_to_host_stderr(&buf); } /// `app_main.py:114-129 handle_sys_exit` — `exitcode = e.code`; `None` exits @@ -3133,8 +3186,9 @@ pub fn system_exit_code(err: &PyError) -> i32 { // and `SystemExit(-1)` both exit 255. return crate::baseobjspace::int_w(code).unwrap_or(-1) as i32; } - let text = - unsafe { crate::display::py_str(code) }.unwrap_or_else(|_| "".to_string()); + // Straight to the host's stderr, which has no codec behind it, so the + // text spends the `backslashreplace` encode here. + let text = unsafe { crate::display::py_str_display(code) }; crate::host_seam::emit_stderr(format!("{text}\n").as_bytes()); 1 } diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 2f145da1437..72e2118615c 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -4260,8 +4260,8 @@ impl OpcodeStepExecutor for PyFrame { } // No `sys` yet (early bootstrap) — native repr print. if !unsafe { pyre_object::is_none(val) } { - let s = unsafe { crate::py_repr(val)? }; - crate::host_seam::emit_stdout(format!("{s}\n").as_bytes()); + let s = unsafe { crate::display::py_repr_wtf8(val)? }; + crate::host_seam::emit_stdout(crate::display::wtf8_format!(s, "\n").as_bytes()); } Ok(()) } @@ -5007,7 +5007,10 @@ mod tests { let (result, _frame) = run_exec_frame("raise int"); let err = result.expect_err("raise int should fail"); assert_eq!(err.kind, PyErrorKind::TypeError); - assert_eq!(err.message, "exceptions must derive from BaseException"); + assert_eq!( + err.message_text(), + "exceptions must derive from BaseException" + ); } #[test] @@ -5046,7 +5049,7 @@ mod tests { let err = result.expect_err("invalid cause should fail"); assert_eq!(err.kind, PyErrorKind::TypeError); assert_eq!( - err.message, + err.message_text(), "exception causes must derive from BaseException" ); } @@ -5238,7 +5241,7 @@ r = acc", .execute_frame(None, None) .expect_err("expected bytecode corruption"); assert_eq!(err.kind, PyErrorKind::BytecodeCorruption); - assert_eq!(err.message, "bytecode corruption"); + assert_eq!(err.message_text(), "bytecode corruption"); } #[test] @@ -6814,7 +6817,7 @@ except (ValueError, 42): matches!(e.kind, crate::PyErrorKind::TypeError), "expected TypeError, got {:?}: {}", e.kind, - e.message, + e.message_text(), ), Ok(_) => panic!("expected TypeError for `except (ValueError, 42):`"), } diff --git a/pyre/pyre-interpreter/src/executioncontext.rs b/pyre/pyre-interpreter/src/executioncontext.rs index 18b931bb6ba..47245ee1c46 100644 --- a/pyre/pyre-interpreter/src/executioncontext.rs +++ b/pyre/pyre-interpreter/src/executioncontext.rs @@ -2330,7 +2330,12 @@ impl UserDelAction { return; } if let Err(error) = crate::baseobjspace::generator_finalize(current()) { - report_error(self.base.space, &error, "", current()); + report_error( + self.base.space, + &error, + rustpython_wtf8::Wtf8::new(""), + current(), + ); } return; } @@ -2356,7 +2361,12 @@ impl UserDelAction { if let Err(error) = unsafe { crate::baseobjspace::get_and_call_function(del(), current(), w_type.as_ptr(), &[]) } { - report_error(self.base.space, &error, "", del()); + report_error( + self.base.space, + &error, + rustpython_wtf8::Wtf8::new(""), + del(), + ); } } } @@ -2386,7 +2396,7 @@ impl AsyncActionOps for UserDelAction { pub fn report_error( space: PyObjectRef, error: &crate::PyError, - where_desc: &str, + where_desc: &rustpython_wtf8::Wtf8, w_obj: PyObjectRef, ) { let mut error = error.clone(); diff --git a/pyre/pyre-interpreter/src/function.rs b/pyre/pyre-interpreter/src/function.rs index 6d08ea7fb62..98a5c5ea358 100644 --- a/pyre/pyre-interpreter/src/function.rs +++ b/pyre/pyre-interpreter/src/function.rs @@ -7,6 +7,7 @@ //! the globals pointer (no clone). use pyre_object::pyobject::*; +use rustpython_wtf8::Wtf8Buf; /// Type descriptor for user-defined functions. pub static FUNCTION_TYPE: PyType = pyre_object::pyobject::new_pytype("function"); @@ -1005,7 +1006,7 @@ pub unsafe fn descr_builtin_function_reduce(obj: PyObjectRef) -> crate::PyResult pyre_object::w_tuple_new(vec![w_self, name]), ])); } - Ok(pyre_object::w_str_new(&unsafe { + Ok(pyre_object::w_str_from_wtf8(unsafe { function_get_qualname(obj) })) } @@ -1243,12 +1244,13 @@ pub unsafe fn fget_func_qualname(obj: PyObjectRef) -> PyObjectRef { /// /// Attribute access itself uses [`fget_func_qualname`] so it returns the /// function-owned Python object rather than a re-wrapped copy. -pub unsafe fn function_get_qualname(obj: PyObjectRef) -> String { - unsafe { - pyre_object::w_str_get_wtf8(fget_func_qualname(obj)) - .to_string_lossy() - .into_owned() - } +/// `function.py:479` reads the name with `space.realutf8_w`, the +/// surrogate-preserving spelling, and `:283` interpolates it into the repr +/// verbatim; `argument.py` builds its TypeErrors from the same string. So the +/// text a lone surrogate is set into has to come back out of here intact -- +/// every consumer either mints a `str` from it or puts it in `e.args[0]`. +pub unsafe fn function_get_qualname(obj: PyObjectRef) -> Wtf8Buf { + unsafe { pyre_object::w_str_get_wtf8(fget_func_qualname(obj)).to_wtf8_buf() } } /// Return the application-level `__qualname__` object. @@ -2812,9 +2814,11 @@ pub unsafe fn descr_method_repr(obj: PyObjectRef) -> Result" + let instance_repr = unsafe { crate::display::py_repr_wtf8(instance)? }; + Ok(pyre_object::w_str_from_wtf8(crate::display::wtf8_format!( + format!("" ))) } diff --git a/pyre/pyre-interpreter/src/gateway.rs b/pyre/pyre-interpreter/src/gateway.rs index a3709b3a787..8114ab53337 100644 --- a/pyre/pyre-interpreter/src/gateway.rs +++ b/pyre/pyre-interpreter/src/gateway.rs @@ -1603,6 +1603,52 @@ pub fn fsdecode_os_str_wtf8(name: &std::ffi::OsStr) -> rustpython_wtf8::Wtf8Buf } } +/// The filesystem bytes behind a host `OsStr` — [`fsdecode_os_str_wtf8`]'s +/// inverse, for a caller that has to retain the name as bytes rather than as +/// text: `co_filename`'s stored spelling, which `pycode.py:431 +/// filename='fsencode'` names in the same units the syscall took. +/// +/// Total, unlike [`fsencode`]: the host handed us this name, so it is already +/// in the filesystem's own units and there is nothing left to reject. +pub fn fsencode_os_str(name: &std::ffi::OsStr) -> Vec { + #[cfg(windows)] + { + use std::os::windows::ffi::OsStrExt; + let units: Vec = name.encode_wide().collect(); + // `FS_ERRORS` is `surrogatepass` here, so a string's own WTF-8 + // spelling is its filesystem encoding. + rustpython_wtf8::Wtf8Buf::from_wide(&units) + .as_bytes() + .to_vec() + } + #[cfg(not(windows))] + { + name.as_encoded_bytes().to_vec() + } +} + +/// [`fsencode_os_str`]'s other direction: the host name filesystem bytes +/// spell, for a caller that has to hand them to an API taking an `OsStr`. +pub fn os_string_from_fs_bytes(data: &[u8]) -> std::ffi::OsString { + #[cfg(windows)] + { + use std::os::windows::ffi::OsStringExt; + let units: Vec = fsdecode_filename_wtf8(data).encode_wide().collect(); + std::ffi::OsString::from_wide(&units) + } + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + std::ffi::OsString::from_vec(data.to_vec()) + } + #[cfg(not(any(unix, windows)))] + { + // No byte spelling on this platform, so the name can only be carried + // as the best text representation of these bytes. + std::ffi::OsString::from(String::from_utf8_lossy(data).into_owned()) + } +} + /// `interp_posix.py:140-152 Path`: the syscall spelling and the resolved path /// object travel together. For `os.PathLike`, `w_path` is the result of the /// single `__fspath__` call, not the wrapper that supplied it. diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 43d64c4dacc..42f208dba74 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -413,7 +413,7 @@ static SYS_PATH: LazyLock>> = LazyLock::new(|| Mutex::new(Vec /// `sys.path` mutations. PyPy records this on interpreter/import state, not /// on an OS thread: import shadowing decisions made by free-threaded workers /// must observe the startup path captured by the launcher. -static SYS_PATH_0: LazyLock>> = LazyLock::new(|| Mutex::new(None)); +static SYS_PATH_0: LazyLock>> = LazyLock::new(|| Mutex::new(None)); /// The literal `sys.path[0]` entry staged by `init_sys_path` and prepended /// by `add_sys_path_0` once `site` has run (`pymain_sys_path_add_path0`): /// `""` for `-c` / stdin / the REPL, the cwd for `-m`, the script's @@ -421,7 +421,8 @@ static SYS_PATH_0: LazyLock>> = LazyLock::new(|| Mutex::new /// the `-i` REPL-after-script path does not prepend it twice. Process-global /// for the same reason as `SYS_PATH_0`: the launcher stages it and whichever /// thread runs the insert must observe that staging. -static SYS_PATH_0_PENDING: LazyLock>> = LazyLock::new(|| Mutex::new(None)); +static SYS_PATH_0_PENDING: LazyLock>> = + LazyLock::new(|| Mutex::new(None)); pub(crate) static BUILTIN_MODULES: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); @@ -1153,7 +1154,7 @@ fn set_builtin_module_spec(_name: &str, _module: PyObjectRef) -> Result<(), crat #[cfg(feature = "host_env")] fn fix_up_source_module_spec( ns: PyObjectRef, - pathname: &str, + pathname: &rustpython_wtf8::Wtf8, cpathname: Option<&str>, ) -> Result { use pyre_object::gc_roots::{pin_root, push_roots, shadow_stack_get, shadow_stack_len}; @@ -1172,7 +1173,7 @@ fn fix_up_source_module_spec( pin_root(w_name); let ext_slot = shadow_stack_len(); pin_root(ext); - let w_path = pyre_object::w_str_new(pathname); + let w_path = pyre_object::w_str_from_wtf8(pathname.to_wtf8_buf()); let path_slot = shadow_stack_len(); pin_root(w_path); let w_cpath = match cpathname { @@ -1211,7 +1212,7 @@ fn fix_up_source_module_spec( #[cfg(not(feature = "host_env"))] fn fix_up_source_module_spec( _ns: PyObjectRef, - _pathname: &str, + _pathname: &rustpython_wtf8::Wtf8, _cpathname: Option<&str>, ) -> Result { Ok(false) @@ -1260,23 +1261,20 @@ fn startup_builtin_module_impl( /// shadowing check to compare against absolute module origins. Under the /// sandbox the path is left as given (a virtual path the controller mediates); /// `canonicalize` would issue raw host syscalls past the seccomp lockdown. -fn canonical_startup_dir(dir: &Path) -> String { +fn canonical_startup_dir(dir: &Path) -> Wtf8Buf { #[cfg(not(feature = "sandbox"))] if let Ok(abs) = std::path::absolute(dir) { - return abs - .canonicalize() - .unwrap_or(abs) - .to_string_lossy() - .into_owned(); + let abs = abs.canonicalize().unwrap_or(abs); + return crate::gateway::fsdecode_os_str_wtf8(abs.as_os_str()); } - dir.to_string_lossy().into_owned() + crate::gateway::fsdecode_os_str_wtf8(dir.as_os_str()) } /// `script_dir` is the shadowing-check anchor (`config->sys_path_0`'s /// directory); `path0` is the literal entry `add_sys_path_0` later prepends to /// `sys.path` — `""` for `-c` / stdin / the REPL, the cwd for `-m`, the /// script's directory for a script. -pub fn init_sys_path(script_dir: &Path, path0: &str) { +pub fn init_sys_path(script_dir: &Path, path0: &std::ffi::OsStr) { // Register builtin modules (PyPy: make_builtins / setup_builtin_modules) install_builtin_modules(); @@ -1290,7 +1288,7 @@ pub fn init_sys_path(script_dir: &Path, path0: &str) { // `site.removeduppaths()` never absolutizes the `-c` / REPL empty entry into // the cwd. Stage the entry here; `add_sys_path_0` performs the insert. // `-P` (safe_path) suppresses it entirely. - *SYS_PATH_0_PENDING.lock().unwrap() = (!safe_path_flag()).then(|| path0.to_string()); + *SYS_PATH_0_PENDING.lock().unwrap() = (!safe_path_flag()).then(|| path0.to_os_string()); { let mut path = SYS_PATH.lock().unwrap(); @@ -1407,7 +1405,7 @@ pub(crate) fn detect_stdlib_path() -> Option { pub fn add_sys_path(dir: &Path) { use pyre_object::gc_roots::{pin_root, push_roots, shadow_stack_get, shadow_stack_len}; - let entry = dir.to_string_lossy(); + let entry = crate::gateway::fsdecode_os_str_wtf8(dir.as_os_str()); if get_sys_module("sys").is_none() { { let mut path = SYS_PATH.lock().unwrap(); @@ -1423,7 +1421,7 @@ pub fn add_sys_path(dir: &Path) { // path is allocation-free until the pinned entry reaches `w_list_append`. let _roots = push_roots(); let slot = shadow_stack_len(); - pin_root(pyre_object::w_str_new(entry.as_ref())); + pin_root(pyre_object::w_str_from_wtf8(entry.clone())); let Some(sys_mod) = get_sys_module("sys") else { return; }; @@ -1440,11 +1438,10 @@ pub fn add_sys_path(dir: &Path) { let n = unsafe { pyre_object::listobject::w_list_len(w_path) }; for i in 0..n { if let Some(item) = unsafe { pyre_object::listobject::w_list_getitem(w_path, i as i64) } { - // A `sys.path` entry read back from the filesystem can hold a - // surrogate escape and so have no `&str` spelling; it simply is - // not equal to this ASCII startup entry. + // Both spellings can hold a surrogate escape, so the two are + // compared as the WTF-8 they are rather than through `&str`. if unsafe { pyre_object::is_str(item) } - && unsafe { pyre_object::w_str_get_value_opt(item) } == Some(entry.as_ref()) + && unsafe { pyre_object::w_str_get_wtf8(item) }.as_bytes() == entry.as_bytes() { return; } @@ -1467,14 +1464,14 @@ pub fn add_sys_path_0() { // No `sys` yet (an embedder that never imports `site`): stage at the front // of the seed instead, which `create_sys_path_list` flushes in order. if get_sys_module("sys").is_none() { - SYS_PATH.lock().unwrap().insert(0, PathBuf::from(&entry)); + SYS_PATH.lock().unwrap().insert(0, PathBuf::from(entry)); return; } // Pin the new entry before any further allocation (`get_sys_module` and the // dict lookup allocate) can relocate it — `add_sys_path` parity. let _roots = push_roots(); let slot = shadow_stack_len(); - pin_root(pyre_object::w_str_new(&entry)); + pin_root(crate::gateway::fsdecode_os_str(&entry)); let Some(sys_mod) = get_sys_module("sys") else { return; }; @@ -2169,7 +2166,7 @@ pub(crate) fn create_sys_path_list() -> PyObjectRef { .lock() .unwrap() .iter() - .map(|d| pyre_object::w_str_new(&d.to_string_lossy())) + .map(|d| crate::gateway::fsdecode_os_str(d.as_os_str())) .collect(); pyre_object::w_list_new(items) } @@ -2242,7 +2239,7 @@ fn exec_code_module( w_code: PyObjectRef, w_globals: pyre_object::PyObjectRef, execution_context: *const PyExecutionContext, - pathname: Option<&str>, + pathname: Option<&rustpython_wtf8::Wtf8>, cpathname: Option<&str>, ) -> Result { // importing.py:272-274 — setdefault('__builtins__', space.builtin). @@ -2265,7 +2262,7 @@ fn exec_code_module( // `write_paths=False` shape (REPL, builtin bootstrap). if let Some(p) = pathname { // importing.py:284 setitem('__file__', w_pathname). - let w_pathname = pyre_object::w_str_new(p); + let w_pathname = pyre_object::w_str_from_wtf8(p.to_wtf8_buf()); unsafe { pyre_object::w_dict_setitem_str(w_globals, "__file__", w_pathname); } @@ -2404,17 +2401,25 @@ fn load_source_module( package_dir: Option<&Path>, execution_context: *const PyExecutionContext, ) -> Result { + // The name reaches the module as `__file__` and the code object as + // `co_filename`, so it is carried in the two spellings `pycode.py:431 + // filename='fsencode'` keeps apart: the filesystem bytes it was named + // with, and the UTF-8 spelling the compiler's own `source_path` is limited + // to. `path_text` is the first, decoded, and is what application level + // sees. + let path_bytes = crate::gateway::fsencode_os_str(pathname.as_os_str()); + let path_text = crate::gateway::fsdecode_filename_wtf8(&path_bytes); let bytes = with_source_provider(|p| p.read_to_bytes(pathname)).map_err(|e| { - crate::PyError::new( - crate::PyErrorKind::ImportError, - format!("cannot read '{}': {e}", pathname.display()), - ) + let mut message = rustpython_wtf8::Wtf8Buf::from_string("cannot read '".to_string()); + message.push_wtf8(&path_text); + message.push_str(&format!("': {e}")); + crate::PyError::new(crate::PyErrorKind::ImportError, message) })?; - let pathname_str = pathname.to_string_lossy(); + let (pathname_str, filename_bytes) = crate::pycode::split_code_filename_bytes(path_bytes, None); // A source file carries its own encoding in a BOM or a PEP 263 cookie; a // bad declaration is the tokenizer's SyntaxError, not an ImportError. - let source = crate::compile::decode_source_bytes(&bytes, &pathname_str, false)?; + let source = crate::compile::decode_source_bytes(&bytes, &path_text, false)?; let _root = pyre_object::gc_roots::push_roots(); // The two importlib bootstrap sources are imported by the native importer @@ -2432,10 +2437,11 @@ fn load_source_module( Some(w_code) => (w_code, false), None => { let code = parse_source_module(&pathname_str, &source).map_err(|e| { - crate::PyError::new( - crate::PyErrorKind::ImportError, - format!("cannot compile '{}': {e}", pathname.display()), - ) + let mut message = + rustpython_wtf8::Wtf8Buf::from_string("cannot compile '".to_string()); + message.push_wtf8(&path_text); + message.push_str(&format!("': {e}")); + crate::PyError::new(crate::PyErrorKind::ImportError, message) })?; ( crate::w_code_new(Box::into_raw(Box::new(code)) as *const ()), @@ -2443,6 +2449,9 @@ fn load_source_module( ) } }; + // The whole unit was named by this path, so the nested constants still + // held unrealized take the same spelling when they are boxed. + unsafe { crate::pycode::set_compilation_unit_filename_bytes(w_code, filename_bytes) }; // Root before any allocation (fresh_module_globals, the cache write) can // collect the freshly boxed code out from under us. pyre_object::gc_roots::pin_root(w_code); @@ -2489,7 +2498,7 @@ fn load_source_module( // `exec_module`; setting it afterwards lets those imports fall through to // sys.path and pick up a same-leaf module from an unrelated package. if let Some(dir) = package_dir { - let path_str = pyre_object::w_str_new(&dir.to_string_lossy()); + let path_str = crate::gateway::fsdecode_os_str(dir.as_os_str()); unsafe { pyre_object::w_dict_setitem_str( w_globals, @@ -2531,13 +2540,7 @@ fn load_source_module( // On exec failure drop the pre-registered module from sys.modules // (`_bootstrap._load`) so a retried import re-runs the body instead of // observing a half-built module. - if let Err(e) = exec_code_module( - w_code, - w_globals, - execution_context, - Some(&pathname_str), - None, - ) { + if let Err(e) = exec_code_module(w_code, w_globals, execution_context, Some(&path_text), None) { remove_sys_module(modulename); return Err(e); } @@ -2800,7 +2803,7 @@ fn load_namespace_package( let path_items: Vec = dirs .iter() - .map(|d| pyre_object::w_str_new(&d.to_string_lossy())) + .map(|d| crate::gateway::fsdecode_os_str(d.as_os_str())) .collect(); unsafe { pyre_object::w_dict_setitem_str(w_globals, "__path__", pyre_object::w_list_new(path_items)); @@ -3573,9 +3576,10 @@ fn relative_import_head( if let Some(w_found) = found { return Ok(w_found); } - let head_repr = unsafe { crate::display::py_repr(shadow_stack_get(head_slot)) }?; - Err(crate::PyError::key_error(format!( - "{head_repr} not in sys.modules as expected" + let head_repr = unsafe { crate::display::py_repr_wtf8(shadow_stack_get(head_slot)) }?; + Err(crate::PyError::key_error(crate::display::wtf8_format!( + head_repr, + " not in sys.modules as expected" ))) } @@ -3826,29 +3830,32 @@ pub(crate) fn spec_file_origin(w_spec: PyObjectRef) -> Result bool { +pub(crate) fn is_possibly_shadowing(origin: &rustpython_wtf8::Wtf8) -> bool { if safe_path_flag() { return false; } let Some(sys_path_0) = SYS_PATH_0.lock().unwrap().clone() else { return false; }; - let sep = std::path::MAIN_SEPARATOR; + let sep = std::path::MAIN_SEPARATOR as u8; + // The separator and `__init__.py` are ASCII, and an ASCII byte never + // occurs inside a multi-byte WTF-8 sequence, so the scan runs on bytes + // while the name itself may hold a surrogate escape. // root = os.path.dirname(origin.removesuffix(os.sep + "__init__.py")) - let mut root = origin.to_string(); - let Some(idx) = root.rfind(sep) else { + let mut root = origin.as_bytes(); + let Some(idx) = root.iter().rposition(|&b| b == sep) else { return false; }; - if root[idx + 1..] == *"__init__.py" { - root.truncate(idx); - let Some(idx2) = root.rfind(sep) else { + if &root[idx + 1..] == b"__init__.py" { + root = &root[..idx]; + let Some(idx2) = root.iter().rposition(|&b| b == sep) else { return false; }; - root.truncate(idx2); + root = &root[..idx2]; } else { - root.truncate(idx); + root = &root[..idx]; } - root == sys_path_0 + root == sys_path_0.as_bytes() } /// The shadowing classification for a module: its spec file origin (a path @@ -3859,9 +3866,11 @@ pub(crate) fn is_possibly_shadowing(origin: &str) -> bool { pub(crate) fn module_shadow_info( w_spec: PyObjectRef, w_name: PyObjectRef, -) -> Result<(Option, bool, bool), crate::PyError> { +) -> Result<(Option, bool, bool), crate::PyError> { let origin = match spec_file_origin(w_spec)? { - Some(o) => unsafe { pyre_object::w_str_get_value(o) }.to_string(), + // A `spec.origin` is a filename, so it can hold the surrogate escape + // an undecodable path byte decodes to and has no `&str` spelling. + Some(o) => unsafe { pyre_object::w_str_get_wtf8(o) }.to_wtf8_buf(), None => return Ok((None, false, false)), }; if !is_possibly_shadowing(&origin) { @@ -3946,7 +3955,9 @@ pub fn import_from( // rather than be masked (`_handle_fromlist`). let absent_submodule = e.kind == crate::PyErrorKind::ModuleNotFoundError - && e.message.contains(&format!("'{fullname}'")); + && e.message.contains(&rustpython_wtf8::Wtf8Buf::from_string( + format!("'{fullname}'"), + )); if !absent_submodule { return Err(e); } @@ -4021,34 +4032,43 @@ pub fn import_from( let uninit_submodule = !initializing && is_spec_uninitialized_submodule(w_spec, &pkgname)?; let pkgpath = crate::baseobjspace::utf8_w(pyre_object::gc_roots::shadow_stack_get(pkgpath_slot))?; - let origin = origin.as_deref().unwrap_or(""); - let msg = if is_shadowing_stdlib { - format!( - "cannot import name '{name}' from '{pkgname}' (consider renaming \ - '{origin}' since it has the same name as the standard library \ - module named '{pkgname}' and prevents importing that standard \ - library module)" - ) + let origin = origin.unwrap_or_default(); + // The origin is a filename and may hold a surrogate escape, so the two + // messages that name it are assembled as WTF-8; `format!` would render it + // through `Display` and substitute U+FFFD. + let renaming_hint = |tail: String| { + let mut msg = Wtf8Buf::from_string(format!( + "cannot import name '{name}' from '{pkgname}' (consider renaming '" + )); + msg.push_wtf8(&origin); + msg.push_str(&tail); + msg + }; + let msg: Wtf8Buf = if is_shadowing_stdlib { + renaming_hint(format!( + "' since it has the same name as the standard library module named \ + '{pkgname}' and prevents importing that standard library module)" + )) } else if initializing { if is_shadowing { - format!( - "cannot import name '{name}' from '{pkgname}' (consider renaming \ - '{origin}' if it has the same name as a library you intended \ - to import)" + renaming_hint( + "' if it has the same name as a library you intended to import)".to_string(), ) } else { format!( "cannot import name '{name}' from partially initialized module \ '{pkgname}' (most likely due to a circular import) ({pkgpath})" ) + .into() } } else if uninit_submodule { format!( "cannot access submodule '{pkgname}' of module '{name}' \ (most likely due to a circular import)" ) + .into() } else { - format!("cannot import name '{name}' from '{pkgname}' ({pkgpath})") + format!("cannot import name '{name}' from '{pkgname}' ({pkgpath})").into() }; let w_pkgname = pyre_object::gc_roots::shadow_stack_get(pkgname_slot); let w_pkgpath = pyre_object::gc_roots::shadow_stack_get(pkgpath_slot); @@ -4205,11 +4225,11 @@ mod tests { crate::test_hooks::install_hash_hook(); let empty = importhook("", PY_NULL, PY_NULL, 0, std::ptr::null()).unwrap_err(); assert_eq!(empty.kind, crate::PyErrorKind::ValueError); - assert_eq!(empty.message, "Empty module name"); + assert_eq!(empty.message_text(), "Empty module name"); let negative = importhook("sys", PY_NULL, PY_NULL, -1, std::ptr::null()).unwrap_err(); assert_eq!(negative.kind, crate::PyErrorKind::ValueError); - assert_eq!(negative.message, "level must be >= 0"); + assert_eq!(negative.message_text(), "level must be >= 0"); let globals = pyre_object::w_dict_new(); unsafe { @@ -4218,7 +4238,7 @@ mod tests { let no_parent = importhook("", globals, PY_NULL, 1, std::ptr::null()).unwrap_err(); assert_eq!(no_parent.kind, crate::PyErrorKind::ImportError); assert_eq!( - no_parent.message, + no_parent.message_text(), "attempted relative import with no known parent package" ); } diff --git a/pyre/pyre-interpreter/src/launch_env.rs b/pyre/pyre-interpreter/src/launch_env.rs index 332c25377be..490581950c3 100644 --- a/pyre/pyre-interpreter/src/launch_env.rs +++ b/pyre/pyre-interpreter/src/launch_env.rs @@ -131,18 +131,11 @@ fn read(name: &str) -> Option { } /// Environment bytes in the host's own spelling. The seam hands back what the -/// platform stores: bytes on unix, where any byte is legal, and the UTF-8 form -/// of the wide value on Windows, where the host has already validated it. +/// platform stores: bytes on unix, where any byte is legal, and the WTF-8 form +/// of the wide value on Windows, whose unpaired surrogates re-encode to the +/// code units they came from. fn os_string_from_bytes(value: &[u8]) -> std::ffi::OsString { - #[cfg(unix)] - { - use std::os::unix::ffi::OsStringExt; - std::ffi::OsString::from_vec(value.to_vec()) - } - #[cfg(not(unix))] - { - std::ffi::OsString::from(String::from_utf8_lossy(value).into_owned()) - } + crate::gateway::os_string_from_fs_bytes(value) } /// Presence of a variable, without decoding it. `_Py_GetEnv` tests the raw diff --git a/pyre/pyre-interpreter/src/lib.rs b/pyre/pyre-interpreter/src/lib.rs index aa2e99ce7b4..5ddc0e70044 100644 --- a/pyre/pyre-interpreter/src/lib.rs +++ b/pyre/pyre-interpreter/src/lib.rs @@ -50,12 +50,13 @@ pub mod host_seam; // every target. #[cfg(not(unix))] pub mod host_seam { - /// Read a process environment value. Numeric `PYTHONHASHSEED` values are - /// ASCII; lossily encoding any other platform string still makes the seed - /// parser reject it instead of treating it as absent. + /// Read a process environment value, in the same filesystem-bytes spelling + /// the unix seam hands back. A Windows value holding an unpaired surrogate + /// keeps it as its WTF-8 encoding, so the caller that turns the bytes back + /// into a platform string recovers the code units the host stored. pub fn getenv(name: &[u8]) -> Result>, ()> { let name = std::str::from_utf8(name).map_err(|_| ())?; - Ok(std::env::var_os(name).map(|value| value.to_string_lossy().into_owned().into_bytes())) + Ok(std::env::var_os(name).map(|value| crate::gateway::fsencode_os_str(&value))) } /// Emit bytes to the interpreter's stdout (fd 1). diff --git a/pyre/pyre-interpreter/src/module/__pypy__/mod.rs b/pyre/pyre-interpreter/src/module/__pypy__/mod.rs index 66d82cc9499..63245968343 100644 --- a/pyre/pyre-interpreter/src/module/__pypy__/mod.rs +++ b/pyre/pyre-interpreter/src/module/__pypy__/mod.rs @@ -159,7 +159,7 @@ const CANONICAL_IDENTITY_DICT_KEY: &str = "@objects_in_repr_identity_dict"; /// `interp_magic.py:280-290 write_unraisable` — turn the supplied exception /// value back into an OperationError and report it through `sys.unraisablehook`. fn write_unraisable(args: &[pyre_object::PyObjectRef]) -> crate::PyResult { - let where_desc = crate::baseobjspace::str_utf8_w(args[0])?; + let where_desc = unsafe { pyre_object::w_str_get_wtf8(args[0]) }.to_wtf8_buf(); // `OperationError(space.type(w_exc), w_exc)` accepts any object, so the // exception tag cannot be read unconditionally: it lives past the header // of a `W_BaseException`, and a plain instance is smaller than that. diff --git a/pyre/pyre-interpreter/src/module/_ast/convert.rs b/pyre/pyre-interpreter/src/module/_ast/convert.rs index 354033f8d76..8972de7cec2 100644 --- a/pyre/pyre-interpreter/src/module/_ast/convert.rs +++ b/pyre/pyre-interpreter/src/module/_ast/convert.rs @@ -159,16 +159,16 @@ impl ObjectConverter { } /// How `%R` names the value an error rejected. - fn repr(&self, object: PyObjectRef) -> AstResult { - unsafe { crate::display::py_repr(object) } + fn repr(&self, object: PyObjectRef) -> AstResult { + unsafe { crate::display::py_repr_wtf8(object) } } /// `obj_to_int` (ast.py:36) — an integer field takes an `int`, or an /// instance of a subclass of one. Nothing else is asked for `__index__`. fn obj_to_int(&self, value: PyObjectRef) -> AstResult { if !unsafe { crate::baseobjspace::isinstance_int_w(value) } { - return Err(crate::PyError::value_error(format!( - "invalid integer value: {}", + return Err(crate::PyError::value_error(crate::display::wtf8_format!( + "invalid integer value: ", self.repr(value)? ))); } @@ -212,8 +212,8 @@ impl ObjectConverter { if unsafe { pyre_object::is_none(item) } || self.is_node(item, "TypeIgnore")? { continue; } - return Err(crate::PyError::type_error(format!( - "expected some sort of type_ignore, but got {}", + return Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of type_ignore, but got ", self.repr(item)? ))); } @@ -234,8 +234,8 @@ impl ObjectConverter { body: Box::new(self.recurse(|this| this.expr(body))?), })); } else { - return Err(crate::PyError::type_error(format!( - "expected some sort of mod, but got {}", + return Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of mod, but got ", self.repr(object)? ))); }; @@ -608,8 +608,8 @@ impl ObjectConverter { cases, })) } else { - Err(crate::PyError::type_error(format!( - "expected some sort of stmt, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of stmt, but got ", self.repr(object)? ))) } @@ -728,8 +728,8 @@ impl ObjectConverter { fn handler(&mut self, object: PyObjectRef) -> AstResult { self.location(object, "excepthandler")?; if !self.is_node(object, "ExceptHandler")? { - return Err(crate::PyError::type_error(format!( - "expected some sort of excepthandler, but got {}", + return Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of excepthandler, but got ", self.repr(object)? ))); } @@ -862,8 +862,8 @@ impl ObjectConverter { default: self.opt_expr(object, "default_value")?, })) } else { - Err(crate::PyError::type_error(format!( - "expected some sort of type_param, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of type_param, but got ", self.repr(object)? ))) } @@ -1002,8 +1002,8 @@ impl ObjectConverter { runtime_patterns: None, })) } else { - Err(crate::PyError::type_error(format!( - "expected some sort of pattern, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of pattern, but got ", self.repr(object)? ))) } @@ -1411,8 +1411,8 @@ impl ObjectConverter { let element = self.interpolation(object)?; Ok(fstring(vec![element], None)) } else { - Err(crate::PyError::type_error(format!( - "expected some sort of expr, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of expr, but got ", self.repr(object)? ))) } @@ -1463,8 +1463,8 @@ impl ObjectConverter { return Ok(op); } } - Err(crate::PyError::type_error(format!( - "expected some sort of boolop, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of boolop, but got ", self.repr(object)? ))) } @@ -1486,8 +1486,8 @@ impl ObjectConverter { return Ok(op); } } - Err(crate::PyError::type_error(format!( - "expected some sort of cmpop, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of cmpop, but got ", self.repr(object)? ))) } @@ -1619,8 +1619,8 @@ impl ObjectConverter { return Ok(ctx); } } - Err(crate::PyError::type_error(format!( - "expected some sort of expr_context, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of expr_context, but got ", self.repr(object)? ))) } @@ -1636,8 +1636,8 @@ impl ObjectConverter { return Ok(op); } } - Err(crate::PyError::type_error(format!( - "expected some sort of unaryop, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of unaryop, but got ", self.repr(object)? ))) } @@ -1662,8 +1662,8 @@ impl ObjectConverter { return Ok(op); } } - Err(crate::PyError::type_error(format!( - "expected some sort of operator, but got {}", + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "expected some sort of operator, but got ", self.repr(object)? ))) } diff --git a/pyre/pyre-interpreter/src/module/_collections/mod.rs b/pyre/pyre-interpreter/src/module/_collections/mod.rs index 122d8671957..a2382dbfc1c 100644 --- a/pyre/pyre-interpreter/src/module/_collections/mod.rs +++ b/pyre/pyre-interpreter/src/module/_collections/mod.rs @@ -1085,9 +1085,9 @@ impl W_Deque { } i += 1; } - Err(crate::PyError::value_error(format!( - "{} is not in deque", - unsafe { crate::py_repr(x)? } + Err(crate::PyError::value_error(crate::display::wtf8_format!( + unsafe { crate::display::py_repr_wtf8(x)? }, + " is not in deque" ))) } fn copy(&self) -> Result { diff --git a/pyre/pyre-interpreter/src/module/_contextvars/mod.rs b/pyre/pyre-interpreter/src/module/_contextvars/mod.rs index da2dbd4a76e..1b148654e70 100644 --- a/pyre/pyre-interpreter/src/module/_contextvars/mod.rs +++ b/pyre/pyre-interpreter/src/module/_contextvars/mod.rs @@ -198,9 +198,9 @@ fn context_var_reset(args: &[PyObjectRef]) -> Result Result Result { +fn context_var_repr_string(obj: PyObjectRef) -> Result { let Some(_guard) = crate::display::ReprGuard::enter(obj) else { - return Ok("...".to_string()); + return Ok(rustpython_wtf8::Wtf8Buf::from_string("...".to_string())); }; let name = crate::baseobjspace::getattr_str(obj, "_name")?; - let name_repr = unsafe { crate::display::py_repr_wtf8(name)? } - .to_string_lossy() - .into_owned(); + let name_repr = unsafe { crate::display::py_repr_wtf8(name)? }; let default = match crate::baseobjspace::findattr_result(obj, "_default")? { - Some(value) => { - let value_repr = unsafe { crate::display::py_repr_wtf8(value)? } - .to_string_lossy() - .into_owned(); - format!(" default={value_repr}") - } - None => String::new(), + Some(value) => crate::display::wtf8_format!(" default=", unsafe { + crate::display::py_repr_wtf8(value)? + }), + None => rustpython_wtf8::Wtf8Buf::new(), }; - Ok(format!( - "", - obj as usize + Ok(crate::display::wtf8_format!( + "", obj as usize), )) } fn context_var_repr(args: &[PyObjectRef]) -> Result { - Ok(w_str_new(&context_var_repr_string(args[0])?)) + Ok(pyre_object::w_str_from_wtf8(context_var_repr_string( + args[0], + )?)) } fn token_type() -> PyObjectRef { @@ -369,22 +368,23 @@ fn token_old_value_get(args: &[PyObjectRef]) -> Result Result { +fn token_repr_string(token: PyObjectRef) -> Result { let var = crate::baseobjspace::getattr_str(token, "_var")?; - let var_repr = unsafe { crate::display::py_repr_wtf8(var)? } - .to_string_lossy() - .into_owned(); + let var_repr = unsafe { crate::display::py_repr_wtf8(var)? }; let used = crate::baseobjspace::is_true(crate::baseobjspace::getattr_str(token, "_used")?)?; - Ok(format!( - "", - if used { "used " } else { "" }, + Ok(crate::display::wtf8_format!( + if used { + "", token as usize), )) } fn token_repr(args: &[PyObjectRef]) -> Result { - Ok(w_str_new(&token_repr_string(args[0])?)) + Ok(pyre_object::w_str_from_wtf8(token_repr_string(args[0])?)) } fn token_enter(args: &[PyObjectRef]) -> Result { diff --git a/pyre/pyre-interpreter/src/module/_csv/mod.rs b/pyre/pyre-interpreter/src/module/_csv/mod.rs index b3bc870e766..cfbf6c3f158 100644 --- a/pyre/pyre-interpreter/src/module/_csv/mod.rs +++ b/pyre/pyre-interpreter/src/module/_csv/mod.rs @@ -58,10 +58,11 @@ struct DialectConfig { /// Build a `PyError` whose raised object is an instance of `_csv.Error` /// (registered by the `exceptions:` block), with `msg` as the single /// argument — `interp_csv.py W_Reader.error` / `W_Writer.error`. -fn csv_error(msg: String) -> PyError { +fn csv_error(msg: impl Into) -> PyError { + let msg = msg.into(); let mut err = PyError::runtime_error(msg.clone()); if let Some(cls) = crate::builtins::lookup_exc_class("_csv.Error") { - let args = [cls, pyre_object::w_str_new(&msg)]; + let args = [cls, pyre_object::w_str_from_wtf8(msg)]; if let Ok(exc) = crate::builtins::exc_exception_new(&args) { err.exc_object = exc; } @@ -892,8 +893,11 @@ fn writer_writerow_impl( let row = match crate::builtins::collect_iterable(w_fields) { Ok(r) => r, Err(e) if e.kind == crate::PyErrorKind::TypeError => { - let r = unsafe { crate::display::py_repr(w_fields) }.unwrap_or_default(); - return Err(csv_error(format!("iterable expected, not {r}"))); + let r = unsafe { crate::display::py_repr_wtf8(w_fields) }.unwrap_or_default(); + return Err(csv_error(crate::display::wtf8_format!( + "iterable expected, not ", + r + ))); } Err(e) => return Err(e), }; @@ -902,15 +906,15 @@ fn writer_writerow_impl( let quote_char = cfg.quotechar.and_then(char::from_u32).unwrap_or('"'); let delim_char = char::from_u32(cfg.delimiter).unwrap_or(','); let n = row.len(); - let mut rec = String::new(); + let mut rec = rustpython_wtf8::Wtf8Buf::new(); for (i, &w_field) in row.iter().enumerate() { let field = if unsafe { pyre_object::is_none(w_field) } { - String::new() + rustpython_wtf8::Wtf8Buf::new() } else if unsafe { pyre_object::is_float(w_field) } { - unsafe { crate::display::py_repr(w_field) }? + unsafe { crate::display::py_repr_wtf8(w_field) }? } else { - unsafe { crate::display::py_str(w_field) }? + unsafe { crate::display::py_str_wtf8(w_field) }? }; let mut quoted = match cfg.quoting { @@ -918,8 +922,8 @@ fn writer_writerow_impl( QUOTE_ALL => true, QUOTE_MINIMAL => { let mut q = false; - for c in field.chars() { - let cp = c as u32; + for c in field.code_points() { + let cp = c.to_u32(); if !special.contains(&cp) { continue; } @@ -966,14 +970,14 @@ fn writer_writerow_impl( } if i > 0 { - rec.push(delim_char); + rec.push_char(delim_char); } if quoted { - rec.push(quote_char); + rec.push_char(quote_char); } - for c in field.chars() { - let cp = c as u32; + for c in field.code_points() { + let cp = c.to_u32(); if special.contains(&cp) { let want_escape = if cfg.quoting == QUOTE_NONE { true @@ -981,7 +985,7 @@ fn writer_writerow_impl( let mut we = false; if Some(cp) == cfg.quotechar { if cfg.doublequote { - rec.push(quote_char); + rec.push_char(quote_char); } else { we = true; } @@ -993,7 +997,7 @@ fn writer_writerow_impl( }; if want_escape { match cfg.escapechar.and_then(char::from_u32) { - Some(e) => rec.push(e), + Some(e) => rec.push_char(e), None => { return Err(csv_error( "need to escape, but no escapechar set".to_string(), @@ -1006,12 +1010,12 @@ fn writer_writerow_impl( } if quoted { - rec.push(quote_char); + rec.push_char(quote_char); } } rec.push_str(&cfg.lineterminator); - crate::call::call_function_impl_result(w_filewrite, &[pyre_object::w_str_new(&rec)]) + crate::call::call_function_impl_result(w_filewrite, &[pyre_object::w_str_from_wtf8(rec)]) } /// `W_Writer.writerows` — serialize a sequence of records. diff --git a/pyre/pyre-interpreter/src/module/_ctypes/cdata.rs b/pyre/pyre-interpreter/src/module/_ctypes/cdata.rs index 9e5f55f6fce..ec487c44fb6 100644 --- a/pyre/pyre-interpreter/src/module/_ctypes/cdata.rs +++ b/pyre/pyre-interpreter/src/module/_ctypes/cdata.rs @@ -327,9 +327,11 @@ fn simplecdata_repr(args: &[PyObjectRef]) -> Result cdata_bytes(obj).unwrap_or(&[]), )) }; - let rendered = unsafe { crate::display::py_repr(value) }?; + let rendered = unsafe { crate::display::py_repr_wtf8(value) }?; let name = unsafe { pyre_object::typeobject::w_type_get_name(cls) }; - Ok(pyre_object::w_str_new(&format!("{name}({rendered})"))) + Ok(pyre_object::w_str_from_wtf8(crate::display::wtf8_format!( + name, "(", rendered, ")" + ))) } /// `_SimpleCData.from_param(cls, value)` — identity stub (see caller note). diff --git a/pyre/pyre-interpreter/src/module/_ctypes/funcptr.rs b/pyre/pyre-interpreter/src/module/_ctypes/funcptr.rs index d128a6bb224..e3fd4b2368e 100644 --- a/pyre/pyre-interpreter/src/module/_ctypes/funcptr.rs +++ b/pyre/pyre-interpreter/src/module/_ctypes/funcptr.rs @@ -161,14 +161,16 @@ fn resolve_from_tuple(t: PyObjectRef) -> Result { } host_ctypes::lookup_function_symbol_addr(handle, &name_bytes).map_err(|e| { use host_ctypes::LookupSymbolError as L; - let sym = String::from_utf8_lossy(&name_bytes); - match e { - L::LibraryNotFound => crate::PyError::value_error("library not found"), - L::LibraryClosed => { - crate::PyError::attribute_error(format!("function '{sym}' not found")) - } - L::Load(_) => crate::PyError::attribute_error(format!("function '{sym}' not found")), + if matches!(e, L::LibraryNotFound) { + return crate::PyError::value_error("library not found"); } + // A symbol name arrives as bytes, so it is decoded the way a name the + // host handed us is: `format!` would fold a byte with no UTF-8 + // spelling to U+FFFD and report a symbol nobody asked for. + let mut msg = rustpython_wtf8::Wtf8Buf::from_string("function '".to_string()); + msg.push_wtf8(&crate::gateway::fsdecode_filename_wtf8(&name_bytes)); + msg.push_str("' not found"); + crate::PyError::attribute_error(msg) }) } @@ -388,15 +390,18 @@ fn callback_result( Ok(value) => value, Err(mut error) => { let callable = instance_get(obj, CALLABLE_KEY).unwrap_or(pyre_object::PY_NULL); + let unknown = || rustpython_wtf8::Wtf8Buf::from_string("".to_string()); let rendered = if callable.is_null() { - "".to_string() + unknown() } else { - unsafe { crate::display::py_repr(callable) } - .unwrap_or_else(|_| "".to_string()) + unsafe { crate::display::py_repr_wtf8(callable) }.unwrap_or_else(|_| unknown()) }; error.write_unraisable( pyre_object::w_none(), - &format!("Exception ignored while calling ctypes callback function {rendered}"), + &crate::display::wtf8_format!( + "Exception ignored while calling ctypes callback function ", + rendered + ), pyre_object::PY_NULL, ); pyre_object::w_int_new(0) diff --git a/pyre/pyre-interpreter/src/module/_ctypes/interp_ctypes.rs b/pyre/pyre-interpreter/src/module/_ctypes/interp_ctypes.rs index 0569a6ae165..a08bec57b10 100644 --- a/pyre/pyre-interpreter/src/module/_ctypes/interp_ctypes.rs +++ b/pyre/pyre-interpreter/src/module/_ctypes/interp_ctypes.rs @@ -76,11 +76,15 @@ fn register_host_ctypes(ns: pyre_object::PyObjectRef) { let h = rustpython_host_env::ctypes::insert_raw_library_handle(ptr); return Ok(pyre_object::w_int_new(h as i64)); } + // A library name is a path, so it reaches `dlopen` in the + // filesystem's own units: a byte with no UTF-8 spelling names + // a real file and must not be replaced with U+FFFD. if pyre_object::is_bytes(args[0]) { - String::from_utf8_lossy(pyre_object::bytesobject::w_bytes_data(args[0])) - .into_owned() + crate::gateway::os_string_from_fs_bytes( + pyre_object::bytesobject::w_bytes_data(args[0]), + ) } else if pyre_object::is_str(args[0]) { - crate::baseobjspace::str_utf8_w(args[0])?.to_string() + crate::gateway::os_string_from_fs_bytes(&crate::gateway::fsencode(args[0])?) } else { return Err(crate::PyError::type_error( "dlopen: name must be a string, bytes or None", @@ -93,8 +97,12 @@ fn register_host_ctypes(ns: pyre_object::PyObjectRef) { None }; let mode = host_ctypes::dlopen_mode(load_flags); - let h = rustpython_host_env::ctypes::open_library_with_mode(&name, mode) - .map_err(|e| crate::PyError::os_error(format!("dlopen({name}): {e}")))?; + let h = rustpython_host_env::ctypes::open_library_with_mode(&name, mode).map_err(|e| { + let mut msg = rustpython_wtf8::Wtf8Buf::from_string("dlopen(".to_string()); + msg.push_wtf8(&crate::gateway::fsdecode_os_str_wtf8(&name)); + msg.push_str(&format!("): {e}")); + crate::PyError::os_error(msg) + })?; Ok(pyre_object::w_int_new(h as i64)) }), ); @@ -533,8 +541,8 @@ fn carg_type() -> pyre_object::PyObjectRef { let d = crate::baseobjspace::getdict_native(args[0]); let value = unsafe { pyre_object::w_dict_getitem_str(d, "_obj") } .unwrap_or_else(pyre_object::w_none); - let rendered = unsafe { crate::display::py_repr(value) }?; - Ok(pyre_object::w_str_new(&format!(""))) + let rendered = unsafe { crate::display::py_repr_wtf8(value) }?; + Ok(pyre_object::w_str_from_wtf8(crate::display::wtf8_format!(""))) }), ); }); diff --git a/pyre/pyre-interpreter/src/module/_io/buffered.rs b/pyre/pyre-interpreter/src/module/_io/buffered.rs index 0ed0b337f65..c8efe30e2bf 100644 --- a/pyre/pyre-interpreter/src/module/_io/buffered.rs +++ b/pyre/pyre-interpreter/src/module/_io/buffered.rs @@ -859,7 +859,7 @@ impl W_BufferedReader { super::call_method_result(self.w_raw, "_dealloc_warn", &[w_source]) } - fn __repr__(&self) -> Result { + fn __repr__(&self) -> Result { let self_obj = self.self_obj(); let Some(_guard) = crate::display::ReprGuard::enter(self_obj) else { return Err(crate::PyError::runtime_error( @@ -868,10 +868,14 @@ impl W_BufferedReader { }; let typename = crate::type_methods::arg_type_name(self_obj); match crate::baseobjspace::getattr_str(self_obj, "name") { - Ok(name) => Ok(format!("<{typename} name={}>", unsafe { - crate::display::py_repr(name)? - })), - Err(_) => Ok(format!("<{typename}>")), + Ok(name) => Ok(crate::display::wtf8_format!( + format!("<{typename} name="), + unsafe { crate::display::py_repr_wtf8(name)? }, + ">" + )), + Err(_) => Ok(rustpython_wtf8::Wtf8Buf::from_string(format!( + "<{typename}>" + ))), } } } diff --git a/pyre/pyre-interpreter/src/module/_io/buffered_random.rs b/pyre/pyre-interpreter/src/module/_io/buffered_random.rs index e034b08cc63..3bee169e347 100644 --- a/pyre/pyre-interpreter/src/module/_io/buffered_random.rs +++ b/pyre/pyre-interpreter/src/module/_io/buffered_random.rs @@ -1000,7 +1000,7 @@ impl W_BufferedRandom { super::call_method_result(self.w_raw, "_dealloc_warn", &[w_source]) } - fn __repr__(&self) -> Result { + fn __repr__(&self) -> Result { let self_obj = self.self_obj(); let Some(_guard) = crate::display::ReprGuard::enter(self_obj) else { return Err(crate::PyError::runtime_error( @@ -1009,10 +1009,14 @@ impl W_BufferedRandom { }; let typename = crate::type_methods::arg_type_name(self_obj); match crate::baseobjspace::getattr_str(self_obj, "name") { - Ok(name) => Ok(format!("<{typename} name={}>", unsafe { - crate::display::py_repr(name)? - })), - Err(_) => Ok(format!("<{typename}>")), + Ok(name) => Ok(crate::display::wtf8_format!( + format!("<{typename} name="), + unsafe { crate::display::py_repr_wtf8(name)? }, + ">" + )), + Err(_) => Ok(rustpython_wtf8::Wtf8Buf::from_string(format!( + "<{typename}>" + ))), } } } diff --git a/pyre/pyre-interpreter/src/module/_io/buffered_writer.rs b/pyre/pyre-interpreter/src/module/_io/buffered_writer.rs index 1eb706101d1..d4548334566 100644 --- a/pyre/pyre-interpreter/src/module/_io/buffered_writer.rs +++ b/pyre/pyre-interpreter/src/module/_io/buffered_writer.rs @@ -527,7 +527,7 @@ impl W_BufferedWriter { super::call_method_result(self.w_raw, "_dealloc_warn", &[w_source]) } - fn __repr__(&self) -> Result { + fn __repr__(&self) -> Result { let self_obj = self.self_obj(); let Some(_guard) = crate::display::ReprGuard::enter(self_obj) else { return Err(crate::PyError::runtime_error( @@ -536,10 +536,14 @@ impl W_BufferedWriter { }; let typename = crate::type_methods::arg_type_name(self_obj); match crate::baseobjspace::getattr_str(self_obj, "name") { - Ok(name) => Ok(format!("<{typename} name={}>", unsafe { - crate::display::py_repr(name)? - })), - Err(_) => Ok(format!("<{typename}>")), + Ok(name) => Ok(crate::display::wtf8_format!( + format!("<{typename} name="), + unsafe { crate::display::py_repr_wtf8(name)? }, + ">" + )), + Err(_) => Ok(rustpython_wtf8::Wtf8Buf::from_string(format!( + "<{typename}>" + ))), } } } diff --git a/pyre/pyre-interpreter/src/module/_io/stringio.rs b/pyre/pyre-interpreter/src/module/_io/stringio.rs index 0f3064407af..7f10b9a382e 100644 --- a/pyre/pyre-interpreter/src/module/_io/stringio.rs +++ b/pyre/pyre-interpreter/src/module/_io/stringio.rs @@ -136,9 +136,10 @@ impl W_StringIO { if let Some(value) = newline && !matches!(value.as_bytes(), b"" | b"\n" | b"\r" | b"\r\n") { - let shown = unsafe { crate::display::py_repr(w_newline) }?; - return Err(crate::PyError::value_error(format!( - "illegal newline value: {shown}", + let shown = unsafe { crate::display::py_repr_wtf8(w_newline) }?; + return Err(crate::PyError::value_error(crate::display::wtf8_format!( + "illegal newline value: ", + shown ))); } diff --git a/pyre/pyre-interpreter/src/module/_io/textio.rs b/pyre/pyre-interpreter/src/module/_io/textio.rs index a1254d4bc39..d158878012d 100644 --- a/pyre/pyre-interpreter/src/module/_io/textio.rs +++ b/pyre/pyre-interpreter/src/module/_io/textio.rs @@ -1784,7 +1784,7 @@ impl W_TextIOWrapper { } } - fn __repr__(&self) -> Result { + fn __repr__(&self) -> Result { self.check_init()?; let self_obj = self.self_obj(); let Some(_guard) = crate::display::ReprGuard::enter(self_obj) else { @@ -1794,20 +1794,24 @@ impl W_TextIOWrapper { }; let typename = crate::type_methods::arg_type_name(self_obj); - let mut fields = String::new(); + let mut fields = rustpython_wtf8::Wtf8Buf::new(); if self.state != STATE_DETACHED { if let Ok(name) = crate::baseobjspace::getattr_str(self.w_buffer, "name") { fields.push_str(" name="); - fields.push_str(&unsafe { crate::display::py_repr(name)? }); + fields.push_wtf8(&unsafe { crate::display::py_repr_wtf8(name)? }); } } if let Ok(mode) = crate::baseobjspace::getattr_str(self_obj, "mode") { fields.push_str(" mode="); - fields.push_str(&unsafe { crate::display::py_repr(mode)? }); + fields.push_wtf8(&unsafe { crate::display::py_repr_wtf8(mode)? }); } fields.push_str(" encoding="); - fields.push_str(&unsafe { crate::display::py_repr(self.w_encoding)? }); - Ok(format!("<{typename}{fields}>")) + fields.push_wtf8(&unsafe { crate::display::py_repr_wtf8(self.w_encoding)? }); + Ok(crate::display::wtf8_format!( + format!("<{typename}"), + fields, + ">" + )) } } diff --git a/pyre/pyre-interpreter/src/module/_pickle/mod.rs b/pyre/pyre-interpreter/src/module/_pickle/mod.rs index 0bdd3f8a54c..702f8085ef1 100644 --- a/pyre/pyre-interpreter/src/module/_pickle/mod.rs +++ b/pyre/pyre-interpreter/src/module/_pickle/mod.rs @@ -175,10 +175,10 @@ pub(crate) fn call_meth( /// Build a `PyError` whose raised object is an instance of the named exception /// class, with `msg` as the single argument. Falls back to a generic ValueError /// carrying the same text if the class is somehow unavailable. -fn pickle_exc(class_name: &str, msg: String) -> PyError { +fn pickle_exc(class_name: &str, msg: rustpython_wtf8::Wtf8Buf) -> PyError { let mut err = PyError::value_error(msg.clone()); if let Some(cls) = crate::builtins::lookup_exc_class(class_name) { - let args = [cls, pyre_object::w_str_new(&msg)]; + let args = [cls, pyre_object::w_str_from_wtf8(msg)]; if let Ok(exc) = crate::builtins::exc_exception_new(&args) { err.exc_object = exc; } @@ -187,15 +187,15 @@ fn pickle_exc(class_name: &str, msg: String) -> PyError { } pub(crate) fn unpickling_error(msg: &str) -> PyError { - pickle_exc("_pickle.UnpicklingError", msg.to_string()) + pickle_exc("_pickle.UnpicklingError", msg.into()) } -pub(crate) fn pickling_error(msg: impl Into) -> PyError { +pub(crate) fn pickling_error(msg: impl Into) -> PyError { pickle_exc("_pickle.PicklingError", msg.into()) } pub(crate) fn eof_error(msg: &str) -> PyError { - pickle_exc("EOFError", msg.to_string()) + pickle_exc("EOFError", msg.into()) } // ── import / dotted attribute resolution (save_global / find_class) ─── @@ -469,11 +469,13 @@ pub(crate) fn getattribute_dotted_obj( .unwrap() }; if unsafe { pyre_object::w_str_get_wtf8(w_part) }.as_bytes() == b"" { - let qualname_repr = - unsafe { crate::py_repr(pyre_object::gc_roots::shadow_stack_get(qualname_slot)) } - .unwrap_or_else(|_| "".to_string()); - return Err(PyError::attribute_error(format!( - "Can't get local attribute {qualname_repr}" + let qualname_repr = unsafe { + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(qualname_slot)) + } + .unwrap_or_else(|_| rustpython_wtf8::Wtf8Buf::from_string("".to_string())); + return Err(PyError::attribute_error(crate::display::wtf8_format!( + "Can't get local attribute ", + qualname_repr ))); } parent_slot = cur_slot; diff --git a/pyre/pyre-interpreter/src/module/_pickle/pickler.rs b/pyre/pyre-interpreter/src/module/_pickle/pickler.rs index 6d279d6ff91..a21f1ef2c92 100644 --- a/pyre/pyre-interpreter/src/module/_pickle/pickler.rs +++ b/pyre/pyre-interpreter/src/module/_pickle/pickler.rs @@ -182,7 +182,11 @@ fn pickle_type_name(w_obj: PyObjectRef) -> Result { /// original exception object. `PyError` is a Rust carrier which the precise /// collector does not scan, so materialise and pin the exception before any /// type-name lookup or `add_note` call can collect. -fn add_pickle_object_note(mut err: PyError, w_obj: PyObjectRef, role: &str) -> PyError { +fn add_pickle_object_note( + mut err: PyError, + w_obj: PyObjectRef, + role: &rustpython_wtf8::Wtf8, +) -> PyError { let _roots = pyre_object::gc_roots::push_roots(); let w_exc = err.to_exc_object(); pyre_object::gc_roots::pin_root(w_exc); @@ -191,7 +195,10 @@ fn add_pickle_object_note(mut err: PyError, w_obj: PyObjectRef, role: &str) -> P let obj_slot = pyre_object::gc_roots::shadow_stack_len() - 1; if let Ok(type_name) = pickle_type_name(pyre_object::gc_roots::shadow_stack_get(obj_slot)) { - let w_note = pyre_object::w_str_new(&format!("when serializing {type_name} {role}")); + let w_note = pyre_object::w_str_from_wtf8(crate::display::wtf8_format!( + format!("when serializing {type_name} "), + role + )); pyre_object::gc_roots::pin_root(w_note); let note_slot = pyre_object::gc_roots::shadow_stack_len() - 1; if let Ok(w_add_note) = crate::baseobjspace::getattr_str( @@ -213,7 +220,11 @@ fn add_pickle_object_note(mut err: PyError, w_obj: PyObjectRef, role: &str) -> P err } -fn add_reduce_note(err: PyError, w_obj_slot: Option, role: &str) -> PyError { +fn add_reduce_note( + err: PyError, + w_obj_slot: Option, + role: &rustpython_wtf8::Wtf8, +) -> PyError { match w_obj_slot { Some(slot) => { add_pickle_object_note(err, pyre_object::gc_roots::shadow_stack_get(slot), role) @@ -1459,7 +1470,7 @@ fn save_reduce_value( add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(obj_slot), - "object", + rustpython_wtf8::Wtf8::new("object"), ) }) } @@ -1750,7 +1761,7 @@ fn save_tuple(ctx: &mut PickleCtx, buf: &mut Framer, w_obj: PyObjectRef) -> Resu return Err(add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(slot), - &format!("item {i}"), + rustpython_wtf8::Wtf8::new(format!("item {i}").as_str()), )); } } @@ -1774,7 +1785,7 @@ fn save_tuple(ctx: &mut PickleCtx, buf: &mut Framer, w_obj: PyObjectRef) -> Resu return Err(add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(slot), - &format!("item {i}"), + rustpython_wtf8::Wtf8::new(format!("item {i}").as_str()), )); } } @@ -1894,7 +1905,7 @@ fn save_set_items( return Err(add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(obj_slot), - "element", + rustpython_wtf8::Wtf8::new("element"), )); } let mut i = 1; @@ -1907,7 +1918,7 @@ fn save_set_items( return Err(add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(obj_slot), - "element", + rustpython_wtf8::Wtf8::new("element"), )); } i += 1; @@ -1917,7 +1928,7 @@ fn save_set_items( return Err(add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(obj_slot), - "element", + rustpython_wtf8::Wtf8::new("element"), )); } } @@ -1959,7 +1970,7 @@ fn save_frozenset( return Err(add_pickle_object_note( err, pyre_object::gc_roots::shadow_stack_get(fs_slot), - "element", + rustpython_wtf8::Wtf8::new("element"), )); } } @@ -2206,7 +2217,11 @@ fn batch_appends( let n = pinned_len(snapshot_slot); for i in 0..n { if let Err(err) = save(ctx, buf, pinned_get(snapshot_slot, i)) { - return Err(add_reduce_note(err, obj_slot, &format!("item {i}"))); + return Err(add_reduce_note( + err, + obj_slot, + rustpython_wtf8::Wtf8::new(format!("item {i}").as_str()), + )); } buf.push(op::APPEND); } @@ -2234,7 +2249,11 @@ fn batch_appends( buf, pyre_object::gc_roots::shadow_stack_get(first_slot), ) { - return Err(add_reduce_note(err, obj_slot, &format!("item {index}"))); + return Err(add_reduce_note( + err, + obj_slot, + rustpython_wtf8::Wtf8::new(format!("item {index}").as_str()), + )); } buf.push(op::APPEND); return Ok(()); @@ -2247,13 +2266,21 @@ fn batch_appends( buf, pyre_object::gc_roots::shadow_stack_get(first_slot), ) { - return Err(add_reduce_note(err, obj_slot, &format!("item {index}"))); + return Err(add_reduce_note( + err, + obj_slot, + rustpython_wtf8::Wtf8::new(format!("item {index}").as_str()), + )); } index += 1; let mut count = 1; loop { if let Err(err) = save(ctx, buf, pyre_object::gc_roots::shadow_stack_get(item_slot)) { - return Err(add_reduce_note(err, obj_slot, &format!("item {index}"))); + return Err(add_reduce_note( + err, + obj_slot, + rustpython_wtf8::Wtf8::new(format!("item {index}").as_str()), + )); } index += 1; count += 1; @@ -2313,8 +2340,12 @@ fn save_pair( // pickle.py only invokes the key's arbitrary __repr__ while // annotating a value-save failure. Successful dictionary saves // must not gain an observable repr call. - let key_repr = unsafe { crate::display::py_repr(pinned_get(pair_slot, 0))? }; - Err(add_reduce_note(err, obj_slot, &format!("item {key_repr}"))) + let key_repr = unsafe { crate::display::py_repr_wtf8(pinned_get(pair_slot, 0))? }; + Err(add_reduce_note( + err, + obj_slot, + &crate::display::wtf8_format!("item ", key_repr), + )) } } } @@ -2433,7 +2464,10 @@ fn memoize(ctx: &mut PickleCtx, buf: &mut Framer, w_obj: PyObjectRef) { /// Build a PicklingError while preserving the intercepted import/attribute /// exception as `__context__`, matching the native pickler's exception /// chaining rather than flattening it to text. -fn pickling_error_with_context(message: String, mut context: PyError) -> PyError { +fn pickling_error_with_context( + message: impl Into, + mut context: PyError, +) -> PyError { let _roots = pyre_object::gc_roots::push_roots(); let w_context = context.to_exc_object(); pyre_object::gc_roots::pin_root(w_context); @@ -2469,10 +2503,12 @@ fn whichmodule(w_obj: PyObjectRef, name: &str) -> Result { pyre_object::gc_roots::pin_root(w_obj); let obj_slot = pyre_object::gc_roots::shadow_stack_len() - 1; if name.split('.').any(|s| s == "") { - let obj_repr = - unsafe { crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(obj_slot))? }; - return Err(pickling_error(format!( - "Can't pickle local object {obj_repr}" + let obj_repr = unsafe { + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(obj_slot))? + }; + return Err(pickling_error(crate::display::wtf8_format!( + "Can't pickle local object ", + obj_repr ))); } // `interp_pickle.py:1738-1742 whichmodule` returns any non-None @@ -2597,11 +2633,11 @@ fn whichmodule(w_obj: PyObjectRef, name: &str) -> Result { ) => { let obj_repr = unsafe { - crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(obj_slot))? + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(obj_slot))? }; - let detail = error.message_text(); + let detail = error.message_wtf8(); return Err(pickling_error_with_context( - format!("Can't pickle {obj_repr}: {detail}"), + crate::display::wtf8_format!("Can't pickle ", obj_repr, ": ", detail), error, )); } @@ -2614,10 +2650,14 @@ fn whichmodule(w_obj: PyObjectRef, name: &str) -> Result { Ok((value, _)) => value, Err(error) if matches!(error.kind, crate::PyErrorKind::AttributeError) => { let obj_repr = unsafe { - crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(obj_slot))? + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(obj_slot))? }; return Err(pickling_error_with_context( - format!("Can't pickle {obj_repr}: it's not found as {module_name}.{name}"), + crate::display::wtf8_format!( + "Can't pickle ", + obj_repr, + format!(": it's not found as {module_name}.{name}") + ), error, )); } @@ -2626,10 +2666,13 @@ fn whichmodule(w_obj: PyObjectRef, name: &str) -> Result { if crate::baseobjspace::is_w(resolved, pyre_object::gc_roots::shadow_stack_get(obj_slot)) { Ok(ModuleName::Utf8(module_name)) } else { - let obj_repr = - unsafe { crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(obj_slot))? }; - Err(pickling_error(format!( - "Can't pickle {obj_repr}: it's not the same object as {module_name}.{name}" + let obj_repr = unsafe { + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(obj_slot))? + }; + Err(pickling_error(crate::display::wtf8_format!( + "Can't pickle ", + obj_repr, + format!(": it's not the same object as {module_name}.{name}") ))) } } @@ -2866,11 +2909,14 @@ fn identifier_encoding_error( let context_obj = context.to_exc_object(); pyre_object::gc_roots::pin_root(context_obj); let context_slot = pyre_object::gc_roots::shadow_stack_len() - 1; - let identifier_repr = - unsafe { crate::py_repr(pyre_object::gc_roots::shadow_stack_get(identifier_slot)) } - .unwrap_or_else(|_| "".to_string()); - let mut error = pickling_error(format!( - "can't pickle {kind} identifier {identifier_repr} using pickle protocol {proto}" + let identifier_repr = unsafe { + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(identifier_slot)) + } + .unwrap_or_else(|_| rustpython_wtf8::Wtf8Buf::from_string("".to_string())); + let mut error = pickling_error(crate::display::wtf8_format!( + format!("can't pickle {kind} identifier "), + identifier_repr, + format!(" using pickle protocol {proto}") )); let exc = error.to_exc_object(); pyre_object::gc_roots::pin_root(exc); @@ -3091,12 +3137,17 @@ fn save_reduce( if !crate::baseobjspace::is_w(args_get(0), w_class) { pyre_object::gc_roots::pin_root(w_class); let class_slot = pyre_object::gc_roots::shadow_stack_len() - 1; - let cls_repr = unsafe { crate::display::py_repr(args_get(0))? }; + let cls_repr = unsafe { crate::display::py_repr_wtf8(args_get(0))? }; let obj_class_repr = unsafe { - crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(class_slot))? + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get( + class_slot, + ))? }; - return Err(pickling_error(format!( - "first argument to __newobj_ex__() must be {obj_class_repr}, not {cls_repr}" + return Err(pickling_error(crate::display::wtf8_format!( + "first argument to __newobj_ex__() must be ", + obj_class_repr, + ", not ", + cls_repr ))); } } @@ -3114,13 +3165,25 @@ fn save_reduce( } if ctx.proto >= 4 { if let Err(err) = save(ctx, buf, args_get(0)) { - return Err(add_reduce_note(err, w_obj_slot, "class")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("class"), + )); } if let Err(err) = save(ctx, buf, args_get(1)) { - return Err(add_reduce_note(err, w_obj_slot, "__new__ arguments")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("__new__ arguments"), + )); } if let Err(err) = save(ctx, buf, args_get(2)) { - return Err(add_reduce_note(err, w_obj_slot, "__new__ arguments")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("__new__ arguments"), + )); } buf.push(op::NEWOBJ_EX); } else { @@ -3169,7 +3232,11 @@ fn save_reduce( pyre_object::gc_roots::pin_root(w_func); let func_slot = pyre_object::gc_roots::shadow_stack_len() - 1; if let Err(err) = save(ctx, buf, pyre_object::gc_roots::shadow_stack_get(func_slot)) { - return Err(add_reduce_note(err, w_obj_slot, "reconstructor")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("reconstructor"), + )); } let w_empty_args = pyre_object::tupleobject::w_tuple_new(Vec::new()); save(ctx, buf, w_empty_args)?; @@ -3194,12 +3261,17 @@ fn save_reduce( if !crate::baseobjspace::is_w(args_get(0), w_class) { pyre_object::gc_roots::pin_root(w_class); let class_slot = pyre_object::gc_roots::shadow_stack_len() - 1; - let cls_repr = unsafe { crate::display::py_repr(args_get(0))? }; + let cls_repr = unsafe { crate::display::py_repr_wtf8(args_get(0))? }; let obj_class_repr = unsafe { - crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(class_slot))? + crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get( + class_slot, + ))? }; - return Err(pickling_error(format!( - "first argument to __newobj__() must be {obj_class_repr}, not {cls_repr}" + return Err(pickling_error(crate::display::wtf8_format!( + "first argument to __newobj__() must be ", + obj_class_repr, + ", not ", + cls_repr ))); } } @@ -3208,22 +3280,38 @@ fn save_reduce( pyre_object::gc_roots::pin_root(w_newargs); let newargs_slot = pyre_object::gc_roots::shadow_stack_len() - 1; if let Err(err) = save(ctx, buf, args_get(0)) { - return Err(add_reduce_note(err, w_obj_slot, "class")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("class"), + )); } if let Err(err) = save( ctx, buf, pyre_object::gc_roots::shadow_stack_get(newargs_slot), ) { - return Err(add_reduce_note(err, w_obj_slot, "__new__ arguments")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("__new__ arguments"), + )); } buf.push(op::NEWOBJ); } else { if let Err(err) = save(ctx, buf, rv_get(0)) { - return Err(add_reduce_note(err, w_obj_slot, "reconstructor")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("reconstructor"), + )); } if let Err(err) = save(ctx, buf, rv_get(1)) { - return Err(add_reduce_note(err, w_obj_slot, "reconstructor arguments")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("reconstructor arguments"), + )); } buf.push(op::REDUCE); } @@ -3251,21 +3339,33 @@ fn save_reduce( if has_state { if has_state_setter { if let Err(err) = save(ctx, buf, rv_get(5)) { - return Err(add_reduce_note(err, w_obj_slot, "state setter")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("state setter"), + )); } let state_obj = w_obj_slot .map(pyre_object::gc_roots::shadow_stack_get) .unwrap_or_else(pyre_object::w_none); save(ctx, buf, state_obj)?; if let Err(err) = save(ctx, buf, rv_get(2)) { - return Err(add_reduce_note(err, w_obj_slot, "state")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("state"), + )); } buf.push(op::TUPLE2); buf.push(op::REDUCE); buf.push(op::POP); } else { if let Err(err) = save(ctx, buf, rv_get(2)) { - return Err(add_reduce_note(err, w_obj_slot, "state")); + return Err(add_reduce_note( + err, + w_obj_slot, + rustpython_wtf8::Wtf8::new("state"), + )); } buf.push(op::BUILD); } diff --git a/pyre/pyre-interpreter/src/module/_socket/interp_socket.rs b/pyre/pyre-interpreter/src/module/_socket/interp_socket.rs index 048ffe2272e..aed777dab86 100644 --- a/pyre/pyre-interpreter/src/module/_socket/interp_socket.rs +++ b/pyre/pyre-interpreter/src/module/_socket/interp_socket.rs @@ -786,9 +786,12 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { )); } let end = buf.iter().position(|&b| b == 0).unwrap_or(buf.len()); - Ok(pyre_object::w_str_new(&String::from_utf8_lossy( - &buf[..end], - ))) + // `interp_func.py:24` is + // `space.fsdecode(space.newbytes(res))` -- the hostname is + // opaque kernel bytes (`sethostname(2)` takes a plain + // `const char*`), so a byte with no UTF-8 spelling has to + // survive as its surrogate escape. + Ok(crate::gateway::fsdecode_filename_bytes(&buf[..end])) }, 0, ), @@ -1290,12 +1293,17 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { let mut p = head; unsafe { while (*p).if_index != 0 && !(*p).if_name.is_null() { - let name = std::ffi::CStr::from_ptr((*p).if_name) - .to_string_lossy() - .into_owned(); + // An interface name is an OS string: `dev_valid_name` + // rejects only NUL, '/', ':', whitespace, '.' and + // '..', so any other octet is legal. The sibling + // `if_nametoindex` below fsencodes, so decoding + // here any other way breaks the round trip. + let name = crate::gateway::fsdecode_filename_bytes( + std::ffi::CStr::from_ptr((*p).if_name).to_bytes(), + ); items.push(pyre_object::w_tuple_new(vec![ pyre_object::w_int_new((*p).if_index as i64), - pyre_object::w_str_new(&name), + name, ])); p = p.add(1); } @@ -1317,12 +1325,15 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { "if_nametoindex() requires 1 argument", )); } - // An interface name is an OS string. PyPy has no - // `if_nametoindex`, so there is no `unwrap_spec` to port; - // `socketmodule.c socket_if_nametoindex` reads it with - // `PyUnicode_FSConverter`, which is the same filesystem - // encoding `fsencode_w` applies and accepts the same - // str / bytes / `__fspath__` argument. + // An interface name is an OS string. + // `interp_socket.py:1316` declares `name='text'` and + // compares against `rsocket.if_nameindex()`'s own names; + // `socketmodule.c socket_if_nametoindex` instead reads it + // with `PyUnicode_FSConverter`, the filesystem encoding + // `fsencode_w` applies, and accepts the same + // str / bytes / `__fspath__` argument. Take the 3.14 + // spelling, which is also what makes this round-trip with + // the `if_nameindex` / `if_indextoname` decode above. let name = crate::gateway::fsencode_bytes_w(args[0])?; let c_name = std::ffi::CString::new(name) .map_err(|_| crate::PyError::value_error("embedded null in name"))?; @@ -1354,7 +1365,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { return Err(socket_io_err(std::io::Error::last_os_error())); } let s = unsafe { std::ffi::CStr::from_ptr(p) }; - Ok(pyre_object::w_str_new(&s.to_string_lossy())) + Ok(crate::gateway::fsdecode_filename_bytes(s.to_bytes())) }, 1, ), diff --git a/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs b/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs index 0e397f6f8c5..0abd501914a 100644 --- a/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs +++ b/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs @@ -1689,24 +1689,37 @@ fn sre_match_expand(args: &[PyObjectRef]) -> Result /// match=R>` with `R` the repr of the whole match truncated to 50 /// characters. Positions are character offsets for a `str` subject and /// byte offsets for a bytes-like subject (the sre-engine driver's units). -pub(crate) fn sre_match_repr_str(m: PyObjectRef) -> Result { +pub(crate) fn sre_match_repr_str( + m: PyObjectRef, +) -> Result { let mp = m as *const W_SRE_Match; let span = unsafe { w_sre_match_get_span(m, 0) }.unwrap_or((-1, -1)); let (start, end) = span; let subj = unsafe { subject_of((*mp).w_string, (*mp).w_buffer) }; let w_match_str = slice_subject(subj, span, w_none()); - let matchrepr: String = unsafe { crate::py_repr(w_match_str) }? - .chars() - .take(50) - .collect(); - Ok(format!( - "" + let matchrepr = truncate_code_points(unsafe { crate::display::py_repr_wtf8(w_match_str) }?, 50); + Ok(crate::display::wtf8_format!( + format!("", )) } +/// The first `limit` code points of `text`, for the repr truncations that +/// `str[:limit]` performs on a subject that may hold a lone surrogate. +fn truncate_code_points(text: rustpython_wtf8::Wtf8Buf, limit: usize) -> rustpython_wtf8::Wtf8Buf { + let mut out = rustpython_wtf8::Wtf8Buf::new(); + for cp in text.code_points().take(limit) { + out.push(cp); + } + out +} + fn sre_match_repr(args: &[PyObjectRef]) -> Result { let m = sre_match_self(args)?; - Ok(w_str_new(&sre_match_repr_str(m as PyObjectRef)?)) + Ok(pyre_object::w_str_from_wtf8(sre_match_repr_str( + m as PyObjectRef, + )?)) } /// `copy_identity_w` (interp_sre.py:701-702) — match results are @@ -1747,10 +1760,12 @@ const SRE_FLAG_NAMES: [&str; 9] = [ /// with the pattern repr truncated to 200 characters and the flag bits /// decoded into their `re.*` names (the implicit `re.UNICODE` on a known /// unicode pattern is suppressed, :160-165). -pub(crate) fn sre_pattern_repr_str(pat: PyObjectRef) -> Result { +pub(crate) fn sre_pattern_repr_str( + pat: PyObjectRef, +) -> Result { let pp = pat as *const W_SRE_Pattern; let w_pattern = unsafe { (*pp).w_pattern }; - let u: String = unsafe { crate::py_repr(w_pattern) }?.chars().take(200).collect(); + let u = truncate_code_points(unsafe { crate::display::py_repr_wtf8(w_pattern) }?, 200); let mut flags = unsafe { (*pp).flags }; let is_known_unicode = unsafe { is_str(w_pattern) }; @@ -1769,16 +1784,19 @@ pub(crate) fn sre_pattern_repr_str(pat: PyObjectRef) -> Result Result { let pat = sre_pattern_self(args)?; - Ok(w_str_new(&sre_pattern_repr_str(pat as PyObjectRef)?)) + Ok(pyre_object::w_str_from_wtf8(sre_pattern_repr_str( + pat as PyObjectRef, + )?)) } /// `descr_eq` (interp_sre.py:180-190): compare flags, compiled code, and diff --git a/pyre/pyre-interpreter/src/module/_symtable/mod.rs b/pyre/pyre-interpreter/src/module/_symtable/mod.rs index f7d271fc1e1..d42882ca5ac 100644 --- a/pyre/pyre-interpreter/src/module/_symtable/mod.rs +++ b/pyre/pyre-interpreter/src/module/_symtable/mod.rs @@ -190,7 +190,7 @@ fn symtable_data(args: &[PyObjectRef]) -> crate::PyResult { } else if pyre_object::is_bytes(arg) { crate::compile::decode_source_bytes( pyre_object::bytesobject::bytes_like_data(arg), - &filename, + pyre_object::w_str_get_wtf8(pyre_object::gc_roots::shadow_stack_get(filename_slot)), false, )? } else { diff --git a/pyre/pyre-interpreter/src/module/_tokenize/mod.rs b/pyre/pyre-interpreter/src/module/_tokenize/mod.rs index da9c9438f96..b374dfa60b0 100644 --- a/pyre/pyre-interpreter/src/module/_tokenize/mod.rs +++ b/pyre/pyre-interpreter/src/module/_tokenize/mod.rs @@ -76,7 +76,7 @@ enum TokenizerPhase { pub struct W_TokenizerIter { readline: PyObjectRef, extra_tokens: bool, - encoding: Option, + encoding: Option, phase: TokenizerPhase, source: String, tokens: Vec, @@ -128,8 +128,16 @@ fn read_line(self_obj: PyObjectRef) -> Result { // Decoding may enter the codec registry. Do not retain a slice // borrowed from a movable bytes object across that call. let bytes = pyre_object::bytesobject::bytes_like_data(raw).to_vec(); - let decoded = crate::typedef::decode_bytes_to_wtf8(&bytes, &encoding, "strict")?; - Ok(decoded.to_string_lossy().into_owned()) + // No registered codec spells its name with a surrogate, so a name + // with no `str` form is simply one the registry does not have. + let Ok(name) = encoding.as_str() else { + let mut msg = + rustpython_wtf8::Wtf8Buf::from_string("unknown encoding: ".to_string()); + msg.push_wtf8(&encoding); + return Err(crate::PyError::new(crate::PyErrorKind::LookupError, msg)); + }; + let decoded = crate::typedef::decode_bytes_to_wtf8(&bytes, name, "strict")?; + crate::typedef::utf8_strict_w(decoded) }, None => unsafe { if !is_str(raw) { @@ -137,7 +145,7 @@ fn read_line(self_obj: PyObjectRef) -> Result { "readline() returned a non-string object", )); } - Ok(w_str_get_wtf8(raw).to_string_lossy().into_owned()) + crate::typedef::utf8_strict_w(w_str_get_wtf8(raw).to_wtf8_buf()) }, } } @@ -220,7 +228,7 @@ impl W_TokenizerIter { type_name_of(value) ))); } - Some(w_str_get_wtf8(value).to_string_lossy().into_owned()) + Some(w_str_get_wtf8(value).to_wtf8_buf()) }, None => None, }; diff --git a/pyre/pyre-interpreter/src/module/_weakref/interp__weakref.rs b/pyre/pyre-interpreter/src/module/_weakref/interp__weakref.rs index 5afaf1432f3..72851c5dfa4 100644 --- a/pyre/pyre-interpreter/src/module/_weakref/interp__weakref.rs +++ b/pyre/pyre-interpreter/src/module/_weakref/interp__weakref.rs @@ -1048,7 +1048,7 @@ pub fn finalize_weakrefs(w_obj: PyObjectRef) { crate::executioncontext::report_error( unsafe { (*ec).space }, &error, - "weakref callback ", + rustpython_wtf8::Wtf8::new("weakref callback "), current_callable(), ); } @@ -2257,7 +2257,10 @@ mod tests { write_attr(proxy, ATTR_W_OBJ_WEAK, pyre_object::w_none()); let err = force(proxy).unwrap_err(); assert_eq!(err.kind, crate::PyErrorKind::ReferenceError); - assert_eq!(err.message, "weakly referenced object no longer exists"); + assert_eq!( + err.message_text(), + "weakly referenced object no longer exists" + ); } /// `len(proxy)` must dispatch through `proxy_typedef_dict["__len__"]`, diff --git a/pyre/pyre-interpreter/src/module/binascii/mod.rs b/pyre/pyre-interpreter/src/module/binascii/mod.rs index d44fa067e81..9152bc3c580 100644 --- a/pyre/pyre-interpreter/src/module/binascii/mod.rs +++ b/pyre/pyre-interpreter/src/module/binascii/mod.rs @@ -74,11 +74,16 @@ fn arg_data_posonly( } if let Some(dict) = kwargs { for (key, _) in unsafe { pyre_object::w_dict_str_entries_wtf8(dict) }.iter() { - let name = key.to_string_lossy(); - if name != "__pyre_kw__" && !kwonly.contains(&name.as_ref()) { - return Err(crate::PyError::type_error(format!( - "{fn_name}() got an unexpected keyword argument '{name}'" - ))); + // A keyword can be any `str`, so the name is compared and reported + // as the WTF-8 it is: `format!` would fold a surrogate to U+FFFD. + let named = key.as_str().ok(); + if named != Some("__pyre_kw__") && !named.is_some_and(|n| kwonly.contains(&n)) { + let mut msg = rustpython_wtf8::Wtf8Buf::from_string(format!( + "{fn_name}() got an unexpected keyword argument '" + )); + msg.push_wtf8(key); + msg.push_str("'"); + return Err(crate::PyError::type_error(msg)); } } } diff --git a/pyre/pyre-interpreter/src/module/importlib/interp_importlib.rs b/pyre/pyre-interpreter/src/module/importlib/interp_importlib.rs index 35dd0d51f6d..6bf6549e858 100644 --- a/pyre/pyre-interpreter/src/module/importlib/interp_importlib.rs +++ b/pyre/pyre-interpreter/src/module/importlib/interp_importlib.rs @@ -63,8 +63,8 @@ pub fn register_pkg(ns: pyre_object::PyObjectRef) { // because the full-name builtin check precedes the __path__ disk search. #[cfg(feature = "host_env")] let path_items = match crate::importing::detect_stdlib_path() { - Some(dir) => vec![pyre_object::w_str_new( - &dir.join("importlib").to_string_lossy(), + Some(dir) => vec![crate::gateway::fsdecode_os_str( + dir.join("importlib").as_os_str(), )], None => vec![], }; diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index cd0a83b8c2c..a95f4fbe253 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -231,7 +231,11 @@ fn run_fork_callbacks(kind: &str) { let Some(callback) = callback else { continue }; if let Err(mut error) = crate::call::call_function_impl_result(callback as PyObjectRef, &[]) { - error.write_unraisable(pyre_object::w_none(), "fork hook", callback as PyObjectRef); + error.write_unraisable( + pyre_object::w_none(), + rustpython_wtf8::Wtf8::new("fork hook"), + callback as PyObjectRef, + ); } } } @@ -444,27 +448,44 @@ fn times_result_seq_type() -> PyObjectRef { /// ending in `:` is drive-relative, anything else is a plain relative part. /// /// Both separators count, so the tests are taken on a copy with `/` rewritten -/// to `\`; that rewrite is character-for-character, so an offset into it -/// addresses the same character of the original. The offsets are per -/// character rather than per byte, which is what makes `"ä:\x"` take the -/// drive branch — at byte 1 it would be a continuation byte and take none. +/// to `\`; that rewrite is code-point-for-code-point, so an offset into it +/// addresses the same code point of the original. The offsets are per code +/// point rather than per byte, which is what makes `"ä:\x"` take the drive +/// branch — at byte 1 it would be a continuation byte and take none. /// /// Compiled everywhere so the tests run on every platform; only the Windows /// build has a caller. #[cfg_attr(not(windows), allow(dead_code))] -fn split_root(path: &str) -> (&str, &str) { - const SEP: char = '\\'; +fn split_root(path: &rustpython_wtf8::Wtf8) -> (&rustpython_wtf8::Wtf8, &rustpython_wtf8::Wtf8) { + use rustpython_wtf8::Wtf8; + + const SEP: u32 = '\\' as u32; + const COLON: u32 = ':' as u32; const UNC_PREFIX: &str = "\\\\?\\UNC\\"; - let norm: Vec = path - .chars() - .map(|c| if c == '/' { SEP } else { c }) + // A path is a sequence of code points, and one of them may be a lone + // surrogate that `str` cannot hold; the separators the split looks for are + // all ASCII, so the scan runs over the code-point values. + let norm: Vec = path + .code_points() + .map(|c| if c.to_u32() == '/' as u32 { SEP } else { c.to_u32() }) .collect(); let byte_at = |index: usize| { - path.char_indices() + path.code_point_indices() .nth(index) .map_or(path.len(), |(offset, _)| offset) }; + let split_at = |offset: usize| { + let bytes = path.as_bytes(); + // The offset comes from `code_point_indices`, so both halves are + // code-point aligned and stay well-formed WTF-8. + unsafe { + ( + Wtf8::from_bytes_unchecked(&bytes[..offset]), + Wtf8::from_bytes_unchecked(&bytes[offset..]), + ) + } + }; let sep_from = |start: usize| { norm.get(start..) .and_then(|rest| rest.iter().position(|&c| c == SEP)) @@ -472,17 +493,17 @@ fn split_root(path: &str) -> (&str, &str) { }; if norm.first() != Some(&SEP) { - if norm.get(1) == Some(&':') { + if norm.get(1) == Some(&COLON) { // `X:\Windows` keeps the separator in the root; `X:Windows` names // a location on the drive's own cursor and has no root at all. let split = if norm.get(2) == Some(&SEP) { 3 } else { 2 }; - return path.split_at(byte_at(split)); + return split_at(byte_at(split)); } - return ("", path); + return (Wtf8::new(""), path); } if norm.get(1) != Some(&SEP) { // A path rooted on the current drive, e.g. `\Windows`. - return path.split_at(byte_at(1)); + return split_at(byte_at(1)); } // A UNC share (`\\server\share`, `\\?\UNC\server\share`) or a device // (`\\.\device`): the root runs to the separator after the share name, @@ -490,12 +511,15 @@ fn split_root(path: &str) -> (&str, &str) { let unc = norm.len() >= 8 && norm[..8] .iter() - .map(|c| c.to_ascii_uppercase()) - .eq(UNC_PREFIX.chars()); + .map(|&c| match u8::try_from(c) { + Ok(b) => u32::from(b.to_ascii_uppercase()), + Err(_) => c, + }) + .eq(UNC_PREFIX.chars().map(u32::from)); let start = if unc { 8 } else { 2 }; match sep_from(start).and_then(|index| sep_from(index + 1)) { - Some(index) => path.split_at(byte_at(index + 1)), - None => (path, ""), + Some(index) => split_at(byte_at(index + 1)), + None => (path, Wtf8::new("")), } } @@ -543,7 +567,8 @@ mod win_nt { fn arg_path( args: &[PyObjectRef], func: &str, - ) -> Result<(String, bool, crate::gateway::FsEncodedPath), crate::PyError> { + ) -> Result<(std::ffi::OsString, bool, crate::gateway::FsEncodedPath), crate::PyError> { + use std::os::windows::ffi::OsStringExt; let Some(&arg) = args.first() else { return Err(crate::PyError::type_error(format!( "{func}() missing required argument 'path'" @@ -551,18 +576,26 @@ mod win_nt { }; let resolved = crate::gateway::fsencode_path_w(arg)?; let as_bytes = unsafe { resolved.is_bytes() }; - // Windows names files in UTF-16, so there is no byte spelling to keep - // here the way there is on a unix path; the host API takes text. - let path = String::from_utf8_lossy(&resolved.as_bytes).into_owned(); - Ok((path, as_bytes, resolved)) + // Windows names files in UTF-16, so the path reaches the host API as + // code units rather than bytes. Going through a Rust `String` on the + // way would replace an undecodable byte with U+FFFD, and the call + // would then address a different file than the caller named -- + // `interp_posix.py:866-884` keeps the syscall spelling intact for the + // same reason. + let wide: Vec = crate::gateway::fsdecode_filename_wtf8(&resolved.as_bytes) + .encode_wide() + .collect(); + Ok((std::ffi::OsString::from_wide(&wide), as_bytes, resolved)) } fn wrap_path(s: &std::ffi::OsStr, as_bytes: bool) -> PyObjectRef { - let text = s.to_string_lossy(); + // One decode feeds both arms: the bytes form is the filesystem + // encoding of the same text, not the UTF-8 of a lossy rendering of it. + let text = crate::gateway::fsdecode_os_str_wtf8(s); if as_bytes { pyre_object::w_bytes_from_bytes(text.as_bytes()) } else { - pyre_object::w_str_new(&text) + pyre_object::w_str_from_wtf8(text) } } @@ -580,7 +613,7 @@ mod win_nt { /// backup-semantics handle so directories open too. pub fn _getfinalpathname(args: &[PyObjectRef]) -> Result { let (path, as_bytes, resolved) = arg_path(args, "_getfinalpathname")?; - if path.contains('\0') { + if path.as_encoded_bytes().contains(&0) { return Err(crate::PyError::value_error("embedded null character")); } match host_nt::getfinalpathname(Path::new(&path)) { @@ -619,7 +652,7 @@ mod win_nt { /// (ERROR_DIRECTORY). pub fn _getdiskusage(args: &[PyObjectRef]) -> Result { let (path, _, resolved) = arg_path(args, "_getdiskusage")?; - if path.contains('\0') { + if path.as_encoded_bytes().contains(&0) { return Err(crate::PyError::value_error("embedded null character")); } match host_nt::getdiskusage(Path::new(&path)) { @@ -770,10 +803,15 @@ fn create_environ() -> pyre_object::PyObjectRef { // _create_environ_mapping demands str keys/values and upper-cases the // keys itself. (_convertenviron: `space.newtext(key), newtext(value)`.) for (key, value) in host_os::vars_os() { + // `_convertenviron`'s Windows arm reads `rwin32._wenviron_items()`, + // the wide-char environment, and keeps those code units. + // `fsdecode_os_str` carries them across with `from_wide`; a lossy + // decode would fold an unpaired one to U+FFFD and stop + // `os.environ` round-tripping. store( dict_slot, - || pyre_object::w_str_new(&key.to_string_lossy()), - || pyre_object::w_str_new(&value.to_string_lossy()), + || crate::gateway::fsdecode_os_str(&key), + || crate::gateway::fsdecode_os_str(&value), ); } } @@ -2620,11 +2658,11 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // Splitting a drive or UNC prefix is a text operation on a // Windows path, and both halves are handed back as `str`, so // this one stays in the text domain rather than the byte one. - let path = String::from_utf8_lossy(&path); + let path = crate::gateway::fsdecode_filename_wtf8(&path); let (root, tail) = split_root(&path); Ok(pyre_object::w_tuple_new(vec![ - pyre_object::w_str_new(root), - pyre_object::w_str_new(tail), + pyre_object::w_str_from_wtf8(root.to_wtf8_buf()), + pyre_object::w_str_from_wtf8(tail.to_wtf8_buf()), ])) }, 1, @@ -4719,7 +4757,10 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { { let msg = crate::host_seam::ops::strerror(code) .map_err(|e| crate::host_seam::seam_os_err(e, ""))?; - return Ok(pyre_object::w_str_new(&String::from_utf8_lossy(&msg))); + // The text comes from the C library in the current + // locale, so a byte with no UTF-8 spelling takes the + // surrogateescape rather than U+FFFD. + return Ok(crate::typedef::charp2uni(&msg)); } #[cfg(not(feature = "sandbox"))] Ok(pyre_object::w_str_new( @@ -4809,7 +4850,9 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { crate::make_builtin_function_with_arity( "getlogin", |_| match host_posix::getlogin() { - Some(name) => Ok(pyre_object::w_str_new(name.to_string_lossy().as_ref())), + // A login name is an OS string; decode it the way every + // other one is so an undecodable byte keeps its escape. + Some(name) => Ok(crate::gateway::fsdecode_filename_bytes(name.as_bytes())), None => Err(crate::PyError::os_error_with_errno( crate::builtins::io_error_posix_errno(&std::io::Error::last_os_error(), 0), "getlogin", @@ -7060,7 +7103,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { let fd = crate::baseobjspace::c_int_w(args[0])?; let bfd = fd_borrow(fd)?; let name = host_posix::ttyname(bfd).map_err(|e| io_err(e, ""))?; - Ok(pyre_object::w_str_new(&name.to_string_lossy())) + Ok(crate::gateway::fsdecode_os_str(name.as_os_str())) }, 1, ), @@ -8618,6 +8661,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { #[cfg(test)] mod split_root_tests { use super::split_root; + use rustpython_wtf8::{CodePoint, Wtf8, Wtf8Buf}; /// Expectations taken from `ntpath.splitroot`, whose three-way split /// joins drive and root into the root this returns. @@ -8660,7 +8704,12 @@ mod split_root_tests { ), ]; for &(path, root, tail) in cases { - assert_eq!(split_root(path), (root, tail), "split_root({path:?})"); + let (got_root, got_tail) = split_root(Wtf8::new(path)); + assert_eq!( + (got_root.as_str(), got_tail.as_str()), + (Ok(root), Ok(tail)), + "split_root({path:?})" + ); assert_eq!( format!("{root}{tail}"), path, @@ -8668,4 +8717,17 @@ mod split_root_tests { ); } } + + /// A path carrying a lone surrogate — what `fsdecode` produces for an + /// undecodable name — splits on the same boundary and keeps the code point. + #[test] + fn keeps_a_lone_surrogate() { + let mut path = Wtf8Buf::from_string("C:\\".to_string()); + path.push(CodePoint::from_u32(0xdcff).unwrap()); + path.push_str("x"); + let (root, tail) = split_root(&path); + assert_eq!(root.as_str(), Ok("C:\\")); + assert_eq!(tail.code_points().next().map(|c| c.to_u32()), Some(0xdcff)); + assert_eq!(tail.len(), path.len() - root.len()); + } } diff --git a/pyre/pyre-interpreter/src/module/signal/interp_signal.rs b/pyre/pyre-interpreter/src/module/signal/interp_signal.rs index 4092cd5ec10..0871dadd1ee 100644 --- a/pyre/pyre-interpreter/src/module/signal/interp_signal.rs +++ b/pyre/pyre-interpreter/src/module/signal/interp_signal.rs @@ -72,7 +72,7 @@ fn errno_exception(class_name: &str, errno: i32) -> crate::PyError { ]; let exc = crate::builtins::exc_os_error_new(&args) .expect("exc_os_error_new is infallible for int/str args"); - let mut err = crate::PyError::os_error(&strerror); + let mut err = crate::PyError::os_error(strerror); err.exc_object = exc; err } diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index ed3394c9238..ea758b2c318 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -363,15 +363,21 @@ fn simple_namespace_repr(args: &[PyObjectRef]) -> crate::PyResult { // `getitem` above ran a lookup that can collect, so the key has to be // reread from its slot rather than reused from before the call. let key = pyre_object::gc_roots::shadow_stack_get(keys_sp + i); - parts.push(format!( - "{}={}", - unsafe { crate::display::py_str(key)? }, - unsafe { - crate::display::py_repr(pyre_object::gc_roots::shadow_stack_get(value_sp))? - } + parts.push(crate::display::wtf8_format!( + unsafe { crate::display::py_str_wtf8(key)? }, + "=", + unsafe { crate::display::py_repr_wtf8(pyre_object::gc_roots::shadow_stack_get(value_sp))? } )); } - Ok(w_str_new(&format!("{name}({})", parts.join(", ")))) + let mut text = rustpython_wtf8::Wtf8Buf::from_string(format!("{name}(")); + for (index, part) in parts.iter().enumerate() { + if index > 0 { + text.push_str(", "); + } + text.push_wtf8(part); + } + text.push_str(")"); + Ok(pyre_object::w_str_from_wtf8(text)) } /// `_structseq.py:185 SimpleNamespace.__eq__` — structural over `__dict__` @@ -994,11 +1000,11 @@ fn sys_unraisablehook(args: &[PyObjectRef]) -> crate::PyResult { let w_tb = crate::baseobjspace::getattr_str(w_hookargs, "exc_traceback")?; let w_err_msg = crate::baseobjspace::getattr_str(w_hookargs, "err_msg")?; let err_msg = if unsafe { pyre_object::is_none(w_err_msg) } { - String::new() + rustpython_wtf8::Wtf8Buf::new() } else if unsafe { pyre_object::is_str(w_err_msg) } { - unsafe { pyre_object::w_str_get_value(w_err_msg) }.to_string() + unsafe { pyre_object::w_str_get_wtf8(w_err_msg) }.to_wtf8_buf() } else { - unsafe { crate::display::py_str(w_err_msg)? } + unsafe { crate::display::py_str_wtf8(w_err_msg)? } }; let w_object = crate::baseobjspace::getattr_str(w_hookargs, "object")?; crate::PyError::write_unraisable_default( diff --git a/pyre/pyre-interpreter/src/module/thread/mod.rs b/pyre/pyre-interpreter/src/module/thread/mod.rs index 78e8ba2c9ad..d044229e872 100644 --- a/pyre/pyre-interpreter/src/module/thread/mod.rs +++ b/pyre/pyre-interpreter/src/module/thread/mod.rs @@ -1621,7 +1621,7 @@ fn spawn_thread( if has_handle { let h = W_ThreadHandle::from_obj(handle_addr as PyObjectRef).unwrap(); if let Err(e) = h.start(ident) { - bootstrap.fail(e.message); + bootstrap.fail(e.message_text()); thread_is_stopping(&mut ec); crate::call::set_last_exec_ctx(std::ptr::null()); drop(worker_roots); @@ -1656,11 +1656,17 @@ fn spawn_thread( // driver owner is made interpreter-global. let _plain_worker = crate::call::force_plain_eval(); if let Err(mut error) = call_thread_target(callable, &args, kwargs, ec_ptr) { - let callable_repr = - unsafe { crate::py_repr(callable).unwrap_or_else(|_| "".to_string()) }; + let callable_repr = unsafe { + crate::display::py_repr_wtf8(callable).unwrap_or_else(|_| { + rustpython_wtf8::Wtf8Buf::from_string("".to_string()) + }) + }; error.write_unraisable( w_none(), - &format!("Exception ignored in thread started by {callable_repr}"), + &crate::display::wtf8_format!( + "Exception ignored in thread started by ", + callable_repr + ), w_none(), ); } diff --git a/pyre/pyre-interpreter/src/module/time/interp_time.rs b/pyre/pyre-interpreter/src/module/time/interp_time.rs index 8e225dd6560..dbcff088e47 100644 --- a/pyre/pyre-interpreter/src/module/time/interp_time.rs +++ b/pyre/pyre-interpreter/src/module/time/interp_time.rs @@ -109,9 +109,12 @@ pub fn sleep(args: &[PyObjectRef]) -> Result { const SECS_TO_NS: i64 = 1_000_000_000; let timeout_ns: i64 = unsafe { let overflow = || { - crate::PyError::overflow_error(format!( - "timestamp {} too large to convert to C _PyTime_t", - crate::py_repr(args[0]).unwrap_or_else(|_| "".to_string()) + crate::PyError::overflow_error(crate::display::wtf8_format!( + "timestamp ", + crate::display::py_repr_wtf8(args[0]).unwrap_or_else(|_| { + rustpython_wtf8::Wtf8Buf::from_string("".to_string()) + }), + " too large to convert to C _PyTime_t" )) }; if is_float(args[0]) { @@ -1401,15 +1404,16 @@ pub fn strftime(args: &[PyObjectRef]) -> Result { &msvc_tm, ); if n != 0 { - return Ok(w_str_from_wtf8( - rustpython_wtf8::Wtf8Buf::from_bytes(buf[..n].to_vec()).unwrap_or_else( - |b| { - rustpython_wtf8::Wtf8Buf::from_string( - String::from_utf8_lossy(&b).into_owned(), - ) - }, - ), - )); + // The same recovery as the unix arm above: unrecognised + // directive bytes are echoed verbatim, so a format that is + // a lone surrogate's WTF-8 encoding comes back unchanged, + // and genuinely undecodable locale output takes + // surrogateescape rather than raising. + let rendered = buf[..n].to_vec(); + return Ok(match rustpython_wtf8::Wtf8Buf::from_bytes(rendered) { + Ok(wtf8) => w_str_from_wtf8(wtf8), + Err(bytes) => crate::typedef::charp2uni(&bytes), + }); } if buf.len() > 16384 { return Ok(w_str_new("")); diff --git a/pyre/pyre-interpreter/src/module/unicodedata/mod.rs b/pyre/pyre-interpreter/src/module/unicodedata/mod.rs index 63c9cfb3a8e..76dcfc8c639 100644 --- a/pyre/pyre-interpreter/src/module/unicodedata/mod.rs +++ b/pyre/pyre-interpreter/src/module/unicodedata/mod.rs @@ -20,7 +20,7 @@ use pyre_object::*; use rustpython_unicode::{self as ucd_core, NormalizeForm}; -use rustpython_wtf8::{CodePoint, Wtf8}; +use rustpython_wtf8::{CodePoint, Wtf8, Wtf8Buf}; use crate::{PyError, PyErrorKind}; @@ -232,10 +232,10 @@ fn lookup(args: &[PyObjectRef]) -> PyResult { buf.push(ch); return Ok(w_str_new(&buf)); } - Err(PyError::key_error(format!( - "undefined character name '{}'", - name.to_string_lossy() - ))) + let mut msg = Wtf8Buf::from_string("undefined character name '".to_string()); + msg.push_wtf8(name); + msg.push_str("'"); + Err(PyError::key_error(msg)) } /// Parse the normalization-form argument (`NFC`/`NFKC`/`NFD`/`NFKD`). diff --git a/pyre/pyre-interpreter/src/objspace/descroperation.rs b/pyre/pyre-interpreter/src/objspace/descroperation.rs index 5220f87a367..939a995bfad 100644 --- a/pyre/pyre-interpreter/src/objspace/descroperation.rs +++ b/pyre/pyre-interpreter/src/objspace/descroperation.rs @@ -6056,12 +6056,12 @@ mod tests { let huge = BigInt::one().lshift(2000).unwrap(); let error = pow(w_long_new(huge), w_int_new(-1)).unwrap_err(); assert_eq!(error.kind, PyErrorKind::OverflowError); - assert_eq!(error.message, "int too large to convert to float"); + assert_eq!(error.message_text(), "int too large to convert to float"); let huge_negative_exponent = BigInt::one().lshift(2000).unwrap().neg(); let error = pow(w_int_new(1), w_long_new(huge_negative_exponent)).unwrap_err(); assert_eq!(error.kind, PyErrorKind::OverflowError); - assert_eq!(error.message, "int too large to convert to float"); + assert_eq!(error.message_text(), "int too large to convert to float"); } #[test] diff --git a/pyre/pyre-interpreter/src/opcode_ops.rs b/pyre/pyre-interpreter/src/opcode_ops.rs index b686ba7117c..7984bf38bc8 100644 --- a/pyre/pyre-interpreter/src/opcode_ops.rs +++ b/pyre/pyre-interpreter/src/opcode_ops.rs @@ -281,9 +281,11 @@ pub fn match_keys_value(subject: PyObjectRef, keys: PyObjectRef) -> Result Result { - unsafe { crate::display::py_repr(self.mapping()?) } + fn __repr__(&self) -> Result { + unsafe { crate::display::py_repr_wtf8(self.mapping()?) } } } @@ -782,10 +782,13 @@ impl FrameBox { /// `pyframe.py:259 initialize_as_generator(name, qualname)` — function /// calls pass the function's current writable metadata so each newly /// created generator freezes it independently of the code object. + /// `__name__` / `__qualname__` are the function's own strings, which may + /// carry a lone surrogate, and they are read back as values -- so they + /// arrive as WTF-8 rather than through a lossy `&str`. pub fn into_generator_named( mut self, - name: Option<&str>, - qualname: Option<&str>, + name: Option<&rustpython_wtf8::Wtf8>, + qualname: Option<&rustpython_wtf8::Wtf8>, ) -> crate::PyResult { self.fix_array_ptrs(); let register_final = code_yields_inside_try(self.code()); @@ -849,11 +852,11 @@ impl FrameBox { let _roots = pyre_object::gc_roots::push_roots(); pyre_object::gc_roots::pin_root(generator); if let Some(name) = name { - let w_name = pyre_object::w_str_new(name); + let w_name = pyre_object::w_str_from_wtf8(name.to_wtf8_buf()); unsafe { pyre_object::generator::w_generator_set_name(generator, w_name) }; } if let Some(qualname) = qualname { - let w_qualname = pyre_object::w_str_new(qualname); + let w_qualname = pyre_object::w_str_from_wtf8(qualname.to_wtf8_buf()); unsafe { pyre_object::generator::w_generator_set_qualname(generator, w_qualname) }; } unsafe { diff --git a/pyre/pyre-interpreter/src/pyopcode.rs b/pyre/pyre-interpreter/src/pyopcode.rs index 000dd8c7a87..04aba93f4ec 100644 --- a/pyre/pyre-interpreter/src/pyopcode.rs +++ b/pyre/pyre-interpreter/src/pyopcode.rs @@ -1598,7 +1598,7 @@ pub trait OpcodeStepExecutor: SharedOpcodeHandler { self.typing_intrinsic_1("_intrinsic_subscript_generic") } IntrinsicFunction1::TypeAlias => self.typing_intrinsic_1("_intrinsic_typealias"), - _ => Err(crate::PyError::type_error(&format!( + _ => Err(crate::PyError::type_error(format!( "intrinsic function {:?} not implemented", func )) @@ -1635,7 +1635,7 @@ pub trait OpcodeStepExecutor: SharedOpcodeHandler { IntrinsicFunction2::SetTypeparamDefault => { self.typing_intrinsic_2("_intrinsic_set_typeparam_default") } - _ => Err(crate::PyError::type_error(&format!( + _ => Err(crate::PyError::type_error(format!( "intrinsic function {:?} not implemented", func )) diff --git a/pyre/pyre-interpreter/src/runtime_ops.rs b/pyre/pyre-interpreter/src/runtime_ops.rs index 7265d8b4d0b..10955a20ff0 100644 --- a/pyre/pyre-interpreter/src/runtime_ops.rs +++ b/pyre/pyre-interpreter/src/runtime_ops.rs @@ -648,10 +648,10 @@ pub fn convert_value(value: PyObjectRef, conv: i64) -> Result crate::builtins::py_ascii(value)?, - _ => unsafe { crate::py_str(value)? }, + 2 => rustpython_wtf8::Wtf8Buf::from_string(crate::builtins::py_ascii(value)?), + _ => unsafe { crate::py_str_wtf8(value)? }, }; - Ok(pyre_object::w_str_new_managed(&s)) + Ok(pyre_object::w_str_from_wtf8_managed(s)) } /// FORMAT_SIMPLE / FORMAT_WITH_SPEC evaluation, shared by the interpreter @@ -1720,7 +1720,7 @@ mod tests { let err = classify_callable(w_int_new(3)).expect_err("non-callable dispatch should fail"); assert!(matches!(err.kind, PyErrorKind::TypeError)); - assert!(err.message.contains("not callable")); + assert!(err.message_text().contains("not callable")); } #[test] diff --git a/pyre/pyre-interpreter/src/type_methods.rs b/pyre/pyre-interpreter/src/type_methods.rs index ebdb27498a4..11c2b7fd5e6 100644 --- a/pyre/pyre-interpreter/src/type_methods.rs +++ b/pyre/pyre-interpreter/src/type_methods.rs @@ -2777,7 +2777,7 @@ fn format_with_spec(val: PyObjectRef, spec: &Wtf8) -> Result Result Result Result { let module = module_require(args.first().copied().unwrap_or(PY_NULL), "__repr__")?; - Ok(pyre_object::w_str_new_managed(&module_repr_string(module)?)) + Ok(pyre_object::w_str_from_wtf8_managed(module_repr_string( + module, + )?)) } fn module_require(obj: PyObjectRef, name: &str) -> Result { @@ -2746,13 +2748,14 @@ fn module_require(obj: PyObjectRef, name: &str) -> Result Result { +pub(crate) fn module_repr_string(module: PyObjectRef) -> Result { + use crate::display::wtf8_format; let importlib = crate::importing::get_sys_module("_frozen_importlib") .or_else(|| crate::importing::get_sys_module("importlib._bootstrap")); if let Some(importlib) = importlib { let repr_fn = crate::baseobjspace::getattr_str(importlib, "_module_repr")?; let result = crate::call::call_function_impl_result(repr_fn, &[module])?; - return Ok(crate::baseobjspace::text_w(result)?.to_string()); + return Ok(unsafe { pyre_object::w_str_get_wtf8(result) }.to_wtf8_buf()); } let w_dict = unsafe { pyre_object::w_module_get_w_dict(module) }; let loader = crate::baseobjspace::finditem_str(w_dict, "__loader__")?; @@ -2768,48 +2771,45 @@ pub(crate) fn module_repr_string(module: PyObjectRef) -> Result", unsafe { - crate::display::py_repr(name)? - })); + return Ok(wtf8_format!("")); } - return Ok(format!( - "", - unsafe { crate::display::py_repr(name)? }, - unsafe { crate::display::py_repr(spec_loader)? } - )); + let loader_repr = unsafe { crate::display::py_repr_wtf8(spec_loader)? }; + return Ok(wtf8_format!("")); } + let name_repr = unsafe { crate::display::py_repr_wtf8(name)? }; let has_location = crate::baseobjspace::getattr_str(spec, "has_location")?; if crate::baseobjspace::is_true(has_location)? { - return Ok(format!( - "", - unsafe { crate::display::py_repr(name)? }, - unsafe { crate::display::py_repr(origin)? } + let origin_repr = unsafe { crate::display::py_repr_wtf8(origin)? }; + return Ok(wtf8_format!( + "" )); } - return Ok(format!( - "", - unsafe { crate::display::py_repr(name)? }, - unsafe { crate::display::py_str(origin)? } - )); + let origin_str = unsafe { crate::display::py_str_wtf8(origin)? }; + return Ok(wtf8_format!("")); } } let name = crate::baseobjspace::finditem_str(w_dict, "__name__")? .unwrap_or_else(|| pyre_object::w_str_new("?")); - let name_repr = unsafe { crate::display::py_repr(name)? }; + let name_repr = unsafe { crate::display::py_repr_wtf8(name)? }; if let Some(filename) = crate::baseobjspace::finditem_str(w_dict, "__file__")? { - return Ok(format!("", unsafe { - crate::display::py_repr(filename)? - })); + let file_repr = unsafe { crate::display::py_repr_wtf8(filename)? }; + return Ok(wtf8_format!( + "" + )); } if let Some(loader) = loader { if !loader.is_null() && !unsafe { pyre_object::is_none(loader) } { - return Ok(format!("", unsafe { - crate::display::py_repr(loader)? - })); + let loader_repr = unsafe { crate::display::py_repr_wtf8(loader)? }; + return Ok(wtf8_format!("")); } } - Ok(format!("")) + Ok(wtf8_format!("")) } /// module.py:130-160 `Module.descr_getattribute`. The object-space module @@ -2842,9 +2842,9 @@ fn module_descr_dir(args: &[PyObjectRef]) -> Result crate::baseobjspace::isinstance_w(w_dict, dict_type.as_ptr()) })) { - return Err(crate::PyError::type_error(format!( - "{}.__dict__ is not a dictionary", - unsafe { crate::display::py_repr(module)? } + return Err(crate::PyError::type_error(crate::display::wtf8_format!( + unsafe { crate::display::py_repr_wtf8(module)? }, + ".__dict__ is not a dictionary" ))); } if let Some(w_dir) = crate::baseobjspace::finditem_str(w_dict, "__dir__")? { @@ -9070,9 +9070,10 @@ fn union_mro_entries_method(args: &[PyObjectRef]) -> crate::PyResult { "descriptor '__mro_entries__' requires a 'types.UnionType' object", )); } - let rendered = unsafe { crate::display::py_repr(self_)? }; - Err(crate::PyError::type_error(format!( - "Cannot subclass {rendered}" + let rendered = unsafe { crate::display::py_repr_wtf8(self_)? }; + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "Cannot subclass ", + rendered ))) } @@ -9329,12 +9330,13 @@ fn init_union_type(ns: PyObjectRef) { |args| { let self_ = args.first().copied().unwrap_or(PY_NULL); let rendered = if self_.is_null() { - "typing.Union".to_string() + Wtf8Buf::from_string("typing.Union".to_string()) } else { - unsafe { crate::py_repr(self_) }? + unsafe { crate::display::py_repr_wtf8(self_) }? }; - Err(crate::PyError::type_error(format!( - "Cannot subclass {rendered}" + Err(crate::PyError::type_error(crate::display::wtf8_format!( + "Cannot subclass ", + rendered ))) }, 2, @@ -12650,10 +12652,12 @@ fn init_function_type(ns: PyObjectRef) { args.first().copied().unwrap_or(pyre_object::PY_NULL), "__repr__", )?; - let qualname = unsafe { crate::function::function_get_qualname(function) }; - Ok(pyre_object::w_str_new(&format!( - "" - ))) + // `format!` renders the WTF-8 qualname through `Display`, + // which substitutes U+FFFD for a lone surrogate. + let mut repr = Wtf8Buf::from_string("")); + Ok(pyre_object::w_str_from_wtf8(repr)) }, 1, ), @@ -12911,23 +12915,29 @@ fn builtin_function_qualname(obj: PyObjectRef) -> crate::PyResult { // (`stamp_method_owners`), which is also what `bool.from_bytes` // reporting `int.from_bytes` requires. if unsafe { pyre_object::is_type(instance) } { - return Ok(pyre_object::w_str_new(&unsafe { + return Ok(pyre_object::w_str_from_wtf8(unsafe { crate::function::function_get_qualname(descr) })); } let actual_type = crate::typedef::r#type(instance).map_or(pyre_object::PY_NULL, |tp| tp.as_ptr()); let type_qualname = crate::baseobjspace::getattr_str(actual_type, "__qualname__")?; - let Some(type_qualname) = (unsafe { pyre_object::w_str_get_value_opt(type_qualname) }) - else { + if !unsafe { pyre_object::is_str(type_qualname) } { return Err(crate::PyError::type_error( ".__class__.__qualname__ is not a unicode object", )); - }; + } + // A class named through `type()` may carry a lone surrogate in its + // qualname, which is a `str` the read has to keep rather than reject. + let type_qualname = unsafe { pyre_object::w_str_get_wtf8(type_qualname) }; let name = unsafe { crate::function::function_get_name(descr) }; - Ok(pyre_object::w_str_new(&format!("{type_qualname}.{name}"))) + Ok(pyre_object::w_str_from_wtf8(crate::display::wtf8_format!( + type_qualname, + ".", + name, + ))) } else { - Ok(pyre_object::w_str_new(&unsafe { + Ok(pyre_object::w_str_from_wtf8(unsafe { crate::function::function_get_qualname(obj) })) } @@ -15293,11 +15303,15 @@ fn staticmethod_descr_repr(args: &[PyObjectRef]) -> crate::PyResult { let sm = staticmethod_require(args.first().copied().unwrap_or(PY_NULL), "__repr__")?; let function = unsafe { pyre_object::function::w_staticmethod_get_func(sm) }; let repr = if function.is_null() { - "".to_string() + Wtf8Buf::from_string("".to_string()) } else { - unsafe { crate::display::py_repr(function)? } + unsafe { crate::display::py_repr_wtf8(function)? } }; - Ok(w_str_new_managed(&format!(""))) + Ok(w_str_from_wtf8_managed(crate::display::wtf8_format!( + "" + ))) } /// function.py:708-709 `StaticMethod.descr_reduce_ex`. @@ -15570,11 +15584,15 @@ fn classmethod_descr_repr(args: &[PyObjectRef]) -> crate::PyResult { let cm = classmethod_require(args.first().copied().unwrap_or(PY_NULL), "__repr__")?; let function = unsafe { pyre_object::function::w_classmethod_get_func(cm) }; let repr = if function.is_null() { - "".to_string() + Wtf8Buf::from_string("".to_string()) } else { - unsafe { crate::display::py_repr(function)? } + unsafe { crate::display::py_repr_wtf8(function)? } }; - Ok(w_str_new_managed(&format!(""))) + Ok(w_str_from_wtf8_managed(crate::display::wtf8_format!( + "" + ))) } /// function.py:763-764 `ClassMethod.descr_reduce_ex`. @@ -19105,7 +19123,7 @@ fn bytearray_descr_init_value( // pos[0] is the class; `bytearray(source, encoding, errors)` accepts at // most three further positional arguments. if pos.len() > 4 { - return Err(crate::PyError::type_error(&format!( + return Err(crate::PyError::type_error(format!( "bytearray() takes at most 3 arguments ({} given)", pos.len() - 1 ))); @@ -21648,6 +21666,30 @@ fn unicode_encode_error_msg( } } +/// `str.encode('utf-8')` under the default error handler, for a consumer that +/// can only hold a Rust `str` — the compiler's source text, the tokenizer's +/// input line. +/// +/// `pyparse.py:9-15 recode_to_utf8` takes this same encode, and it is strict: +/// a lone surrogate has no UTF-8 spelling, so the text is reported rather than +/// silently rewritten with U+FFFD. +pub(crate) fn utf8_strict_w(text: Wtf8Buf) -> Result { + if let Ok(s) = text.as_str() { + return Ok(s.to_owned()); + } + let position = text + .code_points() + .position(|c| c.to_char().is_none()) + .unwrap_or(0); + Err(unicode_encode_error( + "utf-8", + pyre_object::w_str_from_wtf8(text), + position, + position + 1, + "surrogates not allowed", + )) +} + /// unicodehelper.py encode_error_handler — raises a structured /// UnicodeEncodeError, mirroring `OperationError(space.w_UnicodeEncodeError, /// space.newtuple([encoding, w_obj, start, end, msg]))`. Populates the @@ -22306,7 +22348,7 @@ fn bytes_descr_new_impl(args: &[PyObjectRef]) -> Result 4 { - return Err(crate::PyError::type_error(&format!( + return Err(crate::PyError::type_error(format!( "bytes() takes at most 3 arguments ({} given)", pos.len() - 1 ))); @@ -26429,14 +26471,14 @@ fn count_descr_repr(args: &[PyObjectRef]) -> Result let cls_name = full_name.rsplit('.').next().unwrap_or(full_name); let w_c = unsafe { pyre_object::interp_itertools::w_count_get_c(obj) }; let w_step = unsafe { pyre_object::interp_itertools::w_count_get_step(obj) }; - let c = unsafe { crate::display::py_repr(w_c)? }; + let c = unsafe { crate::display::py_repr_wtf8(w_c)? }; let text = if count_single_argument(w_step)? { - format!("{cls_name}({c})") + crate::display::wtf8_format!(cls_name, "(", c, ")") } else { - let step = unsafe { crate::display::py_repr(w_step)? }; - format!("{cls_name}({c}, {step})") + let step = unsafe { crate::display::py_repr_wtf8(w_step)? }; + crate::display::wtf8_format!(cls_name, "(", c, ", ", step, ")") }; - Ok(w_str_new(&text)) + Ok(w_str_from_wtf8(text)) } fn init_count_type(ns: PyObjectRef) { @@ -26505,14 +26547,14 @@ fn repeat_descr_repr(args: &[PyObjectRef]) -> Result( // and rooted by the compiled loop's gcref table thereafter. let _roots = pyre_object::gc_roots::push_roots(); pyre_object::gc_roots::pin_root(exc); - let msg = pyre_object::w_str_new(&err.message); + let msg = pyre_object::w_str_from_wtf8(err.message.clone()); let msg_const = ctx.trace_ctx.const_ref(msg as i64); let args_list = crate::helpers::emit_object_list_inline(ctx.trace_ctx, &[msg_const]); // Stamp the canonical list class exactly as `w_list_new` does (the diff --git a/pyre/pyre-jit/tests/gc_stress.rs b/pyre/pyre-jit/tests/gc_stress.rs index 6ff2b3c8f31..d4006adcc5b 100644 --- a/pyre/pyre-jit/tests/gc_stress.rs +++ b/pyre/pyre-jit/tests/gc_stress.rs @@ -50,7 +50,7 @@ fn run_harness(program: &str, name: &str, vacuity_label: &str) -> Result<(), Str reset_gc_fresh_for_test(); let cwd = std::env::current_dir().map_err(|e| e.to_string())?; - importing::init_sys_path(&cwd, &cwd.to_string_lossy()); + importing::init_sys_path(&cwd, cwd.as_os_str()); // This harness never imports `site`, so perform the post-site `sys.path[0]` // insert directly. importing::add_sys_path_0(); diff --git a/pyre/pyre-macros/src/lib.rs b/pyre/pyre-macros/src/lib.rs index 07afdd187b0..15880c6c609 100644 --- a/pyre/pyre-macros/src/lib.rs +++ b/pyre/pyre-macros/src/lib.rs @@ -933,6 +933,10 @@ fn wrap_value_expr( "f64" => return Ok(quote! { ::pyre_object::w_float_new(#value) }), "bool" => return Ok(quote! { ::pyre_object::w_bool_from(#value) }), "String" => return Ok(quote! { ::pyre_object::w_str_new(&#value) }), + // A method whose text may hold a lone surrogate — a `__repr__` + // naming a filename, say — returns the WTF-8 buffer itself + // rather than a `String` it cannot spell. + "Wtf8Buf" => return Ok(quote! { ::pyre_object::w_str_from_wtf8(#value) }), _ => {} } // `Vec` — bytes / list-of-X. diff --git a/pyre/pyrex/Cargo.toml b/pyre/pyrex/Cargo.toml index 921f0c0c4a0..f59b5c773b0 100644 --- a/pyre/pyrex/Cargo.toml +++ b/pyre/pyrex/Cargo.toml @@ -55,6 +55,7 @@ majit-metainterp = { workspace = true } majit-gc = { workspace = true } lexopt = { workspace = true } rustpython-compiler = { workspace = true } +rustpython-wtf8 = { workspace = true } dirs = { workspace = true } rustyline = { workspace = true } libc = { workspace = true } diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index 2db4a083367..14f395d2b97 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -576,7 +576,7 @@ fn real_main(binary_name: &str) { // cwd is still the anchor the shadowing check compares module // origins against. let cwd = sys_path_cwd(); - importing::init_sys_path(&cwd, ""); + importing::init_sys_path(&cwd, std::ffi::OsStr::new("")); let mut argv = vec![std::ffi::OsString::from("-c")]; argv.extend(args); importing::set_sys_argv(&argv); @@ -589,7 +589,7 @@ fn real_main(binary_name: &str) { // `-m`: sys.path[0] is the cwd (runpy resets argv[0] to the // module's resolved origin via `_run_module_as_main`). let cwd = sys_path_cwd(); - importing::init_sys_path(&cwd, &cwd.to_string_lossy()); + importing::init_sys_path(&cwd, cwd.as_os_str()); let mut argv = vec![std::ffi::OsString::from(&module)]; argv.extend(args); importing::set_sys_argv(&argv); @@ -604,7 +604,11 @@ fn real_main(binary_name: &str) { // the same channel module imports use. The bytes then go through // the tokenizer's BOM / PEP 263 decoding in every build. let source = match importing::read_source_bytes(Path::new(&path)) { - Ok(bytes) => match pyre_interpreter::decode_source_bytes(&bytes, &path, false) { + Ok(bytes) => match pyre_interpreter::decode_source_bytes( + &bytes, + rustpython_wtf8::Wtf8::new(path.as_str()), + false, + ) { Ok(source) => source, Err(error) => { pyre_interpreter::eprint_exception(&error, false); @@ -639,7 +643,7 @@ fn real_main(binary_name: &str) { }; parent.canonicalize().unwrap_or_else(|_| sys_path_cwd()) }; - importing::init_sys_path(&script_dir, &script_dir.to_string_lossy()); + importing::init_sys_path(&script_dir, script_dir.as_os_str()); // sys.argv[0] is the script path; remaining values go to argv[1:]. let mut argv = vec![std::ffi::OsString::from(&path)]; argv.extend(args); @@ -664,7 +668,7 @@ fn real_main(binary_name: &str) { // (`_PyPathConfig_ComputeSysPath0` on an argv[0] of "" / "-"); the // cwd stays the shadowing-check anchor. let cwd = sys_path_cwd(); - importing::init_sys_path(&cwd, ""); + importing::init_sys_path(&cwd, std::ffi::OsStr::new("")); // `sys.argv` is `['']` with no script argument and `['-', …]` for // an explicit dash. let mut argv = vec![std::ffi::OsString::from(argv0)]; @@ -1095,7 +1099,7 @@ fn run_atexit_callbacks(canonical: pyre_object::PyObjectRef, ec_ptr: *const PyEx if let Err(mut error) = result { error.write_unraisable( pyre_object::w_none(), - "_run_exitfuncs", + rustpython_wtf8::Wtf8::new("_run_exitfuncs"), pyre_object::w_none(), ); } diff --git a/pyre/pyrex/src/repl.rs b/pyre/pyrex/src/repl.rs index 9a927a36fe3..8337b033a97 100644 --- a/pyre/pyrex/src/repl.rs +++ b/pyre/pyrex/src/repl.rs @@ -240,7 +240,7 @@ fn read_prompt(sys_module: pyre_object::PyObjectRef, name: &str) -> Option Date: Fri, 7 Aug 2026 21:56:40 +0900 Subject: [PATCH 10/16] posix: encode a Windows path for AddDllDirectory with encode_wide `arg_path` returns an `OsString`; `encode_utf16` is inherent to `str`, so `_add_dll_directory` did not compile for windows-msvc (E0599). `encode_wide` re-emits the code units `arg_path` decoded the path into. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/posix/interp_posix.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index a95f4fbe253..005014e011b 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -699,9 +699,13 @@ mod win_nt { /// DLL_DIRECTORY_COOKIE pointer is returned as an int instead. host_env has /// no AddDllDirectory wrapper, so call windows-sys directly. pub fn _add_dll_directory(args: &[PyObjectRef]) -> Result { + use std::os::windows::ffi::OsStrExt; use windows_sys::Win32::System::LibraryLoader::AddDllDirectory; let (path, _, resolved) = arg_path(args, "_add_dll_directory")?; - let wide: Vec = path.encode_utf16().chain(std::iter::once(0)).collect(); + // `encode_wide` re-emits the code units `arg_path` decoded the path + // into; going back through a `str` would have no spelling for a lone + // surrogate and would address a different directory. + let wide: Vec = path.encode_wide().chain(std::iter::once(0)).collect(); let cookie = unsafe { AddDllDirectory(wide.as_ptr()) }; if cookie.is_null() { return Err(io_err_with_filename( From cc216319ac559da4a5dad0ee2977e5f41609ce48 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 21:56:49 +0900 Subject: [PATCH 11/16] error: write a report that is not WTF-8 to the fd unchanged A frame's `co_filename` is written as the filesystem bytes it was read as, so a path byte with no UTF-8 spelling leaves the report a mix of those bytes and the WTF-8 around them. Reading that mix back through `String::from_utf8_lossy` substituted U+FFFD for the byte that carries the name; pass the buffer through instead. The valid-WTF-8 arm still spends the backslashreplace encode. Assisted-by: Claude --- pyre/pyre-interpreter/src/error.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pyre/pyre-interpreter/src/error.rs b/pyre/pyre-interpreter/src/error.rs index 76c6ae772a7..e9f89d2f69e 100644 --- a/pyre/pyre-interpreter/src/error.rs +++ b/pyre/pyre-interpreter/src/error.rs @@ -3137,12 +3137,20 @@ fn read_source_line(filename: &[u8], lineno: i64) -> Option { /// so a caller writing to one owes that encode itself -- putting the WTF-8 /// bytes straight on the stream would emit a sequence that is not valid UTF-8. pub(crate) fn emit_report_to_host_stderr(buf: &[u8]) { - let text = match rustpython_wtf8::Wtf8::from_bytes(buf) { - Some(report) => crate::display::wtf8_display_string(report.to_wtf8_buf(), ""), - // A byte no writer put there; the lossy read is the last resort for it. - None => String::from_utf8_lossy(buf).into_owned(), - }; - crate::host_seam::emit_stderr(text.as_bytes()); + match rustpython_wtf8::Wtf8::from_bytes(buf) { + Some(report) => { + let text = crate::display::wtf8_display_string(report.to_wtf8_buf(), ""); + crate::host_seam::emit_stderr(text.as_bytes()); + } + // A frame's `co_filename` goes out as the filesystem bytes it was read + // as, so a path byte with no UTF-8 spelling leaves the report a mix of + // those bytes and the WTF-8 around them, which is not a WTF-8 buffer + // and has no encode to spend. Pass it through: the byte form is what + // still names the file to whatever reads the stream, and re-reading it + // as UTF-8 would substitute U+FFFD for the one byte that carries the + // name. + None => crate::host_seam::emit_stderr(buf), + } } pub fn eprint_exception(err: &PyError, include_traceback: bool) { From 463c905692028cc38e25e4c06df33857e82f7822 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 21:56:49 +0900 Subject: [PATCH 12/16] interp: restore the type check on two str reads `__pypy__.write_unraisable`'s first argument and the value `_module_repr` returns were read with `w_str_get_wtf8`, which casts without a tag test, so a non-str was dereferenced as a `W_UnicodeObject` instead of raising TypeError. `text_wtf8_w` runs `expect_str` first and performs the same WTF-8 read. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/__pypy__/mod.rs | 2 +- pyre/pyre-interpreter/src/typedef.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyre/pyre-interpreter/src/module/__pypy__/mod.rs b/pyre/pyre-interpreter/src/module/__pypy__/mod.rs index 63245968343..851303685d8 100644 --- a/pyre/pyre-interpreter/src/module/__pypy__/mod.rs +++ b/pyre/pyre-interpreter/src/module/__pypy__/mod.rs @@ -159,7 +159,7 @@ const CANONICAL_IDENTITY_DICT_KEY: &str = "@objects_in_repr_identity_dict"; /// `interp_magic.py:280-290 write_unraisable` — turn the supplied exception /// value back into an OperationError and report it through `sys.unraisablehook`. fn write_unraisable(args: &[pyre_object::PyObjectRef]) -> crate::PyResult { - let where_desc = unsafe { pyre_object::w_str_get_wtf8(args[0]) }.to_wtf8_buf(); + let where_desc = crate::baseobjspace::text_wtf8_w(args[0])?.to_wtf8_buf(); // `OperationError(space.type(w_exc), w_exc)` accepts any object, so the // exception tag cannot be read unconditionally: it lives past the header // of a `W_BaseException`, and a plain instance is smaller than that. diff --git a/pyre/pyre-interpreter/src/typedef.rs b/pyre/pyre-interpreter/src/typedef.rs index 2265ea32f85..285ddf4bddc 100644 --- a/pyre/pyre-interpreter/src/typedef.rs +++ b/pyre/pyre-interpreter/src/typedef.rs @@ -2755,7 +2755,7 @@ pub(crate) fn module_repr_string(module: PyObjectRef) -> Result Date: Fri, 7 Aug 2026 21:56:57 +0900 Subject: [PATCH 13/16] call, builtins: assemble five messages holding a qualname as WTF-8 Four keyword-binding TypeErrors and `fileio.__repr__` interpolated a `Wtf8Buf` through `format!`, which renders it with `Display` and substitutes U+FFFD, so the same call family spelled a `__qualname__` two different ways depending on which arm raised. The parity fixture gains the two binder arms a defaulted parameter reaches. Assisted-by: Claude --- .../parity_tests/surrogate_name_messages.py | 22 ++++++++++++ pyre/pyre-interpreter/src/builtins.rs | 4 ++- pyre/pyre-interpreter/src/call.rs | 36 ++++++++++--------- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/pyre/extra_tests/parity_tests/surrogate_name_messages.py b/pyre/extra_tests/parity_tests/surrogate_name_messages.py index ad2bb08f09b..adf24082ae1 100644 --- a/pyre/extra_tests/parity_tests/surrogate_name_messages.py +++ b/pyre/extra_tests/parity_tests/surrogate_name_messages.py @@ -52,6 +52,28 @@ def f(a): else: raise AssertionError("no TypeError") + # A defaulted parameter takes the binder down its other arm, which builds + # the same two messages from the same name. + def g(a, b=1): + return a, b + + g.__qualname__ = S + + try: + g(1, 2, 3) + except TypeError as e: + expected = S + "() takes from 1 to 2 positional arguments but 3 were given" + assert str(e) == expected, ascii(str(e)) + else: + raise AssertionError("no TypeError") + + try: + g(1, a=2) + except TypeError as e: + assert str(e) == S + "() got multiple values for argument 'a'", ascii(str(e)) + else: + raise AssertionError("no TypeError") + def check_generator_qualname(): def g(): diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 9331bc71186..6ffe5240e06 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -13989,7 +13989,9 @@ fn fileio_method_repr(args: &[PyObjectRef]) -> Result"))) + Ok(pyre_object::w_str_from_wtf8( + crate::display::wtf8_format!(format!("<{repr_type} "), body, ">"), + )) } /// `_io.FileIO.__init__` — PyPy `W_FileIO.descr_init`. diff --git a/pyre/pyre-interpreter/src/call.rs b/pyre/pyre-interpreter/src/call.rs index d696d6a0809..0099deef775 100644 --- a/pyre/pyre-interpreter/src/call.rs +++ b/pyre/pyre-interpreter/src/call.rs @@ -1975,10 +1975,12 @@ pub(crate) fn resolve_kwargs( } // argument.py:410 — duplicate keyword argument if !result[pi].is_null() { - return Err(crate::PyError::type_error(format!( - "{}() got multiple values for argument '{}'", - fname, param_name - ))); + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(&fname); + msg.push_str(&format!( + "() got multiple values for argument '{param_name}'" + )); + return Err(crate::PyError::type_error(msg)); } result[pi] = kw_value; matched = true; @@ -2071,10 +2073,10 @@ pub(crate) fn resolve_kwargs( if n_pos != 1 { "were" } else { "was" } ) }; - return Err(crate::PyError::type_error(format!( - "{}() takes {} but {}", - fname, takes_str, given_str - ))); + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(&fname); + msg.push_str(&format!("() takes {takes_str} but {given_str}")); + return Err(crate::PyError::type_error(msg)); } // Fill positional defaults (PyPy: _match_signature defs_w) @@ -2717,10 +2719,12 @@ pub fn call_with_kwargs_in_ctx( // argument.py:495 — ArgErrMultipleValues: keyword // duplicates an already-bound positional argument. if !result[pi].is_null() { - return Err(crate::PyError::type_error(format!( - "{}() got multiple values for argument '{}'", - fname, key - ))); + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(&fname); + msg.push_str(&format!( + "() got multiple values for argument '{key}'" + )); + return Err(crate::PyError::type_error(msg)); } result[pi] = value; matched = true; @@ -2779,10 +2783,10 @@ pub fn call_with_kwargs_in_ctx( pos_args.len(), if pos_args.len() != 1 { "were" } else { "was" } ); - return Err(crate::PyError::type_error(format!( - "{}() takes {} but {} given", - fname, takes_str, given_str - ))); + let mut msg = Wtf8Buf::new(); + msg.push_wtf8(&fname); + msg.push_str(&format!("() takes {takes_str} but {given_str} given")); + return Err(crate::PyError::type_error(msg)); } // Fill positional defaults from __defaults__ tuple. From b10583b84d19ca257af3e28b4f8792c670714609 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 7 Aug 2026 21:56:58 +0900 Subject: [PATCH 14/16] repl: let a raising sys.ps1 fall back to the default prompt `read_prompt` took `py_str_display`, which answers `""` rather than failing, so a raising `__str__` became the prompt itself instead of leaving `load_prompt`'s default to stand in. `py_str_display_result` renders the same text and reports the failure. Assisted-by: Claude --- pyre/pyre-interpreter/src/display.rs | 14 ++++++++++++++ pyre/pyrex/src/repl.rs | 5 ++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pyre/pyre-interpreter/src/display.rs b/pyre/pyre-interpreter/src/display.rs index 045011dca6c..434eb14f56a 100644 --- a/pyre/pyre-interpreter/src/display.rs +++ b/pyre/pyre-interpreter/src/display.rs @@ -1362,6 +1362,20 @@ pub unsafe fn py_str_display(obj: PyObjectRef) -> String { } } +/// `str(obj)` rendered for a terminal, like [`py_str_display`], but a raising +/// `__str__` is reported to the caller instead of degrading to a placeholder. +/// +/// For text that is a value the user supplied rather than a diagnostic about +/// one -- a prompt, say -- `""` is the wrong answer: the caller +/// has its own fallback and needs to know the read failed to reach it. +/// +/// # Safety +/// `obj` must be a valid object. +pub unsafe fn py_str_display_result(obj: PyObjectRef) -> Result { + let rendered = unsafe { py_str_wtf8(obj) }?; + Ok(wtf8_display_string(rendered, "")) +} + /// The text a WTF-8 diagnostic becomes on the way to stderr. /// /// `sys.stderr` carries `errors='backslashreplace'`, so an unpaired surrogate diff --git a/pyre/pyrex/src/repl.rs b/pyre/pyrex/src/repl.rs index 8337b033a97..c420c989fb8 100644 --- a/pyre/pyrex/src/repl.rs +++ b/pyre/pyrex/src/repl.rs @@ -240,7 +240,10 @@ fn read_prompt(sys_module: pyre_object::PyObjectRef, name: &str) -> Option Date: Fri, 7 Aug 2026 22:15:42 +0900 Subject: [PATCH 15/16] _json: carry a serialization note's key repr as WTF-8 The note names a dict key, which may hold a lone surrogate. It was built from `display::py_repr`, which this branch removed along with the `to_string_lossy` behind it, so the call site did not compile; `add_json_note` now takes the text as WTF-8 and the key is read with `py_repr_wtf8`. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/_json/mod.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pyre/pyre-interpreter/src/module/_json/mod.rs b/pyre/pyre-interpreter/src/module/_json/mod.rs index 750181bf416..56d096025ed 100644 --- a/pyre/pyre-interpreter/src/module/_json/mod.rs +++ b/pyre/pyre-interpreter/src/module/_json/mod.rs @@ -550,12 +550,14 @@ fn encode_float( /// Attach the PEP 678 context notes emitted by Python 3.14's /// `json.encoder._make_iterencode`. The original exception remains /// authoritative if a pathological `add_note` override itself fails. -fn add_json_note(mut err: PyError, note: String) -> PyError { +fn add_json_note(mut err: PyError, note: impl Into) -> PyError { let _roots = gc_roots::push_roots(); let exc = err.to_exc_object(); let exc_slot = gc_roots::shadow_stack_len(); gc_roots::pin_root(exc); - let note = pyre_object::w_str_new(¬e); + // The note quotes a key the caller supplied, which may hold a lone + // surrogate, so it is carried as the WTF-8 it is. + let note = pyre_object::w_str_from_wtf8(note.into()); let note_slot = gc_roots::shadow_stack_len(); gc_roots::pin_root(note); if let Ok(add_note) = @@ -889,11 +891,14 @@ fn encode_dict( ) .map_err(|err| { let key_repr = - unsafe { crate::display::py_repr(gc_roots::shadow_stack_get(pair_slot + 2)) } - .unwrap_or_else(|_| "".to_owned()); + unsafe { crate::display::py_repr_wtf8(gc_roots::shadow_stack_get(pair_slot + 2)) } + .unwrap_or_else(|_| rustpython_wtf8::Wtf8Buf::from_string("".to_owned())); add_json_note( err, - format!("when serializing {} item {key_repr}", short_type_name(obj)), + crate::display::wtf8_format!( + format!("when serializing {} item ", short_type_name(obj)), + key_repr + ), ) })?; out.push_wtf8(&encoded); From 96c86bce741598ba8a8ecf673b567b6adf660f4c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 8 Aug 2026 06:44:22 +0900 Subject: [PATCH 16/16] style: rustfmt two WTF-8 message assemblers `fileio_method_repr`'s `wtf8_format!` call and the multiple-values `push_str` in `call_with_kwargs_in_ctx` were left in a shape `cargo fmt --check` rejects. Assisted-by: Claude --- pyre/pyre-interpreter/src/builtins.rs | 8 +++++--- pyre/pyre-interpreter/src/call.rs | 4 +--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 6ffe5240e06..2e6c2138200 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -13989,9 +13989,11 @@ fn fileio_method_repr(args: &[PyObjectRef]) -> Result"), - )) + Ok(pyre_object::w_str_from_wtf8(crate::display::wtf8_format!( + format!("<{repr_type} "), + body, + ">" + ))) } /// `_io.FileIO.__init__` — PyPy `W_FileIO.descr_init`. diff --git a/pyre/pyre-interpreter/src/call.rs b/pyre/pyre-interpreter/src/call.rs index 0099deef775..a1ba097539a 100644 --- a/pyre/pyre-interpreter/src/call.rs +++ b/pyre/pyre-interpreter/src/call.rs @@ -2721,9 +2721,7 @@ pub fn call_with_kwargs_in_ctx( if !result[pi].is_null() { let mut msg = Wtf8Buf::new(); msg.push_wtf8(&fname); - msg.push_str(&format!( - "() got multiple values for argument '{key}'" - )); + msg.push_str(&format!("() got multiple values for argument '{key}'")); return Err(crate::PyError::type_error(msg)); } result[pi] = value;