#45 Enhance bytecode decompiler opcode handling and control flow - #46
Merged
sahebbiswas merged 5 commits intoApr 2, 2026
Conversation
…on merging, and add slice/const map reconstruction logic.
…lement POP_EXCEPT cleanup logic
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe diff extends pycrefine's opcode dispatch and control-flow reconstruction (adds many Python 3.11+ opcodes and slice/build handling), changes compound-condition prescan and else/if merge logic, updates marshal/code-object parsing for exception tables, and adds tests and three new test APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant MarshalParser
participant Decompiler as Pycrefine
participant Output
Caller->>MarshalParser: read .pyc stream
MarshalParser->>MarshalParser: parse code, linetable, exceptiontable
MarshalParser->>Decompiler: provide CodeType + tables
Decompiler->>Decompiler: iterate instructions (dispatch by opcode/arg)
Decompiler->>Decompiler: prescan compound conditions, manage stack/blocks
Decompiler->>Output: emit reconstructed source lines (if/else/try/except/slices)
Output-->>Caller: decompiled source
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 6 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 6 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pycrefine.py (2)
3653-3656:⚠️ Potential issue | 🟠 MajorShort-circuit OR/AND handlers still drop semantics by always popping TOS.
Dear friend, on Line 3655 and Line 3684, both handlers model only fall-through and always pop. This loses the preserved operand path required for reconstructing
a or b/a and b, so assignments/returns can be decompiled incorrectly.#!/bin/bash set -euo pipefail python - <<'PY' import dis src = """def f(a, b): x = a or b y = a and b return x, y """ co = compile(src, "<mem>", "exec") fco = next(c for c in co.co_consts if getattr(c, "co_name", "") == "f") print("=== disassembly ===") dis.dis(fco) PY echo echo "=== handler implementation ===" rg -n -A10 -B5 "def _op_jump_if_true_or_pop|def _op_jump_if_false_or_pop|self\\.stack\\.pop\\(" pycrefine.pyAlso applies to: 3683-3685
5275-5286:⚠️ Potential issue | 🟠 MajorException table loading is effectively disabled, and parsing errors are silently swallowed.
With respect, Line 5275 checks
hasattr(types.CodeType, '__code__'), which prevents the 3.11+ exceptiontable path from executing; then Line 5285 uses bareexcept, hiding real marshal/parser faults. This can still strip protected-range metadata and degrade try/with reconstruction.Suggested correction
- try: - # Attempt to read exceptiontable if present (3.11+ marshal format) - # This is speculative - if the stream doesn't have it, we'll catch the exception - if hasattr(types.CodeType, '__code__'): - # Check if the host Python version expects exceptiontable - test_code = (lambda: None).__code__ - if hasattr(test_code, 'co_exceptiontable'): - # Running on 3.11+, try to read exceptiontable from stream - exceptiontable = self.load() - if isinstance(exceptiontable, str): - exceptiontable = bytes(exceptiontable, 'latin1') - elif not isinstance(exceptiontable, bytes): - exceptiontable = bytes(exceptiontable) if exceptiontable else b"" - except: - exceptiontable = b"" + try: + # Host capability check for 3.11+ CodeType shape + if hasattr((lambda: None).__code__, "co_exceptiontable"): + exc_raw = self.load() + if isinstance(exc_raw, str): + exceptiontable = exc_raw.encode("latin1") + elif isinstance(exc_raw, (bytes, bytearray)): + exceptiontable = bytes(exc_raw) + else: + exceptiontable = bytes(exc_raw) if exc_raw else b"" + except (EOFError, TypeError, ValueError, struct.error): + exceptiontable = b""#!/bin/bash set -euo pipefail python - <<'PY' import types print("hasattr(types.CodeType, '__code__') =", hasattr(types.CodeType, '__code__')) print("has co_exceptiontable on host code object =", hasattr((lambda: None).__code__, 'co_exceptiontable')) PY rg -n -A8 -B8 "hasattr\\(types\\.CodeType, '__code__'\\)|exceptiontable = b\"\"|except \\(" pycrefine.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pycrefine.py` around lines 5275 - 5286, Remove the incorrect outer guard that checks hasattr(types.CodeType, '__code__') so the 3.11+ branch uses the runtime test on test_code.co_exceptiontable (i.e., rely on test_code = (lambda: None).__code__ and hasattr(test_code, 'co_exceptiontable') to decide), and replace the bare except: with except Exception as e: to avoid silently swallowing errors—when catching, log the exception (use self.logger.error(...) if available, otherwise fallback to printing to stderr) and then set exceptiontable = b"" as the safe fallback; refer to the identifiers exceptiontable, self.load(), test_code, and co_exceptiontable to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pycrefine.py`:
- Around line 3782-3785: In BUILD_CONST_KEY_MAP handling, guard the
unconditional pops from self.stack: instead of calling self.stack.pop() count
times blindly, check the stack length or test truthiness of self.stack before
each pop and append a safe placeholder (e.g., None) or a sentinel when the stack
is empty so we don't raise IndexError and crash the decompiler; update the loop
that fills values (the block that currently does for _ in range(count):
values.append(self.stack.pop())) to handle underflow and optionally emit a
debug/warning log so reconstruction can degrade gracefully while preserving the
expected number of entries.
---
Duplicate comments:
In `@pycrefine.py`:
- Around line 5275-5286: Remove the incorrect outer guard that checks
hasattr(types.CodeType, '__code__') so the 3.11+ branch uses the runtime test on
test_code.co_exceptiontable (i.e., rely on test_code = (lambda: None).__code__
and hasattr(test_code, 'co_exceptiontable') to decide), and replace the bare
except: with except Exception as e: to avoid silently swallowing errors—when
catching, log the exception (use self.logger.error(...) if available, otherwise
fallback to printing to stderr) and then set exceptiontable = b"" as the safe
fallback; refer to the identifiers exceptiontable, self.load(), test_code, and
co_exceptiontable to locate and update the code.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49c77b14-6a73-42cd-844c-0df16acd2c5d
📒 Files selected for processing (2)
pycrefine.pytests/test_pycrefine.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
pycrefine.py (3)
5269-5292:⚠️ Potential issue | 🟠 MajorThis exception-table probe never executes on the custom parser path.
With respect,
types.CodeTypehas no__code__, so the inner branch is always false andexceptiontablestaysb"". Even if that guard were corrected later, a speculativeself.load()under a blanketexceptis unsafe because a failed probe advancesself.offsetand misaligns the marshal stream. Kindly key this off the input.pycversion and read the field deterministically.#!/bin/bash set -euo pipefail sed -n '5269,5292p' pycrefine.py python - <<'PY' import types print("types.CodeType has __code__:", hasattr(types.CodeType, "__code__")) print("code objects expose co_exceptiontable:", hasattr((lambda: None).__code__, "co_exceptiontable")) PYAlso applies to: 5330-5335
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pycrefine.py` around lines 5269 - 5292, The current probe for exceptiontable never runs (uses hasattr(types.CodeType, '__code__')) and unsafely calls self.load() under a blanket except which can advance self.offset and corrupt parsing; instead determine presence of the exceptiontable from the parsed .pyc header/version (use the magic/version/hash/timestamp you already parse elsewhere) and only read the field when that version indicates 3.11+; when reading, call self.load() exactly once (or use a non-consuming peek method or save/restore self.offset around the call) and remove the blanket except so parsing errors propagate or are handled specifically; apply the same deterministic version-gated logic to the other probe at the region around the 5330-5335 code.
3604-3620:⚠️ Potential issue | 🟠 MajorThe generic
JUMP_IF_NOT_EXC_MATCHpath still has no state for 3.10 typedexcept.With respect, this branch requires two stack values, yet the generic 3.10 flow never materializes the duplicated exception object before
JUMP_IF_NOT_EXC_MATCH; in practice the header can still vanish here. It also never scans forward for theSTORE_*that carriesas e, so bound exception names are still lost on this path.In CPython 3.10 bytecode for `except SomeError as e:`, what values are on the evaluation stack around `DUP_TOP` and `JUMP_IF_NOT_EXC_MATCH`, and where is the `STORE_*` for the `as e` binding placed?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pycrefine.py` around lines 3604 - 3620, The JUMP_IF_NOT_EXC_MATCH path in _op_jump_if_not_exc_match assumes two stack values but for 3.10 typed except the duplicated exception object and the "as e" binding may not be present; update _op_jump_if_not_exc_match to (1) guard for len(self.stack) < 2 and, when missing, scan nearby bytecode (look backwards for a DUP_TOP/DUP_TOP_TWO pattern and forwards for the STORE_* instruction that implements the "as e" binding) using the instruction stream/BytecodeInstruction sequence, (2) synthesize the missing exc_instance value or duplicate the exc_type on the stack so the header reconstruction logic still runs, and (3) capture the target name from the STORE_NAME/STORE_FAST (or other STORE_*) and push a placeholder or the actual bound name (e.g., push the bound variable name instead of losing it) so the reconstructed header becomes "except {exc_type} as {name}:"; keep all changes inside _op_jump_if_not_exc_match and reference JUMP_IF_NOT_EXC_MATCH, DUP_TOP, and STORE_* when locating/handling the missing values.
3621-3685:⚠️ Potential issue | 🟠 MajorPreserve TOS in the
*_OR_POPhandlers.With respect, both handlers still model only the fall-through path by always popping TOS.
a or b,a and b, and chained-comparison scaffolding need the taken branch to leave the left value on the stack, so these forms will still decompile incorrectly.What are the stack semantics of CPython bytecode opcodes `JUMP_IF_TRUE_OR_POP` and `JUMP_IF_FALSE_OR_POP` in Python 3.9 and 3.10, and how are they used for short-circuit expressions and chained comparisons?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pycrefine.py` around lines 3621 - 3685, Both _op_jump_if_true_or_pop and _op_jump_if_false_or_pop currently always pop TOS (modeling only fall-through) which breaks short-circuit and chained-comparison reconstruction; instead, simulate both control-flow paths: take a snapshot of the current stack (e.g. snapshot = self.stack.copy()), for the fall-through path perform the pop (self.stack.pop()) as you already do, and for the taken-jump path preserve the TOS by merging the snapshot (without popping) into the state at jump_target via the existing jump/merge mechanism (use self._get_jump_target(instr) to find target and call the existing stack-merge helper such as self._merge_stack_at(jump_target, snapshot) or append to self._pending_jumps[jump_target] so the analyzer later merges the preserved stack), and remove the unconditional self.stack.pop() so the taken branch is modeled correctly in _op_jump_if_true_or_pop and _op_jump_if_false_or_pop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pycrefine.py`:
- Around line 3776-3790: In _op_build_const_key_map, the recovery path
references self.debug which may not exist and can raise AttributeError; update
the logging check to safely access the flag (e.g., use getattr(self, "debug",
False) or hasattr(self, "debug")) before printing the warning so underflow
recovery never crashes, and keep the existing message/context (including
instr.offset) intact; ensure the same safe check is used anywhere else in this
method that reads self.debug.
- Around line 54-59: The slice opcodes BINARY_SLICE and BUILD_SLICE are missing
from the expression recognition sets, so add "BINARY_SLICE" and "BUILD_SLICE" to
the frozensets _COMPOUND_EXPR_OPS and _TERNARY_PURE (and any other opcode sets
used for boundary checks) and update the mini-evaluator/back-scanning checks to
accept these opcodes (where the code checks instruction membership against those
frozensets and where the evaluator validates rebuildable ops) so that slice
expressions like lst[1:-1] are treated as single expressions rather than
boundaries by the back-scanning logic and the evaluator.
In `@test_files/simple_runner.py`:
- Line 3: The relative import in simple_runner.py using "from . import simple"
fails because test_files is not a package; either make test_files a package by
adding an empty test_files/__init__.py or change the import in simple_runner.py
to an absolute import "from test_files import simple"; also update any
try/except fallback in simple_runner.py that attempts the relative import so it
uses the same absolute import path to avoid the rescue clause failing.
---
Duplicate comments:
In `@pycrefine.py`:
- Around line 5269-5292: The current probe for exceptiontable never runs (uses
hasattr(types.CodeType, '__code__')) and unsafely calls self.load() under a
blanket except which can advance self.offset and corrupt parsing; instead
determine presence of the exceptiontable from the parsed .pyc header/version
(use the magic/version/hash/timestamp you already parse elsewhere) and only read
the field when that version indicates 3.11+; when reading, call self.load()
exactly once (or use a non-consuming peek method or save/restore self.offset
around the call) and remove the blanket except so parsing errors propagate or
are handled specifically; apply the same deterministic version-gated logic to
the other probe at the region around the 5330-5335 code.
- Around line 3604-3620: The JUMP_IF_NOT_EXC_MATCH path in
_op_jump_if_not_exc_match assumes two stack values but for 3.10 typed except the
duplicated exception object and the "as e" binding may not be present; update
_op_jump_if_not_exc_match to (1) guard for len(self.stack) < 2 and, when
missing, scan nearby bytecode (look backwards for a DUP_TOP/DUP_TOP_TWO pattern
and forwards for the STORE_* instruction that implements the "as e" binding)
using the instruction stream/BytecodeInstruction sequence, (2) synthesize the
missing exc_instance value or duplicate the exc_type on the stack so the header
reconstruction logic still runs, and (3) capture the target name from the
STORE_NAME/STORE_FAST (or other STORE_*) and push a placeholder or the actual
bound name (e.g., push the bound variable name instead of losing it) so the
reconstructed header becomes "except {exc_type} as {name}:"; keep all changes
inside _op_jump_if_not_exc_match and reference JUMP_IF_NOT_EXC_MATCH, DUP_TOP,
and STORE_* when locating/handling the missing values.
- Around line 3621-3685: Both _op_jump_if_true_or_pop and
_op_jump_if_false_or_pop currently always pop TOS (modeling only fall-through)
which breaks short-circuit and chained-comparison reconstruction; instead,
simulate both control-flow paths: take a snapshot of the current stack (e.g.
snapshot = self.stack.copy()), for the fall-through path perform the pop
(self.stack.pop()) as you already do, and for the taken-jump path preserve the
TOS by merging the snapshot (without popping) into the state at jump_target via
the existing jump/merge mechanism (use self._get_jump_target(instr) to find
target and call the existing stack-merge helper such as
self._merge_stack_at(jump_target, snapshot) or append to
self._pending_jumps[jump_target] so the analyzer later merges the preserved
stack), and remove the unconditional self.stack.pop() so the taken branch is
modeled correctly in _op_jump_if_true_or_pop and _op_jump_if_false_or_pop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c533d22-2412-4a73-a799-a907f0cb4d20
📒 Files selected for processing (2)
pycrefine.pytest_files/simple_runner.py
…xception handling in decompiler
sahebbiswas
deleted the
45-fix-decompilation-issues-in-function-outputs-api_10-api_11-api_12
branch
April 2, 2026 13:51
This was referenced Apr 3, 2026
Closed
This was referenced Apr 18, 2026
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.
Summary by CodeRabbit
New Features
Bug Fixes
Tests