Skip to content

Commit

Permalink
feat(avm): error handling for address resolution (#9994)
Browse files Browse the repository at this point in the history
Resolves #9131
  • Loading branch information
jeanmon authored Nov 22, 2024
1 parent c6fdf4b commit ceaeda5
Show file tree
Hide file tree
Showing 13 changed files with 828 additions and 402 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ TEST_F(AvmCastTests, indirectAddrTruncationU64ToU8)

TEST_F(AvmCastTests, indirectAddrWrongResolutionU64ToU8)
{
// TODO(#9131): Re-enable as part of #9131
// TODO(#9995): Re-enable as part of #9995
GTEST_SKIP();
// Indirect addresses. src:5 dst:6
// Direct addresses. src:10 dst:11
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ TEST_F(AvmMemOpcodeTests, uninitializedValueMov)

TEST_F(AvmMemOpcodeTests, indUninitializedValueMov)
{
// TODO(#9131): Re-enable once we have error handling on wrong address resolution
// TODO(#9995): Re-enable once we have error handling on wrong address resolution
GTEST_SKIP();

trace_builder.op_set(0, 1, 3, AvmMemoryTag::U32);
Expand All @@ -244,7 +244,7 @@ TEST_F(AvmMemOpcodeTests, indUninitializedValueMov)

TEST_F(AvmMemOpcodeTests, indUninitializedAddrMov)
{
// TODO(#9131): Re-enable once we have error handling on wrong address resolution
// TODO(#9995): Re-enable once we have error handling on wrong address resolution
GTEST_SKIP();

trace_builder.op_set(0, 1, 3, AvmMemoryTag::U32);
Expand All @@ -268,7 +268,7 @@ TEST_F(AvmMemOpcodeTests, indirectMov)

TEST_F(AvmMemOpcodeTests, indirectMovInvalidAddressTag)
{
// TODO(#9131): Re-enable once we have error handling on wrong address resolution
// TODO(#9995): Re-enable once we have error handling on wrong address resolution
GTEST_SKIP();

trace_builder.op_set(0, 15, 100, AvmMemoryTag::U32);
Expand Down Expand Up @@ -369,7 +369,7 @@ TEST_F(AvmMemOpcodeTests, indirectSet)

TEST_F(AvmMemOpcodeTests, indirectSetWrongTag)
{
// TODO(#9131): Re-enable once we have error handling on wrong address resolution
// TODO(#9995): Re-enable once we have error handling on wrong address resolution
GTEST_SKIP();

trace_builder.op_set(0, 100, 10, AvmMemoryTag::U8); // The address 100 has incorrect tag U8.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ TEST_F(AvmSliceTests, indirectTwoCallsOverlap)

TEST_F(AvmSliceTests, indirectFailedResolution)
{
// TODO(#9131): Re-enable as part of #9131
// TODO(#9995): Re-enable as part of #9995
GTEST_SKIP();

gen_trace_builder({ 2, 3, 4, 5, 6 });
Expand Down
47 changes: 36 additions & 11 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/addressing_mode.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "barretenberg/vm/avm/trace/errors.hpp"
#include "barretenberg/vm/avm/trace/mem_trace.hpp"
#include <cstdint>

Expand Down Expand Up @@ -30,6 +31,11 @@ struct AddressWithMode {
AddressWithMode operator+(uint val) const noexcept { return { mode, offset + val }; }
};

template <size_t N> struct AddressResolution {
std::array<uint32_t, N> addresses;
AvmError error;
};

template <size_t N> class Addressing {
public:
Addressing(const std::array<AddressingMode, N>& mode_per_operand, uint8_t space_id)
Expand All @@ -47,26 +53,45 @@ template <size_t N> class Addressing {
return Addressing<N>(modes, space_id);
}

std::array<uint32_t, N> resolve(const std::array<uint32_t, N>& offsets, AvmMemTraceBuilder& mem_builder) const
AddressResolution<N> resolve(const std::array<uint32_t, N>& offsets, AvmMemTraceBuilder& mem_builder) const
{
std::array<uint32_t, N> resolved;
std::array<uint32_t, N> resolved_addresses;

for (size_t i = 0; i < N; i++) {
resolved[i] = offsets[i];
auto& res_addr = resolved_addresses[i];
res_addr = offsets[i];
const auto mode = mode_per_operand[i];
if ((static_cast<uint8_t>(mode) & static_cast<uint8_t>(AddressingMode::RELATIVE)) != 0) {
const auto mem_tag = mem_builder.unconstrained_get_memory_tag(space_id, 0);
// TODO(#9131): Error handling needs to be done
ASSERT(mem_tag == AvmMemoryTag::U32);
resolved[i] += static_cast<uint32_t>(mem_builder.unconstrained_read(space_id, 0));

if (mem_tag != AvmMemoryTag::U32) {
return AddressResolution<N>{ .addresses = resolved_addresses,
.error = AvmError::ADDR_RES_TAG_ERROR };
}

const auto base_addr = static_cast<uint32_t>(mem_builder.unconstrained_read(space_id, 0));

// Test if we overflow over uint32_t
if (res_addr + base_addr < base_addr) {
return AddressResolution<N>{ .addresses = resolved_addresses,
.error = AvmError::REL_ADDR_OUT_OF_RANGE };
}

res_addr += base_addr;
}

if ((static_cast<uint8_t>(mode) & static_cast<uint8_t>(AddressingMode::INDIRECT)) != 0) {
const auto mem_tag = mem_builder.unconstrained_get_memory_tag(space_id, resolved[i]);
// TODO(#9131): Error handling needs to be done
ASSERT(mem_tag == AvmMemoryTag::U32);
resolved[i] = static_cast<uint32_t>(mem_builder.unconstrained_read(space_id, resolved[i]));
const auto mem_tag = mem_builder.unconstrained_get_memory_tag(space_id, res_addr);

if (mem_tag != AvmMemoryTag::U32) {
return AddressResolution<N>{ .addresses = resolved_addresses,
.error = AvmError::ADDR_RES_TAG_ERROR };
}

res_addr = static_cast<uint32_t>(mem_builder.unconstrained_read(space_id, res_addr));
}
}
return resolved;
return AddressResolution<N>{ .addresses = resolved_addresses, .error = AvmError::NO_ERROR };
}

private:
Expand Down
10 changes: 0 additions & 10 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,6 @@ enum class AvmMemoryTag : uint32_t {

static const uint32_t MAX_MEM_TAG = MEM_TAG_U128;

enum class AvmError : uint32_t {
NO_ERROR,
TAG_ERROR,
ADDR_RES_ERROR,
DIV_ZERO,
PARSING_ERROR,
ENV_VAR_UNKNOWN,
CONTRACT_INST_MEM_UNKNOWN
};

static const size_t NUM_MEM_SPACES = 256;
static const uint8_t INTERNAL_CALL_SPACE_ID = 255;
static const uint32_t MAX_SIZE_INTERNAL_STACK = 1 << 16;
Expand Down
19 changes: 19 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#pragma once

#include <cstdint>

namespace bb::avm_trace {

enum class AvmError : uint32_t {
NO_ERROR,
TAG_ERROR,
ADDR_RES_TAG_ERROR,
REL_ADDR_OUT_OF_RANGE,
DIV_ZERO,
PARSING_ERROR,
ENV_VAR_UNKNOWN,
CONTRACT_INST_MEM_UNKNOWN,
RADIX_OUT_OF_BOUNDS,
};

} // namespace bb::avm_trace
13 changes: 11 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,10 @@ std::string to_name(AvmError error)
return "NO ERROR";
case AvmError::TAG_ERROR:
return "TAG ERROR";
case AvmError::ADDR_RES_ERROR:
return "ADDRESS RESOLUTION ERROR";
case AvmError::ADDR_RES_TAG_ERROR:
return "ADDRESS RESOLUTION TAG ERROR";
case AvmError::REL_ADDR_OUT_OF_RANGE:
return "RELATIVE ADDRESS IS OUT OF RANGE";
case AvmError::DIV_ZERO:
return "DIVISION BY ZERO";
case AvmError::PARSING_ERROR:
Expand All @@ -116,12 +118,19 @@ std::string to_name(AvmError error)
return "ENVIRONMENT VARIABLE UNKNOWN";
case AvmError::CONTRACT_INST_MEM_UNKNOWN:
return "CONTRACT INSTANCE MEMBER UNKNOWN";
case AvmError::RADIX_OUT_OF_BOUNDS:
return "RADIX OUT OF BOUNDS";
default:
throw std::runtime_error("Invalid error type");
break;
}
}

bool is_ok(AvmError error)
{
return error == AvmError::NO_ERROR;
}

/**
*
* ONLY FOR TESTS - Required by dsl module and therefore cannot be moved to test/helpers.test.cpp
Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ std::string to_hex(bb::avm_trace::AvmMemoryTag tag);
std::string to_name(bb::avm_trace::AvmMemoryTag tag);

std::string to_name(AvmError error);
bool is_ok(AvmError error);

// Mutate the inputs
void inject_end_gas_values(VmPublicInputs& public_inputs, std::vector<Row>& trace);
Expand Down
Loading

0 comments on commit ceaeda5

Please sign in to comment.