Skip to content

fix(ast): strip stack canary boilerplate from decompiled C output#172

Open
wizardengineer wants to merge 1 commit into
mainfrom
fix/strip-stack-canary
Open

fix(ast): strip stack canary boilerplate from decompiled C output#172
wizardengineer wants to merge 1 commit into
mainfrom
fix/strip-stack-canary

Conversation

@wizardengineer

Copy link
Copy Markdown
Contributor

Add is_stack_canary_operation() to FunctionBuilder that detects compiler-inserted stack protector patterns by checking:

  • Operations with inputs referencing __stack_chk_guard globals
  • CALL operations targeting __stack_chk_fail

Detected operations are skipped during basic block creation, removing the canary entry load and fail call from the generated C.

This is a temporary solution that handles the straightforward cases (entry guard load and fail call) by pattern-matching on global names and call targets. The exit comparison block (if guard == saved) is not fully removed because the AST structuring engine reorganizes it into an if-else before our filter can catch the individual operations.

A complete solution would require value-level taint tracking — marking values derived from __stack_chk_guard and propagating that taint through copies, comparisons, and branches so the entire canary exit path can be identified and stripped. That approach is more invasive and should be discussed before implementing.

Partial fix for #161

@wizardengineer wizardengineer changed the title fix(ast): strip stack canary boilerplate from decompiled C output (#161) fix(ast): strip stack canary boilerplate from decompiled C output Mar 18, 2026

@kyle-elliott-tob kyle-elliott-tob left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two test/coverage gaps in the canary filter. Details inline.

Comment thread test/patchir-decomp/bl_device__process_entry.json
Comment thread lib/patchestry/AST/FunctionBuilder.cpp Outdated
@wizardengineer wizardengineer force-pushed the fix/strip-stack-canary branch from 6b52aa3 to de5158c Compare May 15, 2026 16:28
@wizardengineer

Copy link
Copy Markdown
Contributor Author

@kyle-elliott-tob — addressed both review threads and rebased onto main (the branch was conflicting against the create_basic_blockcreate_block_stmts rename from #237/#238).

  • Test fixture (bl_device__process_entry.json): switched to CGEN-LABEL: bl_device__process_entry( + CGEN-NOT: __stack_chk_fail + CGEN-NOT: __stack_chk_guard. Label anchors to the function definition (not the forward decl) and the negation runs to EOF, so unordered_map emission order can't silently pass it.
  • Switch-case inlining (OperationStmt.cpp): mirrored the canary skip in the inlined-case loop alongside the existing OP_BRANCH skip. The single_pred flag is currently false so the path is dead today, but the two emission paths now stay consistent if inlining is re-enabled.

Add is_stack_canary_operation() to FunctionBuilder that detects
compiler-inserted stack protector patterns by checking:
- Operations with inputs referencing __stack_chk_guard globals
- CALL operations targeting __stack_chk_fail

Detected operations are skipped during basic block creation,
removing the canary entry load and fail call from the generated C.

This is a temporary solution that handles the straightforward cases
(entry guard load and fail call) by pattern-matching on global names
and call targets. The exit comparison block (if guard == saved) is
not fully removed because the AST structuring engine reorganizes it
into an if-else before our filter can catch the individual operations.

A complete solution would require value-level taint tracking — marking
values derived from __stack_chk_guard and propagating that taint
through copies, comparisons, and branches so the entire canary exit
path can be identified and stripped. That approach is more invasive
and should be discussed before implementing.

Partial fix for #161

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants