Merge pull request #1309 from topos-protocol/fix_padding_in_wcopy

Fix `wcopy` and `extcodecopy` for ranges over code limit
0xPolygonZero · Oct 25, 2023 · 44af80f · 44af80f
2 parents ed5ec3c + c1c1ab6
commit 44af80f
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 50 deletions.
diff --git a/evm/src/cpu/kernel/asm/account_code.asm b/evm/src/cpu/kernel/asm/account_code.asm
@@ -139,52 +139,43 @@ extcodecopy_contd:
     DUP1 DUP4
     // stack: offset, code_size, code_size, size, offset, dest_offset, retdest
     GT %jumpi(extcodecopy_large_offset)
+
     // stack: code_size, size, offset, dest_offset, retdest
-    SWAP1
-    // stack: size, code_size, offset, dest_offset, retdest
-    PUSH 0
+    DUP3 DUP3 ADD
+    // stack: offset + size, code_size, size, offset, dest_offset, retdest
+    DUP2 GT %jumpi(extcodecopy_within_bounds)
 
-// Loop copying the `code[offset]` to `memory[dest_offset]` until `i==size`.
-// Each iteration increments `offset, dest_offset, i`.
-// TODO: Consider implementing this with memcpy.
-extcodecopy_loop:
-    // stack: i, size, code_size, offset, dest_offset, retdest
-    DUP2 DUP2 EQ
-    // stack: i == size, i, size, code_size, offset, dest_offset, retdest
-    %jumpi(extcodecopy_end)
-    %stack (i, size, code_size, offset, dest_offset, retdest)
-        -> (offset, code_size, offset, code_size, dest_offset, i, size, retdest)
-    LT
-    // stack: offset < code_size, offset, code_size, dest_offset, i, size, retdest
-    DUP2
-    // stack: offset, offset < code_size, offset, code_size, dest_offset, i, size, retdest
-    %mload_kernel(@SEGMENT_KERNEL_ACCOUNT_CODE)
-    // stack: opcode, offset < code_size, offset, code_size, dest_offset, i, size, retdest
-    %stack (opcode, offset_lt_code_size, offset, code_size, dest_offset, i, size, retdest)
-        -> (offset_lt_code_size, 0, opcode, offset, code_size, dest_offset, i, size, retdest)
-    // If `offset >= code_size`, use `opcode=0`. Necessary since `SEGMENT_KERNEL_ACCOUNT_CODE` might be clobbered from previous calls.
-    %select_bool
-    // stack: opcode, offset, code_size, dest_offset, i, size, retdest
-    DUP4
-    // stack: dest_offset, opcode, offset, code_size, dest_offset, i, size, retdest
-    %mstore_current(@SEGMENT_MAIN_MEMORY)
-    // stack: offset, code_size, dest_offset, i, size, retdest
-    %increment
-    // stack: offset+1, code_size, dest_offset, i, size, retdest
-    SWAP2
-    // stack: dest_offset, code_size, offset+1, i, size, retdest
-    %increment
-    // stack: dest_offset+1, code_size, offset+1, i, size, retdest
-    SWAP3
-    // stack: i, code_size, offset+1, dest_offset+1, size, retdest
-    %increment
-    // stack: i+1, code_size, offset+1, dest_offset+1, size, retdest
-    %stack (i, code_size, offset, dest_offset, size, retdest) -> (i, size, code_size, offset, dest_offset, retdest)
-    %jump(extcodecopy_loop)
+    // stack: code_size, size, offset, dest_offset, retdest
+    DUP3 DUP3 ADD
+    // stack: offset + size, code_size, size, offset, dest_offset, retdest
+    SUB
+    // stack: extra_size = offset + size - code_size, size, offset, dest_offset, retdest
+    DUP1 DUP3 SUB
+    // stack: copy_size = size - extra_size, extra_size, size, offset, dest_offset, retdest
 
+    // Compute the new dest_offset after actual copies, at which we will start padding with zeroes.
+    DUP1 DUP6 ADD
+    // stack: new_dest_offset, copy_size, extra_size, size, offset, dest_offset, retdest
+
+    GET_CONTEXT
+    %stack (context, new_dest_offset, copy_size, extra_size, size, offset, dest_offset, retdest) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, 0, @SEGMENT_KERNEL_ACCOUNT_CODE, offset, copy_size, extcodecopy_end, new_dest_offset, extra_size, retdest)
+    %jump(memcpy_bytes)
+
+extcodecopy_within_bounds:
+    // stack: code_size, size, offset, dest_offset, retdest
+    GET_CONTEXT
+    %stack (context, code_size, size, offset, dest_offset, retdest) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, 0, @SEGMENT_KERNEL_ACCOUNT_CODE, offset, size, retdest)
+    %jump(memcpy_bytes)
+
+// Same as extcodecopy_large_offset, but without `offset` in the stack.
 extcodecopy_end:
-    %stack (i, size, code_size, offset, dest_offset, retdest) -> (retdest)
-    JUMP
+    // stack: dest_offset, size, retdest
+    GET_CONTEXT
+    %stack (context, dest_offset, size, retdest) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, size, retdest)
+    %jump(memset)
 
 extcodecopy_large_offset:
     // offset is larger than the code size. So we just have to write zeros.

diff --git a/evm/src/cpu/kernel/asm/memory/memset.asm b/evm/src/cpu/kernel/asm/memory/memset.asm
@@ -2,8 +2,10 @@
 //     DST = (dst_ctx, dst_segment, dst_addr).
 // This tuple definition is used for brevity in the stack comments below.
 global memset:
