fix(cancun): mcopy check offsets

evm_arithmetization/src/cpu/kernel/asm/memory/memcpy.asm

@@ -45,7 +45,7 @@ global memcpy_bytes:
     %lt_const(0x21)
     // stack: count <= 32, DST, SRC, count, retdest
     %jumpi(memcpy_bytes_finish)
-    
+
     // We will pack 32 bytes into a U256 from the source, and then unpack it at the destination.
     // Copy the next chunk of bytes.
     // stack: DST, SRC, count, retdest
@@ -104,3 +104,54 @@ memcpy_finish:
     %jump(memcpy_bytes)
 %%after:
 %endmacro
+
+// Similar logic to memcpy_bytes, but proceeding the sequence in the backwards direction.
+// Note that this is slightly heavier than the regular `memcpy_bytes`.
+global memcpy_bytes_backwards:
+    // stack: DST, SRC, count, retdest
+
+    // Handle small case
+    DUP3
+    // stack: count, DST, SRC, count, retdest
+    %lt_const(0x21)
+    // stack: count <= 32, DST, SRC, count, retdest
+    %jumpi(memcpy_bytes_finish)
+
+    // We will pack 32 bytes into a U256 from the source, and then unpack it at the destination.
+    // Copy the next chunk of bytes.
+    // stack: DST, SRC, count, retdest
+    PUSH 0x20
+    DUP3
+    // stack: SRC, 32, DST, SRC, count, retdest
+    MLOAD_32BYTES
+    // stack: value, DST, SRC, count, retdest
+    SWAP1
+    // stack: DST, value, SRC, count, retdest
+    MSTORE_32BYTES_32
+    // stack: DST'', SRC, count, retdest
+
+    // Decrement count by 32.
+    SWAP2
+    %sub_const(0x20)
+    SWAP2
+
+    // Decrement DST'' by 32 (from `MSTORE_32BYTES_32` increment) + min(32, count') for the next chunk.
+    // Decrement SRC by min(32, count').
+    // stack: DST'', SRC, count', retdest
+    DUP3 PUSH 0x20 %min
+    // stack: min(32, count'), DST'', SRC, count', retdest
+    DUP1 %add_const(0x20)
+    // stack: 32 + min(32, count'), min(32, count'), DST'', SRC, count', retdest
+    SWAP3 SUB
+    // stack: SRC' = SRC-min(32, count'), DST'', 32 + min(32, count'), count', retdest
+    SWAP2 SWAP1 SUB
+    // stack: DST' = DST''-(32+min(32, count')), SRC', count', retdest
+
+    // Continue the loop.
+    %jump(memcpy_bytes_backwards)
+
+%macro memcpy_bytes_backwards
+    %stack (dst, src, count) -> (dst, src, count, %%after)
+    %jump(memcpy_bytes_backwards)
+%%after:
+%endmacro

evm_arithmetization/src/cpu/kernel/asm/memory/syscalls.asm

@@ -231,6 +231,11 @@ global sys_mcopy:
     DUP1 %ensure_reasonable_offset
     %update_mem_bytes
 
+    %stack (kexit_info, dest_offset, offset, size) -> (offset, size, kexit_info, dest_offset, offset, size)
+    %add_or_fault
+    DUP1 %ensure_reasonable_offset
+    %update_mem_bytes
+
     // stack: kexit_info, dest_offset, offset, size
     DUP3 DUP3 EQ
     // stack: dest_offset = offset, kexit_info, dest_offset, offset, size
@@ -239,46 +244,61 @@ global sys_mcopy:
     // stack: kexit_info, dest_offset, offset, size
     GET_CONTEXT
     PUSH @SEGMENT_MAIN_MEMORY
-    DUP6 DUP6 ADD
-    // stack: offset + size, segment, context, kexit_info, dest_offset, offset, size
-    DUP5 LT
-    // stack: dest_offset < offset + size, segment, context, kexit_info, dest_offset, offset, size
-    DUP6 DUP6 GT
-    // stack: dest_offset > offset, dest_offset < offset + size, segment, context, kexit_info, dest_offset, offset, size
+
+    DUP5 DUP5 LT
+    // stack: dest_offset < offset, kexit_info, dest_offset, offset, size
+    %jumpi(wcopy_within_bounds)
+
+    // stack: segment, context, kexit_info, dest_offset, offset, size
+    DUP6 PUSH 32 %min
+    // stack: shift=min(size, 32), segment, context, kexit_info, dest_offset, offset, size
+    DUP6 DUP8 ADD
+    // stack: offset + size, shift, segment, context, kexit_info, dest_offset, offset, size
+    DUP6 LT
+    // stack: dest_offset < offset + size, shift, segment, context, kexit_info, dest_offset, offset, size
+    DUP2
+    // stack: shift, dest_offset < offset + size, shift, segment, context, kexit_info, dest_offset, offset, size
+    DUP9 GT
+    // stack: size > shift, dest_offset < offset + size, shift, segment, context, kexit_info, dest_offset, offset, size
     MUL // AND
