Skip to content

feat(decompile): retain contextual flow structure - #754

Open
Jon-Becker wants to merge 1 commit into
feat/contextual-control-flow-regionsfrom
feat/decompiler-control-flow-sidecar
Open

feat(decompile): retain contextual flow structure#754
Jon-Becker wants to merge 1 commit into
feat/contextual-control-flow-regionsfrom
feat/decompiler-control-flow-sidecar

Conversation

@Jon-Becker

Copy link
Copy Markdown
Owner

What changed? Why?

Retains canonical contextual control-flow structure in the decompiler sidecar.

CanonicalAnalysis now keeps the ContextualControlFlow artifact from #753 alongside the canonical program, abstract CFG, versioned expression arenas, contextual SSA, effects, and selector mappings. Upcoming function and region recovery can therefore consume dominators, post-dominators, SCCs, natural loops, irreducibility, and uncertainty without rebuilding or flattening the graph.

Legacy Solidity/Yul and ABI generation remain unchanged.

This PR is stacked on #753.

Notes to reviewers

  • The structural artifact is built directly from the retained ContextualCfg, preserving ID consistency with SSA blocks and selector points.
  • Consumers must check control_flow.uncertainty before treating observed graph structure as complete.
  • This is intentionally another sidecar stage; it does not switch production lowering before SESE recovery and typed canonical IR are validated.
  • Measured region-analysis overhead is <1ms for USDT and Uniswap V2, 3ms for the Uniswap V3 router, and 96ms for Seaport.

How has it been tested?

  • cargo test -p heimdall-decompiler --lib — 75 passed
  • cargo check --workspace --all-targets — passed
  • cargo +nightly fmt --all -- --check — passed
  • git diff --check — passed
  • Extended canonical-sidecar coverage to verify structural dominator retention and exactness on a fully resolved test program.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

❌ AI Evaluation Suite for 554fc16

Completed heimdall-eval. View workflow run.
📊 View the full HTML evaluation report.

📊 View evaluation scores
Test Case CFG Decompilation
NestedLoop 100 15
TransientStorage 100 15
NestedMappings 100 45
SimpleStorage 100 92
WhileLoop 100 20
NestedMapping 100 8
Mapping 100 95
WETH9 100 95
SimpleLoop 100 25
Events 100 62
Average 100 47
⚠️ 7 eval(s) scoring <70%

NestedLoop (CFG: 100, Decompilation: 15)

Decompilation

{
  "score": 15,
  "summary": "The public getter for the storage variable is correctly recovered, but the contract's only real function, loop(uint256), is decompiled as an unconditional revert. The nested loops, the counter arithmetic, and the storage write to `number` are entirely absent, so the core program behavior is not captured.",
  "differences": [
    "loop(uint256) is rendered as `revert()` with no body; the original executes a nested for-loop and always succeeds for any input (including loops == 0).",
    "Both loop constructs (outer i < loops and inner j < loops) and their increment/comparison logic are missing entirely.",
    "The repeated `number += 1` storage read-modify-write to slot 0 is missing; the decompiled function performs no SSTORE, so state changes are lost (loops^2 increments in the original).",
    "loop(uint256) is annotated `public view`, but the original is state-mutating; mutability behavior is misrepresented."
  ]
}

TransientStorage (CFG: 100, Decompilation: 15)

Decompilation

{
  "score": 15,
  "summary": "The decompilation fails to recover five of the six functions. Only setTempOwner is emitted with a body (an approximate transient-slot write); incrementCounter, lock, unlock, getCounter, and isLocked are collapsed into bogus 'public constant' declarations, several with fabricated values (getCounter = 1, isLocked = true) that misrepresent actual behavior. The transient counter read-modify-write, the boolean lock/unlock state writes, and both view getters' loads are entirely absent.",
  "differences": [
    "incrementCounter() is not decompiled: the transient load of counter, the +1 arithmetic, and the store back are all missing (emitted as an empty 'bytes public constant incrementCounter').",
    "lock() is not decompiled: the write of true to the transient locked slot is missing (emitted as an empty constant).",
    "unlock() is not decompiled: the write of false to the transient locked slot is missing (emitted as an empty constant).",
    "getCounter() is misrepresented as 'uint256 public constant getCounter = 1' — it returns a hardcoded 1 instead of loading and returning the transient counter value.",
    "isLocked() is misrepresented as 'bool public constant isLocked = true' — it returns a hardcoded true instead of loading and returning the transient locked flag.",
    "setTempOwner is marked 'pure' despite performing a state (transient) write, and its store is modeled as a masked read-modify-write that ORs the argument into the existing slot value rather than a plain address store; it also lacks the 160-bit mask on arg0.",
    "The transient nature of storage (TSTORE/TLOAD) is only partially reflected: the two declared 'tstore_' variables have unknown slots and one of them (tstore_b) is never used, so no counter/locked state is tracked."
  ]
}

