Skip to content

Commit

Permalink
feat!: Brillig typed memory (#5395)
Browse files Browse the repository at this point in the history
Brillig had implicitely typed memory, where the memory typing was
supossedly respected by the compiler (brillig_gen) but was never checked
in runtime. Instead, in runtime the values were truncated to the
appropiate bit size when used. This hid some bugs in typing that the
compiler was outputting and that would have made the AVM crash.

Memory typing bugs found and fixed:
 - to_radix vector length type
 - to_radix limb type
 - keccak256 length type (u32 vs u64)
 - directive quotient not casting arguments
 - directive invert jump condition on non boolean

This PR aligns brillig more with the AVM by having typed memory actually
checked in runtime, and removing the truncations that arithmetic.rs was
doing.
  • Loading branch information
sirasistant authored Mar 26, 2024
1 parent 543f8a2 commit 16b0bdd
Show file tree
Hide file tree
Showing 47 changed files with 2,433 additions and 2,212 deletions.
1 change: 1 addition & 0 deletions avm-transpiler/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use acvm::acir::brillig::Opcode as BrilligOpcode;
use acvm::acir::circuit::brillig::Brillig;

use acvm::brillig_vm::brillig::{
BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, MemoryAddress, Value, ValueOrArray,
BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, MemoryAddress, ValueOrArray,
};
use acvm::FieldElement;

use crate::instructions::{
AvmInstruction, AvmOperand, AvmTypeTag, ALL_DIRECT, FIRST_OPERAND_INDIRECT,
Expand Down Expand Up @@ -798,7 +799,7 @@ fn handle_getter_instruction(
fn handle_const(
avm_instrs: &mut Vec<AvmInstruction>,
destination: &MemoryAddress,
value: &Value,
value: &FieldElement,
bit_size: &u32,
) {
let tag = tag_from_bit_size(*bit_size);
Expand All @@ -808,7 +809,7 @@ fn handle_const(
avm_instrs.push(generate_set_instruction(tag, dest, value.to_u128()));
} else {
// We can't fit a field in an instruction. This should've been handled in Brillig.
let field = value.to_field();
let field = value;
if !field.fits_in_u128() {
panic!("SET: Field value doesn't fit in 128 bits, that's not supported!");
}
Expand Down
Loading

0 comments on commit 16b0bdd

Please sign in to comment.