-    // stack: (dest_offset > offset) && (dest_offset < offset + size), segment, context, kexit_info, dest_offset, offset, size
+    // stack: (size > shift) && (dest_offset < offset + size), shift, segment, context, kexit_info, dest_offset, offset, size
 
-    // If both conditions are satisfied, that means we will get an overlap, in which case we need to process the copy
-    // in two chunks to prevent overwriting memory data before reading it.
+    // If the conditions `size > shift` and `dest_offset < offset + size` are satisfied, that means
+    // we will get an overlap that will overwrite some SRC data. In that case, we will proceed to the
+    // memcpy in the backwards direction to never overwrite the SRC section before it has been read.
     %jumpi(mcopy_with_overlap)
 
-    // stack: segment, context, kexit_info, dest_offset, offset, size
+    // Otherwise, we either have `SRC` < `DST`, or a small enough `size` that a single loop of
+    // `memcpy_bytes` suffices and does not risk to overwrite `SRC` data before being read.
+    // stack: shift, segment, context, kexit_info, dest_offset, offset, size
+    POP
     %jump(wcopy_within_bounds)
 
 mcopy_with_overlap:
-    // We do have an overlap between the SRC and DST ranges. We will first copy the overlapping segment
-    // (i.e. end of the copy portion), then copy the remaining (i.e. beginning) portion. 
-
-    // stack: segment, context, kexit_info, dest_offset, offset, size
-    DUP5 DUP5 SUB
-    // stack: remaining_size = dest_offset - offset, segment, context, kexit_info, dest_offset, offset, size
-    DUP1 DUP8
-    SUB // overlapping_size = size - remaining_size
-    // stack: overlapping_size, remaining_size, segment, context, kexit_info, dest_offset, offset, size
-
-    // Shift the initial offsets to copy the overlapping segment first.
-    DUP2 DUP8 ADD
-    // stack: offset_first_copy, overlapping_size, remaining_size, segment, context, kexit_info, dest_offset, offset, size
-    DUP3 DUP8 ADD
-    // stack: dest_offset_first_copy, offset_first_copy, overlapping_size, remaining_size, segment, context, kexit_info, dest_offset, offset, size
-
-    %stack (dest_offset_first_copy, offset_first_copy, overlapping_size, remaining_size, segment, context, kexit_info, dest_offset, offset, size) ->
-        (context, segment, offset_first_copy, segment, dest_offset_first_copy, context, overlapping_size, wcopy_within_bounds, segment, context, kexit_info, dest_offset, offset, remaining_size)
+    // We do have an overlap between the SRC and DST ranges.
+    // We will proceed to `memcpy` in the backwards direction to prevent overwriting unread SRC data.
+    // For this, we need to update `offset` and `dest_offset` to their final position, corresponding
+    // to `x + size - min(32, size)`.
+
+    // stack: shift=min(size, 32), segment, context, kexit_info, dest_offset, offset, size
+    DUP1
+    // stack: shift, shift, segment, context, kexit_info, dest_offset, offset, size
+    DUP8 DUP8 ADD
+    // stack: offset+size, shift, shift, segment, context, kexit_info, dest_offset, offset, size
+    SUB
+    // stack: offset'=offset+size-shift, shift, segment, context, kexit_info, dest_offset, offset, size
+    SWAP5 DUP8 ADD
+    // stack: dest_offset+size, shift, segment, context, kexit_info, offset', offset, size
+    SUB
+    // stack: dest_offset'=dest_offset+size-shift, segment, context, kexit_info, offset', offset, size
+
+    %stack (next_dst_offset, segment, context, kexit_info, new_offset, offset, size) ->
+        (context, segment, new_offset, segment, next_dst_offset, context, size, wcopy_after, kexit_info)
     %build_address // SRC
     SWAP3
     %build_address // DST
-    // stack: DST, SRC, overlapping_size, wcopy_within_bounds, segment, context, kexit_info, dest_offset, offset, remaining_size
-    %jump(memcpy_bytes)
+    // stack: DST, SRC, size, wcopy_after, kexit_info
+    %jump(memcpy_bytes_backwards)
 
 mcopy_empty:
     // kexit_info, dest_offset, offset, size

evm_arithmetization/src/cpu/kernel/tests/mcopy.rs

