-
-
Notifications
You must be signed in to change notification settings - Fork 34.7k
gh-151022: Fix remote debugging linetable reads #151036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -438,6 +438,41 @@ def _extract_coroutine_stacks_lineno_only(self, stack_trace): | |
| # ============================================================================ | ||
|
|
||
|
|
||
| class TestSelfStackTrace(RemoteInspectionTestBase): | ||
| @skip_if_not_supported | ||
| @unittest.skipIf( | ||
| sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED, | ||
| "Test only runs on Linux with process_vm_readv support", | ||
| ) | ||
| def test_self_trace_with_large_linetable(self): | ||
| script = textwrap.dedent("""\ | ||
| import os | ||
| import _remote_debugging | ||
|
|
||
| assignments = "\\n".join( | ||
| f"value_{i} = {i}" for i in range(1000) | ||
| ) | ||
| source = ( | ||
| f"{assignments}\\n" | ||
| "_remote_debugging.RemoteUnwinder(os.getpid()).get_stack_trace()\\n" | ||
| ) | ||
| code = compile(source, "large_linetable.py", "exec") | ||
| assert len(code.co_linetable) > 4096, len(code.co_linetable) | ||
| exec(code) | ||
| """) | ||
|
|
||
| result = subprocess.run( | ||
| [sys.executable, "-c", script], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=SHORT_TIMEOUT, | ||
| ) | ||
| self.assertEqual( | ||
| result.returncode, 0, | ||
| f"stdout: {result.stdout}\nstderr: {result.stderr}" | ||
|
Comment on lines
+470
to
+472
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @goutamadwant It would be interesting to ensure that the returned linetable is also correct, not |
||
| ) | ||
|
|
||
|
|
||
| @requires_remote_subprocess_debugging() | ||
| class TestGetStackTrace(RemoteInspectionTestBase): | ||
| @skip_if_not_supported | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Fix ``_remote_debugging`` stack traces for code objects with large line | ||
| tables. |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,9 @@ | |||||||||||||
|
|
||||||||||||||
| #include "_remote_debugging.h" | ||||||||||||||
|
|
||||||||||||||
| #define MAX_LINETABLE_SIZE (1024 * 1024) | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @goutamadwant Why exactly 1024 * 1024? :-) I know that's an arbitrary number but my hunch would be to try to check what sizes are we seeing in the wild, or at least in the stdlib. There's |
||||||||||||||
| #define MAX_LINETABLE_ENTRIES 65536 | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not check it but my guess would be that there's much more head room in the cpython/Modules/_remote_debugging/code_objects.c Lines 495 to 500 in 29a920e
To be honest, I'd get rid of Anotehr solution would be along the lines of |
||||||||||||||
|
|
||||||||||||||
| /* ============================================================================ | ||||||||||||||
| * TLBC CACHING FUNCTIONS (Py_GIL_DISABLED only) | ||||||||||||||
| * ============================================================================ */ | ||||||||||||||
|
|
@@ -186,7 +189,6 @@ parse_linetable(const uintptr_t addrq, const char* linetable, Py_ssize_t linetab | |||||||||||||
| int computed_line = firstlineno; // Running accumulator, separate from output | ||||||||||||||
| int val; // Temporary for varint results | ||||||||||||||
| uint8_t byte; // Temporary for byte reads | ||||||||||||||
| const size_t MAX_LINETABLE_ENTRIES = 65536; | ||||||||||||||
| size_t entry_count = 0; | ||||||||||||||
|
|
||||||||||||||
| while (ptr < end && *ptr != '\0' && entry_count < MAX_LINETABLE_ENTRIES) { | ||||||||||||||
|
|
@@ -387,7 +389,8 @@ parse_code_object(RemoteUnwinderObject *unwinder, | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| linetable = read_py_bytes(unwinder, | ||||||||||||||
| GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.linetable), 4096); | ||||||||||||||
| GET_MEMBER(uintptr_t, code_object, unwinder->debug_offsets.code_object.linetable), | ||||||||||||||
| MAX_LINETABLE_SIZE); | ||||||||||||||
| if (!linetable) { | ||||||||||||||
| set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read linetable from code object"); | ||||||||||||||
| goto error; | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goutamadwant I think there's a missing
@requires_remote_subprocess_debugging()decorator :-)