Skip to content

gh-155295: Avoid crash de-instrumenting malformed code objects - #155296

Closed
taegyunkim wants to merge 2 commits into
python:mainfrom
taegyunkim:gh-155295-fix-code-deinstrumentation-crash
Closed

gh-155295: Avoid crash de-instrumenting malformed code objects#155296
taegyunkim wants to merge 2 commits into
python:mainfrom
taegyunkim:gh-155295-fix-code-deinstrumentation-crash

Conversation

@taegyunkim

@taegyunkim taegyunkim commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Prevent _PyCode_GetCode() from dereferencing missing monitoring data when a malformed code object contains INSTRUMENTED_LINE or INSTRUMENTED_INSTRUCTION.

Origin and AI assistance

This failure was initially identified while reviewing production crash logs available to Datadog. The investigation, reproducer, implementation, tests, and this PR write-up were produced primarily with GPT-5.6 Sol (high), with human direction, review, and local validation.

deopt_code() now validates the opcode-specific monitoring array before calling _Py_GetBaseCodeUnit(). It raises SystemError and leaves the bytecode cache empty when the code object's instrumentation state is inconsistent.

PyCode_GetCode() documents that it returns NULL with an exception set on error. Because the new consistency check adds a non-allocation failure path, the marshal writer now preserves an exception already set by _PyCode_GetCode() rather than replacing it with MemoryError.

The regression test uses CodeType.replace() to create the malformed state in a subprocess. Without the original fix, both instrumented opcodes terminate the subprocess with SIGSEGV. The test covers direct code.co_code access and marshal.dumps(code).

Validation:

  • ./python -m test test_code -m test_co_code_with_invalid_monitoring_data --verbose
  • ./python -m test -j3 test_code test_marshal test_dis
  • ./python -m test -R 3:3 test_code -m test_co_code_with_invalid_monitoring_data
  • PATH=/usr/bin:/bin:/usr/sbin:/sbin ./python -m test -j12: 50,379 tests executed, 470 test files passed, 3,449 tests skipped, no failures
  • make patchcheck

@taegyunkim
taegyunkim force-pushed the gh-155295-fix-code-deinstrumentation-crash branch from 463d599 to aa742b4 Compare August 6, 2026 17:31
@taegyunkim
taegyunkim marked this pull request as ready for review August 6, 2026 18:00
@taegyunkim
taegyunkim requested a review from markshannon as a code owner August 6, 2026 18:00

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa742b4cd3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Objects/codeobject.c
@taegyunkim
taegyunkim marked this pull request as draft August 6, 2026 18:11
@taegyunkim

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 6c92458775

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@taegyunkim
taegyunkim marked this pull request as ready for review August 6, 2026 18:39
@taegyunkim

Copy link
Copy Markdown
Contributor Author

https://www.githubstatus.com/incidents/qcvjkzcs7j74 incident going on with GH actions :(
GH actions are not running

@picnixz

picnixz commented Aug 6, 2026

Copy link
Copy Markdown
Member

We don't want to avoid those crashes. We don't and won't protect against wrong instrumented code.

@picnixz picnixz closed this Aug 6, 2026
@taegyunkim

Copy link
Copy Markdown
Contributor Author

Thanks for quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants