Fix #1063: non-ASCII --profile-only no longer crashes with SIGSEGV - #1064
Merged
Conversation
TraceConfig converted each --profile-only pattern with PyUnicode_AsASCIIString and passed the result straight to PyBytes_AsString without a NULL check. For non-ASCII input like "é" the ASCII conversion returns NULL, so PyBytes_AsString(NULL) dereferenced it and crashed the process with SIGSEGV. Decode patterns (and base_path) with PyUnicode_AsUTF8AndSize, which accepts UTF-8/non-ASCII input, and skip items that fail to decode instead of dereferencing NULL. Store owned std::string copies (items is now vector<string>, scalene_base_path is now string) so the data outlives any transient conversion object, which also fixes a latent use-after-free of the base_path conversion temporary. Adds tests/test_issue1063_non_ascii_profile_only.py.
The subprocess-based test ran a full memory-mode "scalene run" and asserted on its exit code / stdout, which was fragile under CI sampling and process timing (both the non-ASCII case and the ASCII control failed on CI while passing locally). Rewrite it to drive the fixed TraceConfig constructor directly via pywhere.setup_trace_config (the CPU-only registration path, which needs no libscalene preload). A regression dereferences NULL inside the extension and crashes the interpreter, so the test still captures the exact SIGSEGV failure mode — now without subprocess/sampling flakiness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1063.
Problem
Running Scalene with a non-ASCII value for
--profile-onlycrashes the profiler with a segmentation fault:Root cause
In
src/include/traceconfig.hpp, theTraceConfigconstructor converted each pattern withPyUnicode_AsASCIIString(item)and fed the result straight intoPyBytes_AsString(...)without a NULL check. For non-ASCII input likeé, the ASCII conversion fails and returnsNULL, soPyBytes_AsString(NULL)dereferences a null pointer and crashes the process.The same code also stored raw
char*pointers into transient conversion objects (and had a latent use-after-free of thebase_pathconversion temporary).Fix
base_path) withPyUnicode_AsUTF8AndSize, which accepts UTF-8 / non-ASCII input.NULL), callPyErr_Clear()and skip the item instead of dereferencing NULL.std::stringcopies —itemsis nowstd::vector<std::string>andscalene_base_pathis nowstd::string— so the data outlives any transient Python object. This also fixes the latent use-after-free.should_trace,print) accordingly.Verification
python setup.py build_ext --inplace).target ran, rc=0) instead of segfaulting.tests/test_issue1063_non_ascii_profile_only.py(non-ASCII case + ASCII control); all related tests pass.Note
The fix accepts/skips non-ASCII patterns rather than raising a Python error. A valid UTF-8
éis now matched correctly; truly non-decodable items are silently skipped, which is appropriate since--profile-onlyis a best-effort substring filter.