Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make $hp point to the first available byte, instead of the one before it #377

Merged
merged 10 commits into from
Mar 15, 2023
Merged

Make $hp point to the first available byte, instead of the one before it #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a9313b2
Make $hp point to the first available byte, instead of the one before it
Dentosal Mar 7, 2023
c829a96
Test that we cannot allocate stack and heap on top of each other
Dentosal Mar 7, 2023
4a268b7
Clippy
bvrooman Mar 7, 2023
0e4cd60
Fix comment
bvrooman Mar 7, 2023
0ae5893
Fix test
bvrooman Mar 7, 2023
cfc60ad
Clippy's else if rules just because
bvrooman Mar 7, 2023
09d67f7
Merge branch 'master' into make-hp-consistent
Mar 8, 2023
c8df7ce
Merge branch 'master' into make-hp-consistent
Dentosal Mar 15, 2023
0211218
fmt
Dentosal Mar 15, 2023
6fb3ee9
Merge branch 'master' into make-hp-consistent
Dentosal Mar 15, 2023
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
2 changes: 1 addition & 1 deletion fuel-vm/src/interpreter/blockchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ impl<'vm, S, I> LoadContractCodeCtx<'vm, S, I> {
let memory_offset_end = checked_add_usize(memory_offset, length)?;

// Validate arguments
if memory_offset_end > *self.hp as usize
if memory_offset_end >= *self.hp as usize
|| contract_id_end as Word > VM_MAX_RAM
|| length > MEM_MAX_ACCESS_SIZE as usize
|| length > self.contract_max_size as usize
Expand Down
2 changes: 1 addition & 1 deletion fuel-vm/src/interpreter/initialization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ where
self.registers[RegId::SSP] = 0;

// Set heap area
self.registers[RegId::HP] = VM_MAX_RAM - 1;
self.registers[RegId::HP] = VM_MAX_RAM;

self.push_stack(self.transaction().id().as_ref())
.map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
Expand Down
65 changes: 39 additions & 26 deletions fuel-vm/src/interpreter/memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use fuel_asm::{PanicReason, RegId};
use fuel_types::{RegisterId, Word};

use std::ops;
use std::ops::Range;

pub type Memory<const SIZE: usize> = Box<[u8; SIZE]>;

Expand Down Expand Up @@ -117,7 +118,7 @@ impl MemoryRange {
{
use ops::Bound::*;

let heap = vm.registers()[RegId::HP].saturating_add(1);
let heap = vm.registers()[RegId::HP];

self.start = match self.start {
Included(start) => Included(heap.saturating_add(start)),
Expand Down Expand Up @@ -272,7 +273,7 @@ where
{
let (result, overflow) = f(*sp, v);

if overflow || result > *hp {
if overflow || result >= *hp {
Err(PanicReason::MemoryOverflow.into())
} else {
*sp = result;
Expand Down Expand Up @@ -335,8 +336,8 @@ pub(crate) fn store_byte(
c: Word,
) -> Result<(), RuntimeError> {
let (ac, overflow) = a.overflowing_add(c);

if overflow || ac >= VM_MAX_RAM || !(owner.has_ownership_stack(ac) || owner.has_ownership_heap(ac)) {
let range = ac..(ac + 1);
if overflow || ac >= VM_MAX_RAM || !(owner.has_ownership_stack(&range) || owner.has_ownership_heap(&range)) {
Err(PanicReason::MemoryOverflow.into())
} else {
memory[ac as usize] = b as u8;
Expand Down Expand Up @@ -483,44 +484,56 @@ impl OwnershipRegisters {
}
}
pub(crate) fn has_ownership_range(&self, range: &MemoryRange) -> bool {
let (a, ab) = range.boundaries(self);
let (start_incl, end_excl) = range.boundaries(self);

let a_is_stack = a < self.sp;
let a_is_heap = a > self.hp;
let range = start_incl..end_excl;

let ab_is_stack = ab <= self.sp;
let ab_is_heap = ab >= self.hp;
if range.is_empty() {
return false;
}

a < ab
&& (a_is_stack && ab_is_stack && self.has_ownership_stack(a) && self.has_ownership_stack_exclusive(ab)
|| a_is_heap && ab_is_heap && self.has_ownership_heap(a) && self.has_ownership_heap_exclusive(ab))
}
if (self.ssp..self.sp).contains(&start_incl) {
return self.has_ownership_stack(&range);
}

pub(crate) const fn has_ownership_stack(&self, a: Word) -> bool {
a <= VM_MAX_RAM && self.ssp <= a && a < self.sp
self.has_ownership_heap(&range)
}

pub(crate) const fn has_ownership_stack_exclusive(&self, a: Word) -> bool {
a <= VM_MAX_RAM && self.ssp <= a && a <= self.sp
/// Zero-length range is never owned
pub(crate) fn has_ownership_stack(&self, range: &Range<Word>) -> bool {
if range.is_empty() {
return false;
}

if range.end > VM_MAX_RAM {
return false;
}

(self.ssp..self.sp).contains(&range.start) && (self.ssp..=self.sp).contains(&range.end)
}

pub(crate) fn has_ownership_heap(&self, a: Word) -> bool {
/// Zero-length range is never owned
pub(crate) fn has_ownership_heap(&self, range: &Range<Word>) -> bool {
// TODO implement fp->hp and (addr, size) validations
// fp->hp
// it means $hp from the previous context, i.e. what's saved in the
// "Saved registers from previous context" of the call frame at
// $fp`
let external = self.context.is_external();
if range.is_empty() {
return false;
}

self.hp < a && (external && a < VM_MAX_RAM || !external && a <= self.prev_hp)
}
if range.start < self.hp {
return false;
}

pub(crate) fn has_ownership_heap_exclusive(&self, a: Word) -> bool {
// TODO reflect the pending changes from `has_ownership_heap`
let external = self.context.is_external();
let heap_end = if self.context.is_external() {
VM_MAX_RAM
} else {
self.prev_hp
};

self.hp < a
&& (external && a <= VM_MAX_RAM || !external && self.prev_hp.checked_add(1).map_or(false, |f| a <= f))
self.hp != heap_end && range.end <= heap_end
}
}

Expand Down
26 changes: 12 additions & 14 deletions fuel-vm/src/interpreter/memory/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,33 +35,31 @@ fn memcopy() {
vm.instruction(op::aloc(0x10)).unwrap();

// r[0x20] := 128
vm.instruction(op::addi(0x20, 0x20, 128)).unwrap();
vm.instruction(op::addi(0x20, RegId::ZERO, 128)).unwrap();

for i in 0..alloc {
vm.instruction(op::addi(0x21, RegId::ZERO, i)).unwrap();
vm.instruction(op::sb(RegId::HP, 0x21, (i + 1) as Immediate12)).unwrap();
vm.instruction(op::sb(RegId::HP, 0x21, i as Immediate12)).unwrap();
}

// r[0x23] := m[$hp, 0x20] == m[0x12, 0x20]
vm.instruction(op::meq(0x23, RegId::HP, 0x12, 0x20)).unwrap();
// r[0x23] := m[$hp, 0x20] == m[$zero, 0x20]
vm.instruction(op::meq(0x23, RegId::HP, RegId::ZERO, 0x20)).unwrap();

assert_eq!(0, vm.registers()[0x23]);

// r[0x12] := $hp + r[0x20]
vm.instruction(op::add(0x12, RegId::HP, 0x20)).unwrap();
vm.instruction(op::add(0x12, RegId::ONE, 0x12)).unwrap();

// Test ownership
vm.instruction(op::add(0x30, RegId::HP, RegId::ONE)).unwrap();
vm.instruction(op::mcp(0x30, 0x12, 0x20)).unwrap();
vm.instruction(op::mcp(RegId::HP, 0x12, 0x20)).unwrap();

// r[0x23] := m[0x30, 0x20] == m[0x12, 0x20]
vm.instruction(op::meq(0x23, 0x30, 0x12, 0x20)).unwrap();
vm.instruction(op::meq(0x23, RegId::HP, 0x12, 0x20)).unwrap();

assert_eq!(1, vm.registers()[0x23]);

// Assert ownership
vm.instruction(op::subi(0x24, RegId::HP, 1)).unwrap();
vm.instruction(op::subi(0x24, RegId::HP, 1)).unwrap(); // TODO: look into this
let ownership_violated = vm.instruction(op::mcp(0x24, 0x12, 0x20));

assert!(ownership_violated.is_err());
Expand All @@ -87,13 +85,13 @@ fn memrange() {
.unwrap();
vm.instruction(op::aloc(0x10)).unwrap();

let m = MemoryRange::new(vm.registers()[RegId::HP], bytes);
let m = MemoryRange::new(vm.registers()[RegId::HP] - 1, bytes);
assert!(!vm.ownership_registers().has_ownership_range(&m));

let m = MemoryRange::new(vm.registers()[RegId::HP] + 1, bytes);
let m = MemoryRange::new(vm.registers()[RegId::HP], bytes);
assert!(vm.ownership_registers().has_ownership_range(&m));

let m = MemoryRange::new(vm.registers()[RegId::HP] + 1, bytes + 1);
let m = MemoryRange::new(vm.registers()[RegId::HP], bytes + 1);
assert!(!vm.ownership_registers().has_ownership_range(&m));

let m = MemoryRange::new(0, bytes).to_heap(&vm);
Expand Down Expand Up @@ -129,7 +127,7 @@ fn stack_alloc_ownership() {
=> false ; "empty stack and heap"
)]
#[test_case(
OwnershipRegisters::test(0..0, 0..0, Context::Script{ block_height: 0}), 0..1
OwnershipRegisters::test(0..0, VM_MAX_RAM..VM_MAX_RAM, Context::Script{ block_height: 0 }), 0..1
=> false ; "empty stack and heap (external)"
)]
#[test_case(
Expand Down Expand Up @@ -157,7 +155,7 @@ fn stack_alloc_ownership() {
=> false; "between ranges (external)"
)]
#[test_case(
OwnershipRegisters::test(0..19, 31..100, Context::Script{ block_height: 0}), 0..1
OwnershipRegisters::test(0..19, 31..100, Context::Script{ block_height: 0 }), 0..1
=> true; "in stack range (external)"
)]
fn test_ownership(reg: OwnershipRegisters, range: Range<u64>) -> bool {
Expand Down
5 changes: 2 additions & 3 deletions fuel-vm/tests/backtrace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,10 @@ fn backtrace() {

contract_call.as_ref().iter().enumerate().for_each(|(i, b)| {
script.push(op::movi(0x10, *b as Immediate18));
script.push(op::sb(RegId::HP, 0x10, 1 + i as Immediate12));
script.push(op::sb(RegId::HP, 0x10, i as Immediate12));
});

script.push(op::addi(0x10, RegId::HP, 1));
script.push(op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS));
script.push(op::call(RegId::HP, RegId::ZERO, RegId::ZERO, RegId::CGAS));
script.push(op::ret(RegId::ONE));

let input_undefined = Input::contract(rng.gen(), rng.gen(), rng.gen(), rng.gen(), contract_undefined);
Expand Down
37 changes: 16 additions & 21 deletions fuel-vm/tests/blockchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ fn state_read_write() {
op::jnei(0x10, 0x31, 45), // (1, b) Unpack arg into 4x16 and xor into state
op::movi(0x20, 32), // r[0x20] := 32
op::aloc(0x20), // aloc 0x20
op::addi(0x20, RegId::HP, 1), // r[0x20] := $hp+1
op::srwq(0x20, SET_STATUS_REG, 0x11, RegId::ONE), // m[0x20,32] := s[m[b, 32], 32]
op::move_(0x20, RegId::HP), // r[0x20] := $hp
op::srwq(0x20, SET_STATUS_REG, 0x11, RegId::ONE), // m[0x20,32] := s[m[b, 32], 32]
op::lw(0x21, 0x11, 4), // r[0x21] := m[b+32, 8]
op::log(0x21, 0x00, 0x00, 0x00),
op::srli(0x22, 0x21, 48), // r[0x22] := r[0x21] >> 48
Expand Down Expand Up @@ -348,15 +348,14 @@ fn load_external_contract_code() {
let index = i as Immediate12;
let value = *byte as Immediate12;
load_contract.extend([
op::xor(reg_a, reg_a, reg_a), // r[a] := 0
op::ori(reg_a, reg_a, value), // r[a] := r[a] | value
op::sb(RegId::HP, reg_a, index + 1), // m[$hp+index+1] := r[a] (=value)
op::xor(reg_a, reg_a, reg_a), // r[a] := 0
op::ori(reg_a, reg_a, value), // r[a] := r[a] | value
op::sb(RegId::HP, reg_a, index), // m[$hp+index] := r[a] (=value)
]);
}

load_contract.extend([
op::move_(reg_a, RegId::HP), // r[a] := $hp
op::addi(reg_a, reg_a, 1), // r[a] += 1
op::xor(reg_b, reg_b, reg_b), // r[b] := 0
op::ori(reg_b, reg_b, 12), // r[b] += 12 (will be padded to 16)
op::ldc(reg_a, RegId::ZERO, reg_b), // Load first two words from the contract
Expand Down Expand Up @@ -497,9 +496,9 @@ fn ldc_reason_helper(cmd: Vec<Instruction>, expected_reason: PanicReason, should
let index = i as Immediate12;
let value = *byte as Immediate12;
load_contract.extend([
op::xor(reg_a, reg_a, reg_a), // r[a] := 0
op::ori(reg_a, reg_a, value), // r[a] := r[a] | value
op::sb(RegId::HP, reg_a, index + 1), // m[$hp+index+1] := r[a] (=value)
op::xor(reg_a, reg_a, reg_a), // r[a] := 0
op::ori(reg_a, reg_a, value), // r[a] := r[a] | value
op::sb(RegId::HP, reg_a, index), // m[$hp+index] := r[a] (=value)
]);
}

Expand Down Expand Up @@ -628,7 +627,6 @@ fn ldc_contract_offset_over_length() {

let load_contract = vec![
op::move_(reg_a, RegId::HP), // r[a] := $hp
op::addi(reg_a, reg_a, 1), // r[a] += 1
op::xor(reg_b, reg_b, reg_b), // r[b] := 0
op::ori(reg_b, reg_b, 12), // r[b] += 12 (will be padded to 16)
op::ldc(reg_a, reg_a, reg_b), // Load first two words from the contract
Expand Down Expand Up @@ -803,7 +801,7 @@ fn code_size_b_over_max_ram() {
fn sww_sets_status() {
#[rustfmt::skip]
let program = vec![
op::sww(0x30, SET_STATUS_REG, RegId::ZERO),
op::sww(0x30, SET_STATUS_REG, RegId::ZERO),
op::srw(0x31, SET_STATUS_REG + 1, RegId::ZERO),
op::log(SET_STATUS_REG, SET_STATUS_REG + 1, 0x00, 0x00),
op::ret(RegId::ONE),
Expand Down Expand Up @@ -832,12 +830,12 @@ fn scwq_clears_status_for_range() {
let program = vec![
op::movi(0x11, 100),
op::aloc(0x11),
op::addi(0x31, RegId::HP, 0x5),
op::addi(0x31, RegId::HP, 0x4),
op::addi(0x32, RegId::ONE, 2),
op::scwq(0x31, SET_STATUS_REG, 0x32),
op::addi(0x31, RegId::HP, 0x5),
op::addi(0x31, RegId::HP, 0x4),
op::swwq(0x31, SET_STATUS_REG + 1, 0x31, 0x32),
op::addi(0x31, RegId::HP, 0x5),
op::addi(0x31, RegId::HP, 0x4),
op::scwq(0x31, SET_STATUS_REG + 2, 0x32),
op::log(SET_STATUS_REG, SET_STATUS_REG + 1, SET_STATUS_REG + 2, 0x00),
op::ret(RegId::ONE),
Expand Down Expand Up @@ -922,11 +920,9 @@ fn swwq_sets_status_with_range() {
let program = vec![
op::movi(0x11, 100),
op::aloc(0x11),
op::addi(0x31, RegId::HP, 0x01),
op::movi(0x32, 0x2),
op::swwq(0x31, SET_STATUS_REG, 0x31, 0x32),
op::addi(0x31, RegId::HP, 0x01),
op::swwq(0x31, SET_STATUS_REG + 1, 0x31, 0x32),
op::swwq(RegId::HP, SET_STATUS_REG, 0x31, 0x32),
op::swwq(RegId::HP, SET_STATUS_REG + 1, 0x31, 0x32),
op::log(SET_STATUS_REG, SET_STATUS_REG + 1, 0x00, 0x00),
op::ret(RegId::ONE),
];
Expand Down Expand Up @@ -1107,11 +1103,10 @@ fn state_r_qword_c_plus_32_over() {
let state_read_qword = vec![
op::movi(0x11, 100),
op::aloc(0x11),
op::addi(0x31, RegId::HP, 0x01),
op::xor(reg_a, reg_a, reg_a),
op::not(reg_a, reg_a),
op::subi(reg_a, reg_a, 31 as Immediate12),
op::srwq(0x31, SET_STATUS_REG, reg_a, RegId::ONE),
op::srwq(RegId::HP, SET_STATUS_REG, reg_a, RegId::ONE),
];

check_expected_reason_for_instructions(state_read_qword, MemoryOverflow);
Expand Down Expand Up @@ -1143,7 +1138,7 @@ fn state_r_qword_c_over_max_ram() {
let state_read_qword = vec![
op::movi(0x11, 100),
op::aloc(0x11),
op::addi(0x31, RegId::HP, 0x01),
op::move_(0x31, RegId::HP),
op::xor(reg_a, reg_a, reg_a),
op::ori(reg_a, reg_a, 1),
op::slli(reg_a, reg_a, MAX_MEM_SHL),
Expand Down
8 changes: 4 additions & 4 deletions fuel-vm/tests/crypto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn ecrecover() {
op::addi(0x22, 0x21, message.as_ref().len() as Immediate12),
op::movi(0x10, PublicKey::LEN as Immediate18),
op::aloc(0x10),
op::addi(0x11, RegId::HP, 1),
op::move_(0x11, RegId::HP),
op::ecr(0x11, 0x20, 0x21),
op::meq(0x12, 0x22, 0x11, 0x10),
op::log(0x12, 0x00, 0x00, 0x00),
Expand Down Expand Up @@ -81,7 +81,7 @@ fn ecrecover_error() {
op::addi(0x22, 0x21, message.as_ref().len() as Immediate12),
op::movi(0x10, PublicKey::LEN as Immediate18),
op::aloc(0x10),
op::addi(0x11, RegId::HP, 1),
op::move_(0x11, RegId::HP),
op::ecr(0x11, 0x20, 0x21),
];

Expand Down Expand Up @@ -159,7 +159,7 @@ fn sha256() {
op::addi(0x21, 0x20, message.len() as Immediate12),
op::movi(0x10, Bytes32::LEN as Immediate18),
op::aloc(0x10),
op::addi(0x11, RegId::HP, 1),
op::move_(0x11, RegId::HP),
op::movi(0x12, message.len() as Immediate18),
op::s256(0x11, 0x20, 0x12),
op::meq(0x13, 0x11, 0x21, 0x10),
Expand Down Expand Up @@ -251,7 +251,7 @@ fn keccak256() {
op::addi(0x21, 0x20, message.len() as Immediate12),
op::movi(0x10, Bytes32::LEN as Immediate18),
op::aloc(0x10),
op::addi(0x11, RegId::HP, 1),
op::move_(0x11, RegId::HP),
op::movi(0x12, message.len() as Immediate18),
op::k256(0x11, 0x20, 0x12),
op::meq(0x13, 0x11, 0x21, 0x10),
Expand Down
6 changes: 3 additions & 3 deletions fuel-vm/tests/flow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn code_copy() {
let mut script_ops = vec![
op::movi(0x10, 2048),
op::aloc(0x10),
op::addi(0x10, RegId::HP, 0x01),
op::move_(0x10, RegId::HP),
op::movi(0x20, 0x00),
op::add(0x11, RegId::ZERO, 0x20),
op::movi(0x12, contract_size as Immediate18),
Expand Down Expand Up @@ -887,7 +887,7 @@ fn retd_from_top_of_heap() {
let script = vec![
op::movi(REG_SIZE, 32), // Allocate 32 bytes.
op::aloc(REG_SIZE), // $hp -= 32.
op::addi(REG_PTR, RegId::HP, 1), // Pointer is $hp + 1, first byte in allocated buffer.
op::move_(REG_PTR, RegId::HP), // Pointer is $hp, first byte in allocated buffer.
op::retd(REG_PTR, REG_SIZE), // Return the allocated buffer.
].into_iter()
.collect::<Vec<u8>>();
Expand Down Expand Up @@ -922,7 +922,7 @@ fn logd_from_top_of_heap() {
let script = vec![
op::movi(REG_SIZE, 32), // Allocate 32 bytes.
op::aloc(REG_SIZE), // $hp -= 32.
op::addi(reg_ptr, RegId::HP, 1), // Pointer is $hp + 1, first byte in allocated buffer.
op::move_(reg_ptr, RegId::HP), // Pointer is $hp, first byte in allocated buffer.
op::logd(RegId::ZERO, RegId::ZERO, reg_ptr, REG_SIZE), // Log the whole buffer
op::ret(RegId::ONE), // Return
].into_iter()
Expand Down
Loading