From e800ea0fdd4ecbe2e28867550a8c36715f3c5ea9 Mon Sep 17 00:00:00 2001
From: Alex Rønne Petersen <alex@alexrp.com>
Date: Sat, 30 Nov 2024 10:20:20 +0100
Subject: wasm: Add a check for zero length around uses of
 memory.copy/memory.fill.

Apparently the WebAssembly spec requires these instructions to trap if the
computed memory access could be out of bounds, even if the length is zero.
Really a rather bizarre design choice.
---
 src/arch/wasm/CodeGen.zig | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

(limited to 'src')

diff --git a/src/arch/wasm/CodeGen.zig b/src/arch/wasm/CodeGen.zig
index 14bc0b2e88..144c85bd9a 100644
--- a/src/arch/wasm/CodeGen.zig
+++ b/src/arch/wasm/CodeGen.zig
@@ -1591,10 +1591,28 @@ fn memcpy(cg: *CodeGen, dst: WValue, src: WValue, len: WValue) !void {
     // When bulk_memory is enabled, we lower it to wasm's memcpy instruction.
     // If not, we lower it ourselves manually
     if (std.Target.wasm.featureSetHas(cg.target.cpu.features, .bulk_memory)) {
+        try cg.startBlock(.block, .empty);
+
+        // Even if `len` is zero, the spec requires an implementation to trap if `src + len` or
+        // `dst + len` are out of memory bounds. This can easily happen in Zig in a case such as:
+        //
+        // const dst: [*]u8 = undefined;
+        // const src: [*]u8 = undefined;
+        // var len: usize = runtime_zero();
+        // @memcpy(dst[0..len], src[0..len]);
+        //
+        // So explicitly avoid using `memory.copy` in the `len == 0` case. Lovely design.
+        try cg.emitWValue(len);
+        try cg.addTag(.i32_eqz);
+        try cg.addLabel(.br_if, 0);
+
         try cg.lowerToStack(dst);
         try cg.lowerToStack(src);
         try cg.emitWValue(len);
         try cg.addExtended(.memory_copy);
+
+        try cg.endBlock();
+
         return;
     }
 
@@ -4782,10 +4800,27 @@ fn memset(cg: *CodeGen, elem_ty: Type, ptr: WValue, len: WValue, value: WValue)
     // When bulk_memory is enabled, we lower it to wasm's memset instruction.
     // If not, we lower it ourselves.
     if (std.Target.wasm.featureSetHas(cg.target.cpu.features, .bulk_memory) and abi_size == 1) {
+        try cg.startBlock(.block, .empty);
+
+        // Even if `len` is zero, the spec requires an implementation to trap if `ptr + len` is
+        // out of memory bounds. This can easily happen in Zig in a case such as:
+        //
+        // const ptr: [*]u8 = undefined;
+        // var len: usize = runtime_zero();
+        // @memset(ptr[0..len], 42);
+        //
+        // So explicitly avoid using `memory.fill` in the `len == 0` case. Lovely design.
+        try cg.emitWValue(len);
+        try cg.addTag(.i32_eqz);
+        try cg.addLabel(.br_if, 0);
+
         try cg.lowerToStack(ptr);
         try cg.emitWValue(value);
         try cg.emitWValue(len);
         try cg.addExtended(.memory_fill);
+
+        try cg.endBlock();
+
         return;
     }
 
-- 
cgit v1.2.3