@@ -0,0 +1,114 @@
+use anyhow::Result;
+use ethereum_types::U256;
+use hex_literal::hex;
+use itertools::Itertools;
+use plonky2::field::goldilocks_field::GoldilocksField as F;
+
+use crate::cpu::kernel::constants::context_metadata::ContextMetadata;
+use crate::cpu::kernel::interpreter::Interpreter;
+use crate::memory::segments::Segment;
+use crate::testing_utils::init_logger;
+
+fn test_mcopy(
+    dest_offset: usize,
+    offset: usize,
+    size: usize,
+    pre_memory: &[u8],
+    post_memory: &[u8],
+) -> Result<()> {
+    init_logger();
+
+    let sys_mcopy = crate::cpu::kernel::aggregator::KERNEL.global_labels["sys_mcopy"];
+    let kexit_info = U256::from(0xdeadbeefu32) + (U256::from(u64::from(true)) << 32);
+    let initial_stack = vec![size.into(), offset.into(), dest_offset.into(), kexit_info];
+
+    let mut interpreter: Interpreter<F> = Interpreter::new(sys_mcopy, initial_stack);
+    interpreter.set_context_metadata_field(
+        0,
+        ContextMetadata::GasLimit,
+        U256::from(1000000000000u64),
+    );
+
+    let pre_memory: Vec<U256> = pre_memory.iter().map(|&b| b.into()).collect_vec();
+    let post_memory: Vec<U256> = post_memory.iter().map(|&b| b.into()).collect_vec();
+
+    interpreter.set_memory_segment(Segment::MainMemory, pre_memory);
+    interpreter.run()?;
+
+    let main_memory_data = interpreter.get_memory_segment(Segment::MainMemory);
+    assert_eq!(&main_memory_data, &post_memory);
+
+    Ok(())
+}
+
+#[test]
+fn test_mcopy_0_32_32() {
+    let dest_offset = 0;
+    let offset = 32;
+    let size = 32;
+    let pre_memory = hex!("0000000000000000000000000000000000000000000000000000000000000000000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f");
+    let post_memory = hex!("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f");
+
+    assert!(test_mcopy(dest_offset, offset, size, &pre_memory, &post_memory).is_ok())
+}
+
+#[test]
+fn test_mcopy_0_0_32() {
+    let dest_offset = 0;
+    let offset = 0;
+    let size = 32;
+    let pre_memory = hex!("0101010101010101010101010101010101010101010101010101010101010101");
+    let post_memory = hex!("0101010101010101010101010101010101010101010101010101010101010101");
+
+    assert!(test_mcopy(dest_offset, offset, size, &pre_memory, &post_memory).is_ok())
+}
+
+#[test]
+fn test_mcopy_0_1_8() {
+    let dest_offset = 0;
+    let offset = 1;
+    let size = 8;
+    let pre_memory = hex!("0001020304050607080000000000000000000000000000000000000000000000");
+    let post_memory = hex!("0102030405060708080000000000000000000000000000000000000000000000");
+
+    assert!(test_mcopy(dest_offset, offset, size, &pre_memory, &post_memory).is_ok())
+}
+
+#[test]
+fn test_mcopy_1_0_8() {
+    let dest_offset = 1;
+    let offset = 0;
+    let size = 8;
+    let pre_memory = hex!("0001020304050607080000000000000000000000000000000000000000000000");
+    let post_memory = hex!("0000010203040506070000000000000000000000000000000000000000000000");
+
+    assert!(test_mcopy(dest_offset, offset, size, &pre_memory, &post_memory).is_ok())
+}
+
+#[test]
+fn test_mcopy_1_0_33() {
+    init_logger();
+    let dest_offset = 1;
+    let offset = 0;
+    let size = 33;
+    let pre_memory =
+        hex!("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f2021222324252627");
+    let post_memory =
+        hex!("00000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20222324252627");
+
+    assert!(test_mcopy(dest_offset, offset, size, &pre_memory, &post_memory).is_ok())
+}
+
+#[test]
+fn test_mcopy_1_2_33() {
+    init_logger();
+    let dest_offset = 1;
+    let offset = 2;
+    let size = 33;
+    let pre_memory =
+        hex!("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728");
+    let post_memory =
+        hex!("0002030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20212222232425262728");
+
+    assert!(test_mcopy(dest_offset, offset, size, &pre_memory, &post_memory).is_ok())
+}

evm_arithmetization/src/cpu/kernel/tests/mod.rs

@@ -13,6 +13,7 @@ mod exp;
 mod hash;
 mod kernel_consistency;
 mod log;
+mod mcopy;
 mod mpt;
 mod packing;
 mod receipt;