NestedMappings (CFG: 100, Decompilation: 45)

Decompilation

{
  "score": 45,
  "summary": "The state-mutating function (approve) is decompiled perfectly, including the correct nested-mapping storage write keyed by msg.sender and the spender argument. However, both read functions (the explicit allowance getter 0xdd62ed3e and the auto-generated public mapping getter 0x55b6ed5c) are emitted as empty bodies with no storage load, no second argument, and no return value, so two thirds of the contract's observable behavior is missing.",
  "differences": [
    "Function 0xdd62ed3e (allowance) has an empty body: the nested mapping read allowances[owner][spender] and the uint256 return are entirely absent; it is typed as 'public pure' with no return value.",
    "Function 0x55b6ed5c (public mapping getter allowances) likewise has an empty body with no storage read and no return value.",
    "Both getters are recovered with a single address parameter instead of two, so the second key of the nested mapping is lost in their signatures.",
    "State mutability is misreported: the two view getters are marked 'pure' (despite reading storage) and approve is marked 'payable' (the original is non-payable and the bytecode rejects value)."
  ]
}

WhileLoop (CFG: 100, Decompilation: 20)

Decompilation

{
  "score": 20,
  "summary": "The public getter for the storage variable is correctly recovered, but the contract's only real function, loop(uint256), is decompiled as an unconditional revert(). The entire while-loop, the storage increment of `number`, and the loop counter logic are absent, and the function is wrongly marked view despite writing state. Fundamental program behavior is not captured.",
  "differences": [
    "loop(uint256) is rendered as `revert()` — the entire function body is missing; the original unconditionally executes and returns normally",
    "The `while (i < loops)` loop and its bounded iteration/comparison logic are entirely absent",
    "The state write `number = number + 1` (SSTORE to slot 0x00 inside the loop) is missing",
    "The loop counter `i = i + 1` and the resulting number of iterations (arg0 times) is missing",
    "loop() is annotated `public view` although the original mutates storage; mutability behavior is misrepresented",
    "Only number() (the auto-generated getter, SLOAD of slot 0) is faithfully preserved"
  ]
}

NestedMapping (CFG: 100, Decompilation: 8)

Decompilation

{
  "score": 8,
  "summary": "The decompilation fails to capture essentially all of the contract's behavior. Every one of the seven recovered selectors is emitted as either a bare revert() or a pair of trivial calldata-length checks followed by `require(true)`; not a single storage read or write appears anywhere in the output. The original contract consists entirely of nested-mapping slot computations (keccak-based hashing of key/slot pairs) and SSTORE/SLOAD operations, plus four public getters that return mapping values — none of this survives. All functions are also marked `pure` and given no return values, which is the opposite of the actual state-mutating / view semantics.",
  "differences": [
    "No SSTORE operations recovered: setAllowance, setGrid, and setDeepNested all write to nested mappings in the original, but the decompiled bodies contain no storage writes at all (they revert or do nothing).",
    "No SLOAD operations recovered: getAllowance and the three auto-generated public mapping getters read storage and return a value; the decompiled functions return nothing and read nothing.",
    "Nested mapping slot derivation (keccak256(key . keccak256(key . slot)) chains, including the three-level chain for deepNested) is entirely absent.",
    "All return values are lost — getAllowance/allowances should return uint256, grid should return bool, deepNested should return uint256; decompiled functions declare no return type and never RETURN a value.",
    "Mutability is wrong for every function: state-mutating setters and view getters are all emitted as `pure`.",
    "Function arity is wrong: setAllowance(address,address,uint256) and setDeepNested(address,uint256,address,uint256) are represented as single- or two-argument stubs; the remaining parameters are only implied by opaque `msg.data.length >= 0x60 / 0x40` checks.",
    "Four of the seven functions (0x8019f65b, 0x9d266b8a, 0x146008e3, 0x1365b4e1) are reduced to an unconditional revert(), which is functionally incorrect — none of the original functions revert unconditionally.",
    "The three functions that are not pure reverts collapse to `require(true)`, a no-op that discards the entire function body.",
    "The recovered selector set does not correspond to the original ABI (setAllowance = 0x9b8dd18a, setGrid, setDeepNested, getAllowance, and the three public getters), indicating selector/dispatch recovery also failed."
  ]
}

SimpleLoop (CFG: 100, Decompilation: 25)

