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/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/optimizeopt/virtualize.rs b/majit/majit-metainterp/src/optimizeopt/virtualize.rs index 118c61f4bfc..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 @@ -983,6 +1068,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 @@ -2410,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, @@ -2423,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 {}", @@ -2452,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 { 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/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/extra_tests/parity_tests/surrogate_name_messages.py b/pyre/extra_tests/parity_tests/surrogate_name_messages.py new file mode 100644 index 00000000000..adf24082ae1 --- /dev/null +++ b/pyre/extra_tests/parity_tests/surrogate_name_messages.py @@ -0,0 +1,139 @@ +"""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") + + # 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(): + 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/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/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/_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 988f023cc76..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" ))); } } @@ -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); } @@ -6636,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); @@ -6670,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)) } @@ -8626,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, ); } @@ -13663,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` @@ -13672,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 @@ -13723,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", @@ -16671,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)) } @@ -18000,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()) } }; @@ -18261,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 @@ -18283,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 @@ -18294,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 @@ -18348,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 @@ -18359,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 @@ -18449,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 df5e67614be..2e6c2138200 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 @@ -8153,8 +8169,8 @@ fn exception_group_new(args: &[PyObjectRef]) -> Result Result Result { @@ -8645,11 +8666,16 @@ fn exception_group_repr(args: &[PyObjectRef]) -> Result Result 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 )) } @@ -9701,9 +9734,10 @@ pub(crate) fn builtin_float(args: &[PyObjectRef]) -> Result Result Result { unsafe { @@ -10895,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 @@ -10984,7 +11020,7 @@ fn builtin_compile(args: &[PyObjectRef]) -> Result source, "compile", "string, bytes or AST", - &filename, + &filename_text, &mut flags, ) .map_err(|error| { @@ -11134,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 { @@ -12041,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 ))); @@ -13942,16 +13978,22 @@ 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`. @@ -16846,7 +16888,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" ); } @@ -17116,6 +17158,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..a1ba097539a 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` @@ -1960,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; @@ -2056,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) @@ -2290,7 +2307,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 +2316,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)); } @@ -2699,10 +2719,10 @@ 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; @@ -2761,10 +2781,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. @@ -3127,7 +3147,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 +4752,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 6f1a0bc5821..434eb14f56a 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 @@ -1306,17 +1358,43 @@ 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, "") } } +/// `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 +/// 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 @@ -1515,14 +1593,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`, @@ -1534,7 +1612,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()) } } @@ -1561,9 +1639,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(), } } @@ -1572,9 +1650,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(), } } @@ -1672,19 +1750,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) } @@ -1760,21 +1843,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) } @@ -1882,8 +1967,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 1b8e121127f..e9f89d2f69e 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_wtf8(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_wtf8(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,20 +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 { - 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()) -} - 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) } { @@ -2423,7 +2460,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 +2657,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 +2691,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 => { @@ -3052,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) @@ -3072,13 +3127,39 @@ 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]) { + 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) { // 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 @@ -3113,8 +3194,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..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 = crate::baseobjspace::str_utf8_w(args[0])?; + 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/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/_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); 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 0570dcd27e9..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(), ); } @@ -1514,7 +1514,9 @@ pub fn proxy_trunc(args: &[PyObjectRef]) -> 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 { @@ -2255,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 1b2cb064bfc..005014e011b 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)) { @@ -666,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( @@ -770,10 +807,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 +2662,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, @@ -4396,10 +4438,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() @@ -4717,7 +4761,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( @@ -4807,7 +4854,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", @@ -7058,7 +7107,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, ), @@ -8616,6 +8665,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. @@ -8658,7 +8708,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, @@ -8666,4 +8721,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 894f3604dd3..11c2b7fd5e6 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 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(crate::baseobjspace::text_wtf8_w(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__")? { @@ -7963,9 +7963,9 @@ fn init_mappingproxy_type(ns: PyObjectRef) { return Ok(pyre_object::w_str_new("")); } unsafe { - Ok(pyre_object::w_str_new_managed(&crate::display::py_str( - args[0], - )?)) + Ok(pyre_object::w_str_from_wtf8_managed( + crate::display::py_str_wtf8(args[0])?, + )) } }), ) @@ -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`. @@ -18748,8 +18766,8 @@ fn init_object_type(ns: PyObjectRef) { crate::type_methods::arg_type_name(args[0]) ))); } - Ok(pyre_object::w_str_new_managed(&unsafe { - crate::py_str(args[0])? + Ok(pyre_object::w_str_from_wtf8_managed(unsafe { + crate::display::py_str_wtf8(args[0])? })) }, 2, @@ -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( .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 diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 6edb08b8a3e..5dbf53161ce 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -10531,7 +10531,7 @@ pub(crate) fn try_walker_trace_immutable_type_attr_raise( // 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/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 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..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