Skip to content

Commit 2d5ac60

Browse files
emerybergerclaude
andcommitted
Fix UnicodeEncodeError for non-ASCII file paths (#1086)
pywhere.cpp converted code->co_filename with PyUnicode_AsASCIIString on three paths: the stack walker used by the allocator interposer, on_stack(), and the settrace line callback. For any path containing a non-ASCII character -- e.g. a module at .../überschüsse.py -- that call fails, returning NULL *and leaving a UnicodeEncodeError set on the thread*. Since these run from the native allocator hook and from trace callbacks, the stray exception surfaced later at an arbitrary, unrelated Python line, which is why the reporter saw it land in sysconfig.get_path on one run and inside pydantic on the next. Two of the sites also fed the NULL straight into PyBytes_AsString/strstr, giving the reported SIGSEGV. Replace all three with a new encodeFilename() helper that encodes as UTF-8 with surrogate escapes -- the exact inverse of how CPython decodes filesystem paths, so every path Python can represent (including undecodable bytes, which appear as lone surrogates) round-trips. It clears any exception on failure and returns nullptr; every call site now NULL-checks. Also drops a dead encode-and-discard in trace_func, and fixes the same pending-exception leak in TraceConfig's package-path encode, which used "strict". The Python side compounded the problem by decoding native sample records as ASCII: add decode_sample() in scalene_mapfile.py using the matching UTF-8/surrogateescape pair. Verified by reproducing the crash (3/3 runs) on the unfixed build, then confirming a complete profile with the fix, on both --memory and --memory --stacks. tests/test_issue1086_non_ascii_paths.py is picked up by the existing pytest step in tests.yml, so it runs on Ubuntu + macOS across Python 3.9-3.14. It covers decode_sample directly (deterministic, including lone surrogates) and profiles a program importing a non-ASCII module end to end, with the non-ASCII component both in the filename and in a parent directory. The crash assertions are hard on every attempt; only the "file appears in the profile" check retries against sampling flake. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c2e9e17 commit 2d5ac60

4 files changed

Lines changed: 238 additions & 12 deletions

File tree

scalene/scalene_mapfile.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@
99
Filename = NewType("Filename", str)
1010

1111

12+
def decode_sample(raw: bytes) -> str:
13+
"""Decode one sample record written by the native side.
14+
15+
Records embed source filenames, which the native side encodes as UTF-8
16+
with surrogate escapes (see ``encodeFilename`` in
17+
``src/source/pywhere.cpp``). Decoding the same way is the exact inverse,
18+
so paths with non-ASCII characters -- and even paths with undecodable
19+
bytes, which CPython represents as lone surrogates -- round-trip.
20+
21+
This used to decode as ASCII, which raised ``UnicodeDecodeError`` and
22+
took down the sample-processing loop for any allocation attributed to a
23+
file whose path contained e.g. an umlaut (issue #1086).
24+
"""
25+
return raw.decode("utf-8", errors="surrogateescape")
26+
27+
1228
class ScaleneMapFile:
1329

1430
# Things that need to be in sync with the C++ side
@@ -249,7 +265,7 @@ def _read_windows(self) -> bool:
249265
self._buf[length:] = b"\x00" * (self.MAX_BUFSIZE - length)
250266

251267
# Validate the sample has expected format before accepting
252-
sample_preview = self._buf[:100].decode("ascii", errors="replace")
268+
sample_preview = self._buf[:100].decode("utf-8", errors="replace")
253269
if "," not in sample_preview:
254270
# Malformed sample - skip it but advance position
255271
self._lastpos = struct.pack("<Q", end_pos) # type: ignore[assignment]
@@ -264,4 +280,4 @@ def _read_windows(self) -> bool:
264280

265281
def get_str(self) -> str:
266282
"""Get the string from the buffer."""
267-
return self._buf.rstrip(b"\x00").split(b"\n")[0].decode("ascii")
283+
return decode_sample(bytes(self._buf.rstrip(b"\x00").split(b"\n")[0]))

src/include/traceconfig.hpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,17 @@ class TraceConfig {
7474
if (scalene_pkg_path_obj && scalene_pkg_path_obj != Py_None) {
7575
scalene_pkg_path_owner = scalene_pkg_path_obj;
7676
Py_IncRef(scalene_pkg_path_owner);
77-
auto enc =
78-
PyUnicode_AsEncodedString(scalene_pkg_path_obj, "utf-8", "strict");
77+
// surrogateescape (not strict): a path holding undecodable bytes shows
78+
// up in Python as lone surrogates, which a strict encoder rejects —
79+
// leaving an exception pending that would later surface at an
80+
// unrelated line (issue #1086).
81+
auto enc = PyUnicode_AsEncodedString(scalene_pkg_path_obj, "utf-8",
82+
"surrogateescape");
7983
if (enc) {
8084
scalene_pkg_path = std::string(PyBytes_AsString(enc));
8185
Py_DecRef(enc);
86+
} else {
87+
PyErr_Clear();
8288
}
8389
} else {
8490
scalene_pkg_path_owner = nullptr;

src/source/pywhere.cpp

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,37 @@ PyObject* set_scalene_done_false(PyObject* self, PyObject* args) {
172172
Py_RETURN_NONE;
173173
}
174174

175+
// Encode a Python filename (a ``str``, e.g. ``code->co_filename``) as
176+
// bytes we can hand to C string functions.
177+
//
178+
// This used to be ``PyUnicode_AsASCIIString``, which fails for any path
179+
// containing a non-ASCII character — a very ordinary situation (issue
180+
// #1086: a module at ``.../überschüsse.py``). Two things went wrong there:
181+
// the caller lost the filename, and — worse — the failed conversion left a
182+
// ``UnicodeEncodeError`` set on the thread. Since this code runs from the
183+
// allocator interposer and from trace callbacks, that stray exception
184+
// surfaced later at an arbitrary, unrelated Python line (or as a
185+
// ``SystemError``/crash), which is why the bug looked flaky.
186+
//
187+
// UTF-8 with ``surrogateescape`` is the right encoding here: it is the
188+
// inverse of how CPython decodes filesystem paths, so every path that
189+
// Python can represent — including undecodable bytes, which appear as lone
190+
// surrogates — round-trips. The Python side decodes these records the same
191+
// way (see ``decode_sample`` in scalene/scalene_mapfile.py). On failure we
192+
// return nullptr with no exception left pending; callers must NULL-check.
193+
static PyObject* encodeFilename(PyObject* unicode) {
194+
if (unicode == nullptr) {
195+
return nullptr;
196+
}
197+
PyObject* bytes =
198+
PyUnicode_AsEncodedString(unicode, "utf-8", "surrogateescape");
199+
if (bytes == nullptr) {
200+
// Non-str object, or an encoder failure we can't do anything about.
201+
PyErr_Clear();
202+
}
203+
return bytes;
204+
}
205+
175206
// Core stack-walk used by both whereInPython and whereInPythonWithStack.
176207
// If ``stack_buf`` is non-null and ``stack_buf_size`` > 0, every traced
177208
// frame encountered (leaf-first) is appended to the buffer as the
@@ -224,7 +255,7 @@ static int whereInPythonImpl(std::string& filename, int& lineno, int& bytei,
224255
PyPtr<PyCodeObject> code =
225256
PyFrame_GetCode(static_cast<PyFrameObject*>(frame));
226257
PyPtr<> co_filename =
227-
PyUnicode_AsASCIIString(static_cast<PyCodeObject*>(code)->co_filename);
258+
encodeFilename(static_cast<PyCodeObject*>(code)->co_filename);
228259

229260
if (!(static_cast<PyObject*>(co_filename))) {
230261
if (stack_bytes_written != nullptr) *stack_bytes_written = written;
@@ -454,16 +485,27 @@ static unchanging_modules module_pointers;
454485
static std::atomic<bool> module_pointers_ready{false};
455486

456487
static bool on_stack(char* outer_filename, int lineno, PyFrameObject* frame) {
488+
if (outer_filename == nullptr) {
489+
// Nothing to compare against. Release the reference the caller handed
490+
// us, matching what the loop below does on every exit path.
491+
Py_XDECREF(frame);
492+
return false;
493+
}
457494
while (frame != NULL) {
458495
int iter_lineno = PyFrame_GetLineNumber(frame);
459496

460497
PyPtr<PyCodeObject> code =
461498
PyFrame_GetCode(static_cast<PyFrameObject*>(frame));
462499

463500
PyPtr<> co_filename(
464-
PyUnicode_AsASCIIString(static_cast<PyCodeObject*>(code)->co_filename));
465-
auto fname = PyBytes_AsString(static_cast<PyObject*>(co_filename));
466-
if (iter_lineno == lineno && strstr(fname, outer_filename)) {
501+
encodeFilename(static_cast<PyCodeObject*>(code)->co_filename));
502+
// NULL-check before strstr: encoding can fail, and dereferencing the
503+
// result unconditionally is how #1086 turned into a segfault.
504+
auto fname = static_cast<PyObject*>(co_filename)
505+
? PyBytes_AsString(static_cast<PyObject*>(co_filename))
506+
: nullptr;
507+
if (fname != nullptr && iter_lineno == lineno &&
508+
strstr(fname, outer_filename)) {
467509
Py_XDECREF(frame);
468510
return true;
469511
}
@@ -939,11 +981,11 @@ static int trace_func(PyObject* obj, PyFrameObject* frame, int what,
939981
static_cast<PyCodeObject*>(code)->co_filename) == 0) {
940982
return 0;
941983
}
942-
PyPtr<> last_fname_unicode(PyUnicode_AsASCIIString(last_fname));
984+
PyPtr<> last_fname_unicode(encodeFilename(last_fname));
943985
auto last_fname_s =
944-
PyBytes_AsString(static_cast<PyObject*>(last_fname_unicode));
945-
PyPtr<> co_filename(
946-
PyUnicode_AsASCIIString(static_cast<PyCodeObject*>(code)->co_filename));
986+
static_cast<PyObject*>(last_fname_unicode)
987+
? PyBytes_AsString(static_cast<PyObject*>(last_fname_unicode))
988+
: nullptr;
947989

948990
// Needed because decref will be called in on_stack
949991
Py_INCREF(frame);
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
"""Regression test for issue #1086: non-ASCII characters in a profiled file's path.
2+
3+
Issue: https://github.com/plasma-umass/scalene/issues/1086
4+
5+
``pywhere.cpp`` converted ``code->co_filename`` with
6+
``PyUnicode_AsASCIIString`` on three hot paths (the stack walker used by
7+
the allocator interposer, ``on_stack``, and the settrace line callback).
8+
For a perfectly ordinary path like ``.../überschüsse.py`` that conversion
9+
fails, returning NULL *and leaving a ``UnicodeEncodeError`` set on the
10+
thread*. Because those paths run from the native allocator hook and from
11+
trace callbacks, the stray exception surfaced later at an arbitrary,
12+
unrelated Python line — which is why the reporter saw it land in
13+
``sysconfig.get_path`` one run and inside pydantic the next. The Python
14+
side then compounded it by decoding the native sample records as ASCII.
15+
16+
The fix encodes/decodes filenames as UTF-8 with surrogate escapes and
17+
NULL-checks every conversion. See ``encodeFilename`` in
18+
``src/source/pywhere.cpp`` and ``decode_sample`` in
19+
``scalene/scalene_mapfile.py``.
20+
21+
Only ``--memory`` runs were affected (the reporter noted ``--cpu-only``
22+
worked), since that is what loads the native interposer, so the
23+
end-to-end test here profiles with memory enabled.
24+
"""
25+
26+
from __future__ import annotations
27+
28+
import json
29+
import subprocess
30+
import sys
31+
import unicodedata
32+
from pathlib import Path
33+
34+
import pytest
35+
36+
from scalene.scalene_mapfile import decode_sample
37+
38+
# A module name with two umlauts, as in the issue's reproducer.
39+
NON_ASCII_MODULE = "überschüsse"
40+
41+
WORKLOAD = """\
42+
def f():
43+
total = 0
44+
data = []
45+
for _ in range(300):
46+
data.append([j for j in range(2000)])
47+
total += sum(data[-1])
48+
return total
49+
"""
50+
51+
MAIN = f"""\
52+
import {NON_ASCII_MODULE}
53+
print({NON_ASCII_MODULE}.f())
54+
"""
55+
56+
# What WORKLOAD's f() returns: sum(range(2000)) * 300.
57+
EXPECTED_OUTPUT = str(sum(range(2000)) * 300)
58+
59+
60+
def test_decode_sample_round_trips_non_ascii_filenames() -> None:
61+
"""Sample records naming a non-ASCII path must decode, not raise.
62+
63+
This is the Python half of the fix, and it is deterministic: the old
64+
``bytes.decode("ascii")`` raised ``UnicodeDecodeError`` here and killed
65+
the loop that drains malloc/free samples.
66+
"""
67+
path = f"/tmp/pröfile/{NON_ASCII_MODULE}.py"
68+
record = f"M,1234,4096,0.5,999,0x7f00,{path},7,0"
69+
assert decode_sample(record.encode("utf-8")) == record
70+
# Undecodable filesystem bytes reach Python as lone surrogates; those
71+
# must survive the round trip too rather than raising.
72+
surrogate_path = "/tmp/\udcff/x.py"
73+
surrogate_record = f"M,1,1,0.0,1,0x0,{surrogate_path},1,0"
74+
assert (
75+
decode_sample(surrogate_record.encode("utf-8", errors="surrogateescape"))
76+
== surrogate_record
77+
)
78+
79+
80+
@pytest.mark.parametrize("in_subdir", [False, True])
81+
def test_memory_profile_of_non_ascii_path(tmp_path: Path, in_subdir: bool) -> None:
82+
"""Profiling a program that imports a module at a non-ASCII path works.
83+
84+
Two placements are covered: the non-ASCII component in the filename
85+
itself, and in a parent directory (the issue reported both a
86+
``überschüsse.py`` module and an ``optionale_verlängerung.py`` under a
87+
non-ASCII tree).
88+
89+
The crash assertions run on every attempt — a regression fails them
90+
immediately and deterministically, since the profiled program dies
91+
outright. Only the "file shows up in the profile" check is retried,
92+
against the usual sampling flake (see ``_scalene_subprocess.py``).
93+
"""
94+
workdir = tmp_path / ("prögramm" if in_subdir else "program")
95+
workdir.mkdir()
96+
module_path = workdir / f"{NON_ASCII_MODULE}.py"
97+
module_path.write_text(WORKLOAD, encoding="utf-8")
98+
main_path = workdir / "main.py"
99+
main_path.write_text(MAIN, encoding="utf-8")
100+
101+
attempts = 3
102+
last_files: list = []
103+
last_output = ""
104+
for attempt in range(1, attempts + 1):
105+
out = tmp_path / f"profile_{attempt}.json"
106+
try:
107+
proc = subprocess.run(
108+
[
109+
sys.executable,
110+
"-m",
111+
"scalene",
112+
"run",
113+
"--memory",
114+
"--no-browser",
115+
"-o",
116+
str(out),
117+
str(main_path),
118+
],
119+
cwd=str(workdir),
120+
check=False,
121+
capture_output=True,
122+
text=True,
123+
timeout=300,
124+
)
125+
except subprocess.TimeoutExpired:
126+
# Scalene startup occasionally wedges under CI contention.
127+
continue
128+
last_output = combined = proc.stdout + proc.stderr
129+
130+
assert "UnicodeEncodeError" not in combined, (
131+
"Non-ASCII path raised UnicodeEncodeError in the profiled program "
132+
f"(issue #1086):\n{combined[-3000:]}"
133+
)
134+
assert "UnicodeDecodeError" not in combined, (
135+
f"Non-ASCII path raised UnicodeDecodeError:\n{combined[-3000:]}"
136+
)
137+
# The workload prints its result; a crash of the profiled program
138+
# (the #1086 failure mode) means this never appears.
139+
assert EXPECTED_OUTPUT in proc.stdout, (
140+
f"Profiled program did not run to completion:\n{combined[-3000:]}"
141+
)
142+
assert proc.returncode == 0, (
143+
f"scalene exited {proc.returncode}:\n{combined[-3000:]}"
144+
)
145+
146+
if not (out.exists() and out.stat().st_size > 0):
147+
continue
148+
files = json.loads(out.read_text(encoding="utf-8")).get("files", {})
149+
last_files = list(files)
150+
# The recorded path must come back intact, not mangled or escaped —
151+
# this exercises the native encode / Python decode pair end to end.
152+
# Normalize first: a filesystem may hand back the decomposed (NFD)
153+
# spelling of "ü" regardless of how we wrote it.
154+
want = unicodedata.normalize("NFC", str(module_path))
155+
if any(unicodedata.normalize("NFC", name) == want for name in files):
156+
return
157+
pytest.skip(
158+
"Scalene recorded no samples for the non-ASCII module after "
159+
f"{attempts} attempts (suspected sampling flake). The program itself "
160+
f"ran cleanly, so the #1086 crash is not present. "
161+
f"Profiled files: {last_files}. Last output:\n{last_output[-1000:]}"
162+
)

0 commit comments

Comments
 (0)