Ownership test of a validly-positioned zero-length range should succe…

…ed (#416)

fuel-vm/src/interpreter/memory.rs

@@ -485,44 +485,34 @@ impl OwnershipRegisters {
     }
     pub(crate) fn has_ownership_range(&self, range: &MemoryRange) -> bool {
         let (start_incl, end_excl) = range.boundaries(self);
-
         let range = start_incl..end_excl;
-
-        if range.is_empty() {
-            return false;
-        }
-
-        if (self.ssp..self.sp).contains(&start_incl) {
-            return self.has_ownership_stack(&range);
-        }
-
-        self.has_ownership_heap(&range)
+        self.has_ownership_stack(&range) || self.has_ownership_heap(&range)
     }
 
-    /// Zero-length range is never owned
+    /// Empty range is owned iff the range.start is owned
     pub(crate) fn has_ownership_stack(&self, range: &Range<Word>) -> bool {
-        if range.is_empty() {
+        if range.is_empty() && range.start == self.ssp {
+            return true;
+        }
+
+        if !(self.ssp..self.sp).contains(&range.start) {
             return false;
         }
 
         if range.end > VM_MAX_RAM {
             return false;
         }
 
-        (self.ssp..self.sp).contains(&range.start) && (self.ssp..=self.sp).contains(&range.end)
+        (self.ssp..=self.sp).contains(&range.end)
     }
 
-    /// Zero-length range is never owned
+    /// Empty range is owned iff the range.start is 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`
-        if range.is_empty() {
-            return false;
-        }
-
         if range.start < self.hp {
             return false;
         }

fuel-vm/src/interpreter/memory/allocation_tests.rs

@@ -40,6 +40,7 @@ fn test_memclear(has_ownership: bool, a: Word, b: Word) -> Result<(), RuntimeErr
     Ok(())
 }
 
+#[test_case(true, 1, 20, 0 => Ok(()); "Can copy zero bytes")]
 #[test_case(true, 1, 20, 10 => Ok(()); "Can copy some bytes")]
 #[test_case(true, 10, 20, 10 => Ok(()); "Can copy some bytes in close range")]
 #[test_case(true, 21, 20, 10 => Err(PanicReason::MemoryOverflow.into()); "b <= a < bc")]

fuel-vm/src/interpreter/memory/tests.rs

@@ -116,11 +116,11 @@ fn stack_alloc_ownership() {
 
 #[test_case(
     OwnershipRegisters::test(0..0, 0..0, Context::Call{ block_height: Default::default()}), 0..0
-    => false ; "empty mem range"
+    => true ; "empty mem range"
 )]
 #[test_case(
     OwnershipRegisters::test(0..0, 0..0, Context::Script{ block_height: Default::default()}), 0..0
-    => false ; "empty mem range (external)"
+    => true ; "empty mem range (external)"
 )]
 #[test_case(
     OwnershipRegisters::test(0..0, 0..0, Context::Call{ block_height: Default::default()}), 0..1

fuel-vm/src/tests/memory.rs

@@ -46,6 +46,23 @@ fn test_lw() {
     assert_eq!(1, result);
 }
 
