zig icon indicating copy to clipboard operation
zig copied to clipboard

Compiler misteriously modifies field value on debug mode

Open Aandreba opened this issue 2 years ago • 3 comments
trafficstars

Zig Version

0.11.0-dev.3380+7e0a02ee2

Steps to Reproduce and Observed Behavior

This bug was found executing a shared-memory Wasm library on Deno. When the code is compiled with ReleaseSmall, the bug disapears and the code works as expected.

When executing the main loop of the "join" function, on first pass the "futex" field points to the correct address. On second pass, the "futex" field always points to address 16908300 (in decimal).

I don't really know how this behavior can be reproduced.

Here is the Zig struct with the implementation.

const JsImpl = struct {
    worker: this.Worker,
    futex: *const std.atomic.Atomic(u32),

    const waiting: u32 = 0;
    const success: u32 = 1;

    fn join(self: JsImpl) void {
        // defer self.deinit();
        while (true) {
            // js.print("checking {} {}", .{ @ptrToInt(self.futex), 65536 * @wasmMemorySize(0) });
            // on first pass, the futex points to the correct address. on second pass, address always becomes 16908300.
            if (self.futex.load(.Acquire) != waiting) break;
            Thread.Futex.wait(self.futex, waiting);
        }
    }
}

Here is the generated LLVM IR

define internal fastcc void @main.JsImpl.join(ptr nonnull readonly align 4 %0) unnamed_addr #0 !dbg !2099 {
Entry:
  %1 = alloca ptr, align 4
  %2 = alloca i32, align 4
  %3 = alloca i3, align 1
  %4 = alloca ptr, align 4
  %5 = alloca %main.JsImpl, align 4
  call void @llvm.dbg.declare(metadata ptr %0, metadata !2101, metadata !DIExpression()), !dbg !2102
  br label %Loop

Loop:                                             ; preds = %Block, %Entry
  call void @llvm.memcpy.p0.p0.i32(ptr align 4 %5, ptr align 4 %0, i32 8, i1 false), !dbg !2103
  %6 = getelementptr inbounds %main.JsImpl, ptr %5, i32 0, i32 1, !dbg !2107
  %7 = load ptr, ptr %6, align 4, !dbg !2108
  store ptr %7, ptr %4, align 4, !dbg !2108
  call void @llvm.dbg.declare(metadata ptr %4, metadata !2109, metadata !DIExpression()), !dbg !2114
  store i3 2, ptr %3, align 1, !dbg !2108
  call void @llvm.dbg.declare(metadata ptr %3, metadata !2113, metadata !DIExpression()), !dbg !2114
  store ptr %7, ptr %1, align 4, !dbg !2115
  %8 = load ptr, ptr %1, align 4, !dbg !2117
  %9 = getelementptr inbounds %"atomic.Atomic.Atomic(u32)", ptr %8, i32 0, i32 0, !dbg !2117
  %10 = load atomic i32, ptr %9 acquire, align 4, !dbg !2117
  store i32 %10, ptr %2, align 4, !dbg !2117
  %11 = load i32, ptr %2, align 4, !dbg !2119
  %12 = icmp ne i32 %11, 0, !dbg !2119
  br i1 %12, label %Then, label %Else, !dbg !2119

Then:                                             ; preds = %Loop
  br label %Block1, !dbg !2119

Else:                                             ; preds = %Loop
  br label %Block, !dbg !2119

Block:                                            ; preds = %Else
  %13 = getelementptr inbounds %main.JsImpl, ptr %0, i32 0, i32 1, !dbg !2120
  %14 = load ptr, ptr %13, align 4, !dbg !2120
  call fastcc void @futex.wait(ptr %14, i32 0), !dbg !2121
  br label %Loop, !dbg !2121

Block1:                                           ; preds = %Then
  ret void, !dbg !2121
}

Note Thread is not the standard library thread, it's a custom Wasm thread, with the futex methods calling imported JavaScript atomic functions.

Expected Behavior

The contents of the "futex" field should had never changed, since it's passed by value and not by reference.

Aandreba avatar Jun 12 '23 12:06 Aandreba

The contents of the "futex" field should had never changed, since it's passed by value and not by reference.

This is not true. Zig is allowed to optimize function parameters to be passed by reference. See also #5973

IntegratedQuantum avatar Jun 13 '23 10:06 IntegratedQuantum

Zig is allowed to pass by reference as an optimization but if this results in different observable behaviour then it is a bug.

leecannon avatar Jun 13 '23 11:06 leecannon

I reckon this may be a problem with Zig assuming it's working on a single-threaded enviroment (which is currently always assumed in Wasm, with no way to opt-out) and doing a bunch of optimizations that break on multi-threaded enviroments.

Aandreba avatar Jun 14 '23 16:06 Aandreba

@Aandreba What are the exact command line arguments to reproduce? Zig version etc?

matu3ba avatar Jun 17 '23 08:06 matu3ba

Zig version is listed at the top of the issue (0.11.0-dev.3380+7e0a02ee2), and the code is compiled with this options in the build file

example.shared_memory = true;
example.import_memory = true;
example.max_memory = 2 * (1 << 30); // 2 GiB
example.linkage = .dynamic;
example.rdynamic = true;

Aandreba avatar Jun 19 '23 07:06 Aandreba