+    // stack: DST, count, retdest
+
     // Handle empty case
-    DUP7
+    DUP4
     // stack: count, DST, count, retdest
     ISZERO
     // stack: count == 0, DST, count, retdest
@@ -33,9 +35,9 @@ global memset:
     %add_const(0x20)
     SWAP2
     // Decrement count.
-    SWAP4
+    SWAP3
     %sub_const(0x20)
-    SWAP4
+    SWAP3
 
     // Continue the loop.
     %jump(memset)

diff --git a/evm/src/cpu/kernel/asm/memory/syscalls.asm b/evm/src/cpu/kernel/asm/memory/syscalls.asm
@@ -92,12 +92,48 @@ calldataload_large_offset:
     // stack: offset, total_size, kexit_info, dest_offset, offset, size
     GT %jumpi(wcopy_large_offset)
 
+    PUSH $segment
+    %mload_context_metadata($context_metadata_size)
+    // stack: total_size, segment, kexit_info, dest_offset, offset, size
+    DUP6 DUP6 ADD
+    // stack: offset + size, total_size, segment, kexit_info, dest_offset, offset, size
+    LT %jumpi(wcopy_within_bounds)
+
+    %mload_context_metadata($context_metadata_size)
+    // stack: total_size, segment, kexit_info, dest_offset, offset, size
+    DUP6 DUP6 ADD
+    // stack: offset + size, total_size, segment, kexit_info, dest_offset, offset, size
+    SUB // extra_size = offset + size - total_size
+    // stack: extra_size, segment, kexit_info, dest_offset, offset, size
+    DUP1 DUP7 SUB
+    // stack: copy_size = size - extra_size, extra_size, segment, kexit_info, dest_offset, offset, size
+
+    // Compute the new dest_offset after actual copies, at which we will start padding with zeroes.
+    DUP1 DUP6 ADD
+    // stack: new_dest_offset, copy_size, extra_size, segment, kexit_info, dest_offset, offset, size
+
     GET_CONTEXT
-    %stack (context, kexit_info, dest_offset, offset, size) ->
-        (context, @SEGMENT_MAIN_MEMORY, dest_offset, context, $segment, offset, size, wcopy_after, kexit_info)
+    %stack (context, new_dest_offset, copy_size, extra_size, segment, kexit_info, dest_offset, offset, size) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, context, segment, offset, copy_size, wcopy_over_range, new_dest_offset, extra_size, kexit_info)
     %jump(memcpy_bytes)
 %endmacro
 
+wcopy_within_bounds:
+    // stack: segment, kexit_info, dest_offset, offset, size
+    GET_CONTEXT
+    %stack (context, segment, kexit_info, dest_offset, offset, size) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, context, segment, offset, size, wcopy_after, kexit_info)
+    %jump(memcpy_bytes)
+
+
+// Same as wcopy_large_offset, but without `offset` in the stack.
+wcopy_over_range:
+    // stack: dest_offset, size, kexit_info
+    GET_CONTEXT
+    %stack (context, dest_offset, size, kexit_info) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, size, wcopy_after, kexit_info)
+    %jump(memset)
+
 wcopy_empty:
     // stack: Gverylow, kexit_info, dest_offset, offset, size
     %charge_gas
@@ -149,10 +185,30 @@ global sys_returndatacopy:
     // stack: offset, total_size, kexit_info, dest_offset, offset, size
     GT %jumpi(wcopy_large_offset)
 
+    PUSH @SEGMENT_RETURNDATA
+    %mload_context_metadata(@CTX_METADATA_RETURNDATA_SIZE)
+    // stack: total_size, returndata_segment, kexit_info, dest_offset, offset, size
+    DUP6 DUP6 ADD
+    // stack: offset + size, total_size, returndata_segment, kexit_info, dest_offset, offset, size
+    LT %jumpi(wcopy_within_bounds)
+
+    %mload_context_metadata(@CTX_METADATA_RETURNDATA_SIZE)
+    // stack: total_size, returndata_segment, kexit_info, dest_offset, offset, size
+    DUP6 DUP6 ADD
+    // stack: offset + size, total_size, returndata_segment, kexit_info, dest_offset, offset, size
+    SUB // extra_size = offset + size - total_size
+    // stack: extra_size, returndata_segment, kexit_info, dest_offset, offset, size
+    DUP1 DUP7 SUB
+    // stack: copy_size = size - extra_size, extra_size, returndata_segment, kexit_info, dest_offset, offset, size
+
+    // Compute the new dest_offset after actual copies, at which we will start padding with zeroes.
+    DUP1 DUP6 ADD
+    // stack: new_dest_offset, copy_size, extra_size, returndata_segment, kexit_info, dest_offset, offset, size
+
     GET_CONTEXT
-    %stack (context, kexit_info, dest_offset, offset, size) ->
-        (context, @SEGMENT_MAIN_MEMORY, dest_offset, context, @SEGMENT_RETURNDATA, offset, size, wcopy_after, kexit_info)
-    %jump(memcpy)
+    %stack (context, new_dest_offset, copy_size, extra_size, returndata_segment, kexit_info, dest_offset, offset, size) ->
+        (context, @SEGMENT_MAIN_MEMORY, dest_offset, context, returndata_segment, offset, copy_size, wcopy_over_range, new_dest_offset, extra_size, kexit_info)
+    %jump(memcpy_bytes)
 
 returndatacopy_empty:
     %stack (kexit_info, dest_offset, offset, size) -> (kexit_info)