fix(brillig): emit a trap for unreachable terminators - #13448
Open
asterite wants to merge 1 commit into
Open
Conversation
Brillig codegen emitted no opcode for an `unreachable` terminator. When the block's last instruction is a `constrain`, its "assertion passed" section label is registered at the next opcode's position, which ended up one past the end of the function's own code. The linker then resolved that label into whatever bytecode was placed next, so the shipped `JumpIf` targeted another function, a shared procedure, or an index past the end of the program (e.g. `regression_5435` shipped a jump target equal to its opcode count). Emit a message-less trap for `unreachable` instead. This keeps every section label in range by construction and turns a wrong unreachability proof into a loud abort instead of silent wrong control flow. Fixes noir-lang/noir-claude#1541 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Changes to Brillig bytecode sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
asterite
marked this pull request as ready for review
August 3, 2026 22:34
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.
In reality the unreachable SSA instruction can never be triggered. That's why we didn't produce any brillig opcode in this case. However, it's probably safer to do a trap in this case. It slightly increases the number of opcodes in a program in this case, but it's always a small constant factor, and not for every program.
Description
Problem
Resolves https://github.com/noir-lang/noir-claude/issues/1541
Brillig codegen emitted no opcode for an
unreachableterminator. When the block's last instruction is aconstrain, its "assertion passed" section label is registered at the position of the next opcode to be emitted — which, with no terminator opcode following, is one past the end of the function's own code. The linker resolves that dangling label like any other, so the shippedJumpIftargets whatever bytecode happened to be linked next: another function's body, a shared procedure such asRevertWithString, or an index past the end of the whole program. Five programs intest_programsship such a jump today;execution_success/regression_5435ships a jump target equal to its opcode count.The edge is currently provably dead (every source-reachable
unreachablesits behind a constrain over unequal constants), so no execution behavior changes — but the malformed address ships in real artifacts, and correctness rests on an unenforced upstream invariant.Summary
Emit a message-less trap for the
unreachableterminator instead of nothing:The trap emission is extracted into a
codegen_traphelper, reused by the existing empty-error-data constrain path (identical bytecode as before).Cost: two opcodes (
const+trap) perunreachableterminator, which by construction only appears on paths believed unreachable.Additional Context
Regression test:
unreachable_terminator_keeps_jump_targets_in_rangecompiles the minimal reproduction (constrain v0 == u32 7followed byunreachable) and asserts everyJump/JumpIf/Calltarget in the linked program is in range. It fails on master withJumpIftargeting 17 in a 17-opcode program.The updated
brillig_global_array_not_coalesced_with_block_paramsnapshot shows the fix directly: the constrain's passed-edge labelf0/b1/1previously resolved to the first opcode of the next block (f0/b2); it now lands on the trap inside its own block.Verified locally: full
noirc_evaluatorsuite (1921 tests) and thenargo_cliexecution integration suite (9130 tests) pass;regression_5435now compiles with all jump targets in range and still executes successfully.PR Checklist
cargo fmton default settings.🤖 Generated with Claude Code