Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like I mention in the TS file, it might be better to just throw OutOfRange in the mem_builder and catch in the opcode implementation


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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for now until we think how we want to handle errors elegantly

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