Skip to content

Commit d936b01

Browse files
authored
fix(avm): Fix relative addressing in fuzzer (#19550)
Resolves https://linear.app/aztec-labs/issue/AVM-198/fix-relative-addressing Relative addressing was pretty broken in the fuzzer: we created references to addresses with different base pointers for the same instruction. This made it so the last base pointer "won", rendering all the other references to addresses invalid, triggering more tag mismatch errors. With this fix we execute roughly 50% more opcodes in the same fuzzer iterations. The fix is to have a common base pointer for all instructions in an instruction block, and to generate code aware of that base pointer. When generating bytecode for that instruction block, we write the base pointer first and then we generate the bytecode for the recorded instructions.
1 parent 253e438 commit d936b01

File tree

15 files changed

+1091
-928
lines changed

15 files changed

+1091
-928
lines changed

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/control_flow.cpp

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ void ControlFlow::process_insert_simple_instruction_block(InsertSimpleInstructio
3838
return;
3939
}
4040
auto instruction_block = instruction_blocks->at(instruction.instruction_block_idx % instruction_blocks->size());
41-
for (const auto& instr : instruction_block) {
42-
current_block->process_instruction(instr);
43-
}
41+
current_block->process_instruction_block(instruction_block);
4442
}
4543

4644
void ControlFlow::process_jump_to_new_block(JumpToNewBlock instruction)
@@ -55,9 +53,7 @@ void ControlFlow::process_jump_to_new_block(JumpToNewBlock instruction)
5553
instruction_blocks->at(instruction.target_program_block_instruction_block_idx % instruction_blocks->size());
5654
ProgramBlock* target_block = new ProgramBlock();
5755
current_block->finalize_with_jump(target_block);
58-
for (const auto& instr : target_instruction_block) {
59-
target_block->process_instruction(instr);
60-
}
56+
target_block->process_instruction_block(target_instruction_block);
6157
current_block = target_block;
6258
}
6359

@@ -76,12 +72,8 @@ void ControlFlow::process_jump_if_to_new_block(JumpIfToNewBlock instruction)
7672
ProgramBlock* target_then_block = new ProgramBlock();
7773
ProgramBlock* target_else_block = new ProgramBlock();
7874
current_block->finalize_with_jump_if(target_then_block, target_else_block, instruction.condition_offset_index);
79-
for (const auto& instr : target_then_instruction_block) {
80-
target_then_block->process_instruction(instr);
81-
}
82-
for (const auto& instr : target_else_instruction_block) {
83-
target_else_block->process_instruction(instr);
84-
}
75+
target_then_block->process_instruction_block(target_then_instruction_block);
76+
target_else_block->process_instruction_block(target_else_instruction_block);
8577
current_block = target_then_block;
8678
}
8779

@@ -178,9 +170,7 @@ void ControlFlow::process_insert_internal_call(InsertInternalCall instruction)
178170
instruction_blocks->at(instruction.target_program_block_instruction_block_idx % instruction_blocks->size());
179171
ProgramBlock* target_block = new ProgramBlock();
180172
current_block->insert_internal_call(target_block);
181-
for (const auto& instr : target_instruction_block) {
182-
target_block->process_instruction(instr);
183-
}
173+
target_block->process_instruction_block(target_instruction_block);
184174
target_block->caller = current_block;
185175
current_block = target_block;
186176
}

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/control_flow.hpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22

33
#include "barretenberg/avm_fuzzer/fuzz_lib/instruction.hpp"
44
#include "barretenberg/avm_fuzzer/fuzz_lib/program_block.hpp"
5+
#include "barretenberg/avm_fuzzer/mutations/instructions/instruction_block.hpp"
56
#include "barretenberg/serialize/msgpack.hpp"
67
#include "barretenberg/vm2/testing/instruction_builder.hpp"
78
#include <vector>
89

