[llvm-bugs] [Bug 51702] New: wasm-ld __wasm_init_memory calls atomic.wait which will fail on the main thread or in (Audio)Worklets

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=51702

            Bug ID: 51702
           Summary: wasm-ld __wasm_init_memory calls atomic.wait which
                    will fail on the main thread or in (Audio)Worklets
           Product: lld
           Version: unspecified
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: wasm
          Assignee: unassignedbugs at nondot.org
          Reporter: tibor.klajnscek at gmail.com
                CC: llvm-bugs at lists.llvm.org, sbc at chromium.org

When compiling with `sharedMemory` wasm-ld emits the __wasm_init_memory memory
function and runs it via the `start` section. The function is emitted in
"lld/wasm/Writer.cpp" -> Writer::createInitMemoryFunction()

The function is responsible for initializing the memory from the data segments
stored in the WASM. 

The first thread to enter will acquire the lock via atomic RMW, setting the
guard value from 0 to 1, then proceed to initialize the memory, then set it to
2 at the end.

Any other threads that enter will fail the RMW and attempt to `atomic.wait` on
the guard value with the expected value of 1. If it's already 2 the wait
doesn't block and the thread carries on as usual.

Now this works fine in regular WebWorkers, but we've hit a problem with the
current design when using WASM in AudioWorkletGlobalScope where atomics.wait is
not allowed to be called. The memory is already initialized on the main thread
so the wait will never block, but it still gets called and immediately traps.

Furthermore, this will also fail if the the WASM is initialized in a Worker
first, then instantiated on the main thread, where atomics.wait is not allowed
either. AFAICT this can never be the case when running with the current
Emscripten JS boilerplate, but it's easy enough to hit when working with the
compiled WASM manually.

I think the best solution is to replace the notify/wait pair with a simple spin
lock. It will work correctly in all contexts and the lock duration for the
memory init should be very short in any case so I think this makes sense.

Effectively, what I'm proposing is changing this:
>(if
> (i32.atomic.rmw.cmpxchg align=2 offset=0 (i32.const $__init_memory_flag) (i32.const 0) (i32.const 1))
> (then
>  (drop
>   (i32.atomic.wait align=2 offset=0 (i32.const $__init_memory_flag) (i32.const 1) (i32.const -1)
>   )
>  )
> )
> (else
>  ( ... initialize data segments ... )
>  (i32.atomic.store align=2 offset=0 (i32.const $__init_memory_flag) (i32.const 2)
>  )
>  (drop
>   (i32.atomic.notify align=2 offset=0 (i32.const $__init_memory_flag)(i32.const -1u)
>   )
>  )
> )
>)

To this:
>(if
> (i32.atomic.rmw.cmpxchg align=2 offset=0 (i32.const $__init_memory_flag) (i32.const 0) (i32.const 1)
> )
> (then
>  (loop
>   (br_if 1
>    (i32.eq
>     (i32.atomic.load  align=2 offset=0 (i32.const $__init_memory_flag))
>     (i32.const 2)
>    )
>   )
>  )
> )
> (else
>  ( ... initialize data segments ... )
>  (i32.atomic.store align=2 offset=0 (i32.const $__init_memory_flag) (i32.const 2)
>  )
> )
>)

I've made the change to the source code and I'll post a link to the patch for
review once I have it up.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210901/05aafc1e/attachment.html>