+#[test]
+fn test_lw_unaglined() {
+    let ops = vec![
+        op::movi(0x10, 9),
+        op::movi(0x11, 1),
+        op::aloc(0x10),
+        op::move_(0x10, RegId::HP),
+        op::sw(0x10, 0x11, 0),
+        op::lw(0x13, 0x10, 0),
+        op::ret(RegId::ONE),
+    ];
+    let vm = setup(ops);
+    let vm: &Interpreter<MemoryStorage, Script> = vm.as_ref();
+    let result = vm.registers()[0x13_usize];
+    assert_eq!(1, result);
+}
+
 #[test]
 fn test_lb() {
     let ops = vec![
@@ -63,6 +80,26 @@ fn test_lb() {
     assert_eq!(1, result);
 }
 
+#[test]
+fn test_aloc_sb_lb_last_byte_of_memory() {
+    let ops = vec![
+        op::move_(0x20, RegId::HP),
+        op::movi(0x10, 1),
+        op::aloc(0x10),
+        op::move_(0x21, RegId::HP),
+        op::sb(RegId::HP, 0x10, 0),
+        op::lb(0x13, RegId::HP, 0),
+        op::ret(RegId::ONE),
+    ];
+    let vm = setup(ops);
+    let vm: &Interpreter<MemoryStorage, Script> = vm.as_ref();
+    let r1 = vm.registers()[0x20_usize];
+    let r2 = vm.registers()[0x21_usize];
+    assert_eq!(r1 - 1, r2);
+    let result = vm.registers()[0x13_usize] as u8;
+    assert_eq!(1, result);
+}
+
 #[test_case(1, false)]
 #[test_case(2, false)]
 #[test_case(1, true)]
@@ -181,3 +218,157 @@ fn dynamic_call_frame_ops() {
         initial_sp + STACK_EXTEND_AMOUNT as u64 - STACK_SHRINK_AMOUNT as u64
     );
 }
+
+#[rstest::rstest]
+fn test_mcl_and_mcli(
+    #[values(0, 1, 7, 8, 9, 255, 256, 257)] count: u32,
+    #[values(true, false)] half: bool, // Clear only first count/2 bytes
+    #[values(true, false)] mcli: bool, // Test mcli instead of mcl
+) {
+    // Allocate count + 1 bytes of memory, so we can check that the last byte is not cleared
+    let mut ops = vec![op::movi(0x10, count + 1), op::aloc(0x10), op::movi(0x11, 1)];
+    // Fill with ones
+    for i in 0..(count + 1) {
+        ops.push(op::sb(RegId::HP, 0x11, i as u16));
+    }
+    // Clear it, or only half if specified
+    if mcli {
+        if half {
+            ops.push(op::mcli(RegId::HP, count / 2));
+        } else {
+            ops.push(op::mcli(RegId::HP, count));
+        }
+    } else {
+        ops.push(op::movi(0x10, count));
+        if half {
+            ops.push(op::divi(0x10, 0x10, 2));
+        }
+        ops.push(op::mcl(RegId::HP, 0x10));
+    }
+    // Log the result and return
+    ops.push(op::movi(0x10, count + 1));
+    ops.push(op::logd(0, 0, RegId::HP, 0x10));
+    ops.push(op::ret(RegId::ONE));
+
+    let vm = setup(ops);
+    let vm: &Interpreter<MemoryStorage, Script> = vm.as_ref();
+
+    if let Some(Receipt::LogData { data, .. }) = vm.receipts().first() {
+        let c = count as usize;
+        assert_eq!(data.len(), c + 1);
+        if half {
+            assert!(data[..c / 2] == vec![0u8; c / 2]);
+            assert!(data[c / 2..] == vec![1u8; c - c / 2 + 1]);
+        } else {
+            assert!(data[..c] == vec![0u8; c]);
+            assert!(data[c] == 1);
+        }
+    } else {
+        panic!("Expected LogData receipt");
+    }
+}
+
+#[rstest::rstest]
+fn test_mcp_and_mcpi(
+    #[values(0, 1, 7, 8, 9, 255, 256, 257)] count: u32,
+    #[values(true, false)] mcpi: bool, // Test mcpi instead of mcp
+) {
+    // Allocate (count + 1) * 2 bytes of memory, so we can check that the last byte is not copied
+    let mut ops = vec![
+        op::movi(0x10, (count + 1) * 2),
+        op::aloc(0x10),
+        op::movi(0x11, 1),
+        op::movi(0x12, 2),
+    ];
+    // Fill count + 1 bytes with ones, and the next count + 1 bytes with twos
+    for i in 0..(count + 1) * 2 {
+        ops.push(op::sb(RegId::HP, if i < count + 1 { 0x11 } else { 0x12 }, i as u16));
+    }
+    // Compute dst address
+    ops.push(op::addi(0x11, RegId::HP, (count + 1) as u16));
+    // Copy count bytes
+    if mcpi {
+        ops.push(op::mcpi(0x11, RegId::HP, count as u16));
+    } else {
+        ops.push(op::movi(0x10, count));
+        ops.push(op::mcp(0x11, RegId::HP, 0x10));
+    }
+    // Log the result and return
+    ops.push(op::movi(0x10, (count + 1) * 2));
+    ops.push(op::logd(0, 0, RegId::HP, 0x10));
+    ops.push(op::ret(RegId::ONE));
+
+    let vm = setup(ops);
+    let vm: &Interpreter<MemoryStorage, Script> = vm.as_ref();
+
+    if let Some(Receipt::LogData { data, .. }) = vm.receipts().first() {
+        let c = count as usize;
+        assert_eq!(data.len(), (c + 1) * 2);
+        let mut expected = vec![1u8; c * 2 + 1];
+        expected.push(2);
+        assert!(data == &expected);
+    } else {
+        panic!("Expected LogData receipt");
+    }
+}
+
+#[rstest::rstest]
+fn test_meq(
+    #[values(0, 1, 7, 8, 9, 255, 256, 257)] count: u32,
+    #[values("equal", "last-not-equal", "first-not-equal")] pattern: &str,
+) {
+    // Allocate count * 2 bytes of memory
+    let mut ops = vec![
+        op::movi(0x10, count * 2),
+        op::aloc(0x10),
+        op::movi(0x11, 1),
+        op::movi(0x12, 2),
+    ];
+    // Fill count*2 bytes with ones, and then patch with given pattern
+    for i in 0..(count * 2) {
+        ops.push(op::sb(RegId::HP, 0x11, i as u16));
+    }
+    if count != 0 {
+        match pattern {
+            "equal" => {
+                // Do nothing
+            }
+            "last-not-equal" => {
+                ops.push(op::sb(RegId::HP, 0x12, (count * 2 - 1) as u16));
+            }
+            "first-not-equal" => {
+                ops.push(op::sb(RegId::HP, 0x12, 0));
+            }
+            _ => unreachable!(),
+        }
+    }
+
+    // Compare
+    ops.push(op::movi(0x10, count));
+    ops.push(op::addi(0x11, RegId::HP, count as u16));
+    ops.push(op::meq(0x10, RegId::HP, 0x11, 0x10));
+    // Log the result and return
+    ops.push(op::log(0x10, RegId::ZERO, RegId::ZERO, RegId::ZERO));
+    ops.push(op::ret(RegId::ONE));
+
+    let vm = setup(ops);
+    let vm: &Interpreter<MemoryStorage, Script> = vm.as_ref();
+
+    if let Some(Receipt::Log { ra, .. }) = vm.receipts().first() {
+        if count == 0 {
+            assert_eq!(*ra, 1); // Empty ranges always equal
+            return;
+        }
+        match pattern {
+            "equal" => {
+                assert_eq!(*ra, 1);
+            }
+            "last-not-equal" | "first-not-equal" => {
+                assert_eq!(*ra, 0);
+            }
+            _ => unreachable!(),
+        }
+    } else {
+        panic!("Expected LogData receipt");
+    }
+}