Decompilation

{
  "score": 25,
  "summary": "The public getter for `number` is correctly recovered (reads storage slot 0x00), but the contract's only behavioral function, `loop(uint256)`, is decompiled as an unconditional `revert()`. The loop construct, the loop-bound comparison against arg0, the increment of the counter, and the repeated storage read-modify-write of `number` are all absent, so the primary program behavior is not captured. The function is additionally marked `view` despite mutating state.",
  "differences": [
    "`loop(uint256)` body is reduced to `revert()`; the original iterates `i` from 0 to `loops` and increments storage `number` each iteration",
    "The bounded for-loop control flow (comparison `i < loops`, increment `i++`, back-edge) is entirely missing",
    "The SSTORE/SLOAD read-modify-write increment of `number` inside the loop is missing, so no state change is represented",
    "`loop` is annotated `public view`, but the original is state-mutating (non-payable, writes storage)",
    "The decompiled `loop` unconditionally reverts, which inverts the success/revert behavior of the original function for all inputs"
  ]
}

Events (CFG: 100, Decompilation: 62)

Decompilation

{
  "score": 62,
  "summary": "All 7 external functions are recovered with correct argument shapes and calldata bounds checks, and 5 of them reproduce their event emissions faithfully (Deposit, Withdrawal, Log(string), LogBytes(bytes), and the three-event emitMultiple including the correct inline string literal \"Multiple events emitted\"). However, the two three-argument functions (emitTransfer and emitApproval) decompile to nothing but calldata-length requires — their sole behavior, the LOG emission, is entirely missing, and the Approval event does not appear anywhere in the output.",
  "differences": [
    "Unresolved_5687f2b8 and Unresolved_23de6651 (the 3-argument functions corresponding to emitTransfer and emitApproval) contain no event emission at all; their only observable effect in the original is lost.",
    "The Approval event is absent from the entire decompilation (not declared and never emitted), so Approval(owner, spender, value) logs are unrepresented.",
    "Both 3-argument functions are decompiled with a single address parameter instead of (address, address, uint256), so two of the three arguments are dropped from the signature.",
    "Indexed vs non-indexed topic layout is not preserved in the recovered event declarations (e.g. Transfer/Deposit/Withdrawal indexed address parameters appear as plain data parameters).",
    "Functions that emit logs are annotated `pure`, which is not equivalent to the original non-payable, state-log-producing mutability."
  ]
}
  • Run AI Evaluation Suite

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Report for a248f24

Metric Value
Base branch 75.42%
PR branch 75.45%
Diff +0.03%

@Jon-Becker
Jon-Becker force-pushed the feat/contextual-control-flow-regions branch from a5cd474 to 87fd8a4 Compare September 6, 2026 21:41
@Jon-Becker
Jon-Becker force-pushed the feat/decompiler-control-flow-sidecar branch from b5be299 to 554fc16 Compare September 6, 2026 21:41
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Benchmark for a248f24

Click to view benchmark
Test Base PR %
heimdall_cfg/complex 198.2±1.72ms 198.1±1.20ms -0.05%
heimdall_cfg/simple 635.1±20.92µs 634.1±8.61µs -0.16%
heimdall_decoder/seaport 58.0±2.12µs 56.0±1.98µs -3.45%
heimdall_decoder/transfer 4.1±0.31µs 3.8±0.24µs -7.32%
heimdall_decoder/uniswap 15.6±0.63µs 15.6±0.69µs 0.00%
heimdall_decompiler/abi_complex 260.4±5.13ms 280.8±1.12ms +7.83%
heimdall_decompiler/abi_simple 2.2±0.01ms 2.2±0.01ms 0.00%
heimdall_decompiler/sol_complex 279.2±3.90ms 298.0±4.24ms +6.73%
heimdall_decompiler/sol_simple 2.7±0.09ms 2.7±0.01ms 0.00%
heimdall_decompiler/yul_complex 278.4±1.98ms 286.5±2.61ms +2.91%
heimdall_decompiler/yul_simple 2.4±0.10ms 2.4±0.05ms 0.00%
heimdall_disassembler/complex 1512.7±25.62µs 1523.3±71.10µs +0.70%
heimdall_disassembler/simple 72.3±4.89µs 72.1±2.57µs -0.28%
heimdall_vm/erc20_transfer 263.0±16.04µs 263.4±10.31µs +0.15%
heimdall_vm/fib 858.9±42.25µs 846.0±10.46µs -1.50%
heimdall_vm/ten_thousand_hashes 707.2±15.57ms 704.9±15.09ms -0.33%

📊 View the full Criterion report

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.

1 participant