10+
using InstructionBlock = bb::avm2::fuzzer::InstructionBlock;
11+
912
struct ReturnOptions {
1013
uint8_t return_size;
1114
MemoryTagWrapper return_value_tag;
@@ -139,7 +142,7 @@ class ControlFlow {
139142
ProgramBlock* current_block;
140143
/// @brief the entry block of the program
141144
ProgramBlock* start_block;
142-
std::vector<std::vector<FuzzInstruction>>* instruction_blocks;
145+
std::vector<InstructionBlock>* instruction_blocks;
143146

144147
/// @brief add instructions to the current block from the instruction block at the given index
145148
/// taken modulo length of the instruction blocks vector
@@ -200,7 +203,7 @@ class ControlFlow {
200203
std::vector<ProgramBlock*> get_reachable_blocks(ProgramBlock* block);
201204

202205
public:
203-
ControlFlow(std::vector<std::vector<FuzzInstruction>>& instruction_blocks)
206+
ControlFlow(std::vector<InstructionBlock>& instruction_blocks)
204207
: current_block(new ProgramBlock())
205208
, start_block(current_block)
206209
, instruction_blocks(&instruction_blocks)

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/fuzz.test.cpp

Lines changed: 148 additions & 137 deletions
Large diffs are not rendered by default.

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/fuzzer_data.hpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,30 @@
11
#pragma once
22

33
#include <cstdint>
4+
#include <iostream>
45
#include <vector>
56

67
#include "barretenberg/avm_fuzzer/fuzz_lib/control_flow.hpp"
78
#include "barretenberg/avm_fuzzer/fuzz_lib/instruction.hpp"
9+
#include "barretenberg/avm_fuzzer/mutations/instructions/instruction_block.hpp"
810
#include "barretenberg/serialize/msgpack.hpp"
911

1012
/// @brief describes the data which will be used for fuzzing
1113
/// Should contain instructions, calldata, CFG instructions, options to disable/enable instructions, etc
1214
struct FuzzerData {
13-
std::vector<std::vector<FuzzInstruction>> instruction_blocks;
15+
std::vector<InstructionBlock> instruction_blocks;
1416
std::vector<CFGInstruction> cfg_instructions;
1517
std::vector<bb::avm2::FF> calldata;
1618
ReturnOptions return_options;
1719
MSGPACK_FIELDS(instruction_blocks, cfg_instructions, calldata, return_options);
1820
};
1921

20-
#include <iostream>
21-
2222
inline std::ostream& operator<<(std::ostream& os, const FuzzerData& data)
2323
{
2424
os << "FuzzerData {\n";
2525
os << " instructions: [\n";
2626
for (const auto& block : data.instruction_blocks) {
27-
os << " [\n";
28-
for (const auto& instr : block) {
29-
os << " " << instr << ",\n";
30-
}
31-
os << " ],\n";
27+
os << " " << block << ",\n";
3228
}
3329
os << " ],\n";
3430
os << " cfg_instructions: [\n";

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/instruction.hpp

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,9 @@ struct VariableRef {
105105
/// Used for Indirect/IndirectRelative modes only
106106
uint16_t pointer_address_seed = 0;
107107

108-
/// @brief A seed for the generation of the base offset
109-
/// Used for Relative/IndirectRelative modes only
110-
/// Sets M[0] = base_offset
111-
uint32_t base_offset_seed = 0;
112108
AddressingModeWrapper mode = AddressingMode::Direct;
113109

114-
MSGPACK_FIELDS(tag, index, pointer_address_seed, base_offset_seed, mode);
110+
MSGPACK_FIELDS(tag, index, pointer_address_seed, mode);
115111
};
116112

117113
struct AddressRef {
@@ -121,12 +117,8 @@ struct AddressRef {
121117
/// Used for Indirect/IndirectRelative modes only
122118
uint16_t pointer_address_seed = 0;
123119

124-
/// @brief A seed for the generation of the base offset
125-
/// Used for Relative/IndirectRelative modes only
126-
/// Sets M[0] = base_offset
127-
uint32_t base_offset_seed = 0;
128120
AddressingModeWrapper mode = AddressingMode::Direct;
129-
MSGPACK_FIELDS(address, pointer_address_seed, base_offset_seed, mode);
121+
MSGPACK_FIELDS(address, pointer_address_seed, mode);
130122
};
131123

132124
using ParamRef = std::variant<VariableRef, AddressRef>;
@@ -138,10 +130,21 @@ using ParamRef = std::variant<VariableRef, AddressRef>;
138130
struct ResolvedAddress {
139131
uint32_t absolute_address = 0;
140132
uint32_t operand_address = 0;
141-
std::optional<uint32_t> base_pointer = std::nullopt;
142133
std::optional<uint32_t> pointer_address = std::nullopt;
134+
bool via_relative = false;
143135
};
144136

137+
inline std::ostream& operator<<(std::ostream& os, const ResolvedAddress& address)
138+
{
139+
os << "ResolvedAddress {\n";
140+
os << " absolute_address: " << address.absolute_address << ",\n";
141+
os << " operand_address: " << address.operand_address << ",\n";
142+
os << " pointer_address: " << address.pointer_address.value() << ",\n";
143+
os << " via_relative: " << address.via_relative << ",\n";
144+
os << "}";
145+
return os;
146+
}
147+
145148
/// @brief mem[result_offset] = mem[a_address] + mem[b_address]
146149
struct ADD_8_Instruction {
147150
ParamRef a_address;
@@ -715,7 +718,7 @@ inline std::ostream& operator<<(std::ostream& os, const MemoryTagWrapper& tag)
715718

716719
inline std::ostream& operator<<(std::ostream& os, const VariableRef& variable)
717720
{
718-
os << "VariableRef " << variable.tag << " " << variable.index << " " << variable.base_offset_seed << " "
721+
os << "VariableRef " << variable.tag << " " << variable.index << " "
719722
<< static_cast<int>(static_cast<AddressingMode>(variable.mode));
720723
return os;
721724
}

barretenberg/cpp/src/barretenberg/avm_fuzzer/fuzz_lib/memory_manager.cpp

Lines changed: 75 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -30,31 +30,41 @@ std::optional<std::ranges::range_value_t<Range>> get_nth_filtered(Range&& range,
3030
return *std::ranges::next(filtered.begin(), static_cast<long>(index % static_cast<size_t>(count)));
3131
}
3232

33-
uint32_t get_max_variable_address(AddressingMode mode, uint32_t literal_max_value)
33+
uint32_t get_max_variable_address(AddressingMode mode, uint32_t base_offset, uint32_t max_operand_address)
3434
{
35-
if (mode == AddressingMode::Direct) {
36-
return literal_max_value;
35+
switch (mode) {
36+
case AddressingMode::Direct:
37+
return max_operand_address;
38+
case AddressingMode::Relative:
39+
return base_offset + max_operand_address;
40+
case AddressingMode::Indirect:
41+
case AddressingMode::IndirectRelative:
42+
return AVM_HIGHEST_MEM_ADDRESS;
3743
}
38-
// In relative addressing, and indirect addressing, we can reach any variable address.
39-
return AVM_HIGHEST_MEM_ADDRESS;
4044
}
4145

42-
uint32_t trimmed_pointer_address(uint32_t pointer_address_seed, uint32_t max_operand_address)
46+
uint32_t get_min_variable_address(AddressingMode mode, uint32_t base_offset)
4347
{
44-
return pointer_address_seed % (max_operand_address + 1);
48+
switch (mode) {
49+
case AddressingMode::Relative:
50+
return base_offset;
51+
case AddressingMode::Direct:
52+
case AddressingMode::Indirect:
53+
case AddressingMode::IndirectRelative:
54+
return 0;
55+
}
4556
}
4657

47-
uint32_t trimmed_relative_operand_address(uint32_t base_offset_seed,
48-
uint32_t relative_target,
49-
uint32_t max_operand_address)
58+
uint32_t trimmed_pointer_address(uint32_t pointer_address_seed, uint32_t max_operand_address)
5059
{
51-
uint32_t max_operand = std::min(relative_target, max_operand_address);
52-
return (base_offset_seed % (max_operand + 1));
60+
return pointer_address_seed % (max_operand_address + 1);
5361
}
5462

55-
uint32_t trimmed_base_pointer(uint32_t base_offset_seed, uint32_t relative_target, uint32_t max_operand_address)
63+
uint32_t trimmed_relative_pointer_address(uint32_t pointer_address_seed,
64+
uint32_t base_offset,
65+
uint32_t max_operand_address)
5666
{
57-
return relative_target - trimmed_relative_operand_address(base_offset_seed, relative_target, max_operand_address);
67+
return base_offset + (pointer_address_seed % (max_operand_address + 1));
5868
}
5969

6070
} // namespace
@@ -107,19 +117,19 @@ ResolvedAddress MemoryManager::resolve_address(VariableRef variable,
107117
break;
108118
}
109119
case AddressingMode::Relative:
110-
resolved_address.base_pointer =
111-
trimmed_base_pointer(variable.base_offset_seed, absolute_address, max_operand_address);
112-
resolved_address.operand_address =
113-
trimmed_relative_operand_address(variable.base_offset_seed, absolute_address, max_operand_address);
120+
BB_ASSERT_LTE(absolute_address, base_offset + max_operand_address);
121+
resolved_address.operand_address = absolute_address - base_offset;
122+
resolved_address.via_relative = true;
114123
break;
115-
case AddressingMode::IndirectRelative:
116-
resolved_address.pointer_address = variable.pointer_address_seed;
124+
case AddressingMode::IndirectRelative: {
125+
uint32_t trimmed_pointer_address_value =
126+
trimmed_relative_pointer_address(variable.pointer_address_seed, base_offset, max_operand_address);
127+
resolved_address.pointer_address = trimmed_pointer_address_value;
117128
// Relative addressing target is the pointer
118-
resolved_address.base_pointer =
119-
trimmed_base_pointer(variable.base_offset_seed, variable.pointer_address_seed, max_operand_address);
120-
resolved_address.operand_address = trimmed_relative_operand_address(
121-
variable.base_offset_seed, variable.pointer_address_seed, max_operand_address);
129+
resolved_address.operand_address = trimmed_pointer_address_value - base_offset;
130+
resolved_address.via_relative = true;
122131
break;
132+
}
123133
case AddressingMode::Direct:
124134
// We can't change the absolute address, or it won't find anything useful there.
125135
resolved_address.operand_address = absolute_address;
@@ -136,30 +146,31 @@ ResolvedAddress MemoryManager::resolve_address(AddressRef address, uint32_t max_
136146
};
137147
switch (address.mode) {
138148
case AddressingMode::Indirect: {
139-
// Trim the pointer to 16 bits for it to be writable by SET_32
149+
// Trim the pointer to max_operand_address bits for it to be reachable by the instruction
140150
uint32_t trimmed_pointer_address_value =
141151
trimmed_pointer_address(address.pointer_address_seed, max_operand_address);
142152
resolved_address.pointer_address = trimmed_pointer_address_value;
143153
resolved_address.operand_address = trimmed_pointer_address_value;
144154
break;
145155
}
146156
case AddressingMode::Relative:
147-
resolved_address.base_pointer =
148-
trimmed_base_pointer(address.base_offset_seed, resolved_address.absolute_address, max_operand_address);
149-
resolved_address.operand_address = trimmed_relative_operand_address(
150-
address.base_offset_seed, resolved_address.absolute_address, max_operand_address);
157+
BB_ASSERT_LTE(address.address, base_offset + max_operand_address);
158+
resolved_address.operand_address = address.address - base_offset;
159+
resolved_address.via_relative = true;
151160
break;
152-
case AddressingMode::IndirectRelative:
153-
resolved_address.pointer_address = address.pointer_address_seed;
161+
case AddressingMode::IndirectRelative: {
154162
// Relative addressing target is the pointer
155-
resolved_address.base_pointer =
156-
trimmed_base_pointer(address.base_offset_seed, address.pointer_address_seed, max_operand_address);
157-
resolved_address.operand_address = trimmed_relative_operand_address(
158-
address.base_offset_seed, address.pointer_address_seed, max_operand_address);
163+
uint32_t trimmed_pointer_address_value =
164+
trimmed_relative_pointer_address(address.pointer_address_seed, base_offset, max_operand_address);
165+
resolved_address.pointer_address = trimmed_pointer_address_value;
166+
167+
resolved_address.operand_address = trimmed_pointer_address_value - base_offset;
168+
resolved_address.via_relative = true;
159169
break;
170+
}
160171
case AddressingMode::Direct:
161172
// Do not delete this assert, if it fails, it means that some address was generated / mutated incorrectly in
162-
// instruction.cpp. Check all the `max_operand` parameters that you're passing to generate_address_ref.
173+
// instruction_block.cpp. Check all the `max_operand` parameters that you're passing to generate_address_ref.
163174
BB_ASSERT_LTE(address.address, max_operand_address);
164175
resolved_address.operand_address = resolved_address.absolute_address;
165176
break;
@@ -197,7 +208,10 @@ std::optional<std::pair<ResolvedAddress, bb::avm2::testing::OperandBuilder>> Mem
197208
std::optional<std::pair<ResolvedAddress, bb::avm2::testing::OperandBuilder>> MemoryManager::
198209
get_resolved_address_and_operand_8(VariableRef address)
199210
{
200-
auto actual_address = get_variable_address(address.tag, address.index, get_max_variable_address(address.mode, 255));
211+
auto actual_address = get_variable_address(address.tag,
212+
address.index,
213+
get_min_variable_address(address.mode, base_offset),
214+
get_max_variable_address(address.mode, base_offset, 255));
201215
if (!actual_address.has_value()) {
202216
return std::nullopt;
203217
}
@@ -214,6 +228,9 @@ std::optional<std::pair<ResolvedAddress, bb::avm2::testing::OperandBuilder>> Mem
214228
get_resolved_address_and_operand_8(AddressRef address)
215229
{
216230
auto resolved_address = resolve_address(address, 255);
231+
232+
BB_ASSERT_LTE(resolved_address.operand_address, uint32_t{ 255 });
233+
217234
auto operand = OperandBuilder::from<uint8_t>(static_cast<uint8_t>(resolved_address.operand_address));
218235
return std::make_pair(resolved_address, get_memory_address_operand(operand, address.mode));
219236
}
@@ -229,8 +246,10 @@ std::optional<std::pair<ResolvedAddress, bb::avm2::testing::OperandBuilder>> Mem
229246
std::optional<std::pair<ResolvedAddress, bb::avm2::testing::OperandBuilder>> MemoryManager::
230247
get_resolved_address_and_operand_16(VariableRef address)
231248
{
232-
auto actual_address =
233-
get_variable_address(address.tag, address.index, get_max_variable_address(address.mode, 65535));
249+
auto actual_address = get_variable_address(address.tag,
250+
address.index,
251+
get_min_variable_address(address.mode, base_offset),
252+
get_max_variable_address(address.mode, base_offset, 65535));
234253
if (!actual_address.has_value()) {
235254
return std::nullopt;
236255
}
@@ -246,18 +265,26 @@ std::optional<std::pair<ResolvedAddress, bb::avm2::testing::OperandBuilder>> Mem
246265
get_resolved_address_and_operand_16(AddressRef address)
247266
{
248267
auto resolved_address = resolve_address(address, 65535);
268+
BB_ASSERT_LTE(resolved_address.operand_address, uint32_t{ 65535 });
269+
249270
auto operand = OperandBuilder::from<uint16_t>(static_cast<uint16_t>(resolved_address.operand_address));
250271
return std::make_pair(resolved_address, get_memory_address_operand(operand, address.mode));
251272
}
252273

253-
std::optional<uint32_t> MemoryManager::get_variable_address(bb::avm2::MemoryTag tag, uint32_t index, uint32_t max_value)
274+
std::optional<uint32_t> MemoryManager::get_variable_address(bb::avm2::MemoryTag tag,
275+
uint32_t index,
276+
uint32_t min_value,
277+
uint32_t max_value)
254278
{
255-
return get_nth_filtered(this->stored_variables[tag], [max_value](uint32_t val) { return val <= max_value; }, index);
279+
return get_nth_filtered(
280+
this->stored_variables[tag],
281+
[min_value, max_value](uint32_t val) { return val >= min_value && val <= max_value; },
282+
index);
256283
}
257284

258285
std::optional<uint8_t> MemoryManager::get_memory_offset_8(bb::avm2::MemoryTag tag, uint32_t index)
259286
{
260-
auto value = get_variable_address(tag, index, 255);
287+
auto value = get_variable_address(tag, index, 0, 255);
261288
if (!value.has_value()) {
262289
return std::nullopt;
263290
}
@@ -267,7 +294,7 @@ std::optional<uint8_t> MemoryManager::get_memory_offset_8(bb::avm2::MemoryTag ta
267294

268295
std::optional<uint16_t> MemoryManager::get_memory_offset_16(bb::avm2::MemoryTag tag, uint32_t index)
269296
{
270-
auto value = get_variable_address(tag, index, 65535);
297+
auto value = get_variable_address(tag, index, 0, 65535);
271298
if (!value.has_value()) {
272299
return std::nullopt;
273300
}
@@ -308,3 +335,8 @@ std::optional<uint16_t> MemoryManager::get_leaf_index(uint16_t note_hash_index)
308335
}
309336
return note_hash_index % emitted_note_hashes.size();
310337
}
338+
339+
void MemoryManager::set_base_offset(uint32_t base_offset)
340+
{
341+
this->base_offset = base_offset;
342+
}

0 commit comments

Comments
 (0)