Skip to content

Commit 030e0d4

Browse files
asteriteclaude
andauthored
fix(brillig): emit a trap for unreachable terminators (#13448)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 86ced11 commit 030e0d4

4 files changed

Lines changed: 52 additions & 13 deletions

File tree

compiler/noirc_evaluator/src/brillig/brillig_gen.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ pub(crate) fn gen_brillig_for(
110110

111111
#[cfg(test)]
112112
mod entry_point {
113+
use acvm::acir::brillig::Opcode as BrilligOpcode;
114+
113115
use crate::{
114116
assert_artifact_snapshot,
115117
brillig::{
@@ -287,4 +289,36 @@ mod entry_point {
287289
60: return
288290
");
289291
}
292+
293+
#[test]
294+
fn unreachable_terminator_keeps_jump_targets_in_range() {
295+
let src = "
296+
brillig(inline) fn main f0 {
297+
b0(v0: u32):
298+
constrain v0 == u32 7
299+
unreachable
300+
}
301+
";
302+
let ssa = Ssa::from_str(src).unwrap();
303+
let options = BrilligOptions::default();
304+
let brillig = ssa.to_brillig(&options);
305+
306+
let args = vec![BrilligParameter::SingleAddr(32)];
307+
let entry = gen_brillig_for(ssa.main(), &args, &brillig, &options).unwrap();
308+
309+
let len = entry.byte_code.len();
310+
for (index, opcode) in entry.byte_code.iter().enumerate() {
311+
let target = match opcode {
312+
BrilligOpcode::Jump { location }
313+
| BrilligOpcode::JumpIf { location, .. }
314+
| BrilligOpcode::Call { location } => *location,
315+
_ => continue,
316+
};
317+
assert!(
318+
target < len,
319+
"opcode {index} ({opcode:?}) targets {target}, \
320+
which is out of range for a program of {len} opcodes"
321+
);
322+
}
323+
}
290324
}

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
605605
self.brillig_context.codegen_return(&return_registers);
606606
}
607607
TerminatorInstruction::Unreachable { .. } => {
608-
// If we assume this is unreachable code then there's nothing to do here
608+
self.brillig_context.codegen_trap();
609609
}
610610
}
611611
}

compiler/noirc_evaluator/src/brillig/brillig_gen/tests/memory.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ fn brillig_global_array_not_coalesced_with_block_param() {
186186
let brillig = ssa_to_brillig_artifacts(src);
187187
let main = &brillig.ssa_function_to_brillig[&Id::test_new(0)];
188188
// Key opcodes:
189-
// 13: sp[2] = @68 — global g0 lives in global register @68, copied into sp[2] (param v3's slot)
190-
// 15: return — returns sp[1]; global and param use separate allocations (not coalesced)
189+
// 19: sp[2] = @68 — global g0 lives in global register @68, copied into sp[2] (param v3's slot)
190+
// 21: return — returns sp[1]; global and param use separate allocations (not coalesced)
191191
assert_artifact_snapshot!(main, @r"
192192
fn main
193193
0: sp[3] = @1
@@ -201,14 +201,16 @@ fn brillig_global_array_not_coalesced_with_block_param() {
201201
8: @0 = sp[0]
202202
9: sp[4] = sp[8]
203203
10: jump if sp[4] to 0 // -> 12: f0/b1
204-
11: jump to 0 // -> 17: f0/b2
204+
11: jump to 0 // -> 19: f0/b2
205205
12: sp[2] = const bool 1 // f0/b1
206206
13: sp[3] = bool eq @69, sp[2]
207207
14: jump if sp[3] to 0 // -> 17: f0/b1/1
208208
15: sp[4] = const u32 0
209209
16: trap @[@1; sp[4]]
210-
17: sp[2] = @68 // f0/b1/1, f0/b2
211-
18: jump to 0 // -> 19: f0/b3
212-
19: return // f0/b3
210+
17: sp[2] = const u32 0 // f0/b1/1
211+
18: trap @[@1; sp[2]]
212+
19: sp[2] = @68 // f0/b2
213+
20: jump to 0 // -> 21: f0/b3
214+
21: return // f0/b3
213215
");
214216
}

compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,14 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
186186
self.enter_section(end_section);
187187
}
188188

189+
/// Emits an unconditional trap with empty error data (no error information).
190+
pub(crate) fn codegen_trap(&mut self) {
191+
let error_data = self.make_usize_constant_instruction(0_usize.into()).map(|size| {
192+
HeapVector { pointer: ReservedRegisters::free_memory_pointer(), size: size.address }
193+
});
194+
self.trap_instruction(*error_data);
195+
}
196+
189197
/// Jump to a trap condition if `condition` is false.
190198
/// The trap will include the given message as error data.
191199
///
@@ -204,12 +212,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
204212

205213
// Special case: No error selector means completely empty error data
206214
let Some(error_selector) = error_selector else {
207-
let error_data =
208-
ctx.make_usize_constant_instruction(0_usize.into()).map(|size| HeapVector {
209-
pointer: ReservedRegisters::free_memory_pointer(),
210-
size: size.address,
211-
});
212-
ctx.trap_instruction(*error_data);
215+
ctx.codegen_trap();
213216
return;
214217
};
215218

0 commit comments

Comments
 (0)