emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.91k stars 3.32k forks source link

ASYNCIFY_PROPAGATE_ADD does not work with -O1 and above #23015

Open kichikuou opened 1 week ago

kichikuou commented 1 week ago
$ emcc --version
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.72 (437140d149d9c977ffc8b09dbaf9b0f5a02db190)

asyncify_test.c:

#include <stdio.h>
#include <emscripten.h>

void (*indirect_sleep)(unsigned int) = &emscripten_sleep;

EMSCRIPTEN_KEEPALIVE
void my_sleep() {
    puts("sleeping");
    indirect_sleep(100);
    puts("awake");
}

int main() {
    my_sleep();
    my_sleep();
    return 0;
}
$ emcc -o asyncify_test.js asyncify_test.c \
       -O1 \
       -sASYNCIFY=1 \
       -sASYNCIFY_IGNORE_INDIRECT=1 \
       -sASYNCIFY_PROPAGATE_ADD=1 \
       -sASYNCIFY_ADD=my_sleep \
       -sASYNCIFY_ADVISE=1

[asyncify] emscripten_sleep is an import that can change the state
[asyncify] my_sleep is in the add-list, add

Note that main() is not instrumented even though ASYNCIFY_PROPAGATE_ADD is enabled.

The generated asyncify_test.js causes an error when executed:

$ node asyncify_test.js
sleeping
awake
sleeping
Aborted(invalid state: 1)
/Users/k/asyncify-test/asyncify_test.js:418
  var e = new WebAssembly.RuntimeError(what);
          ^

RuntimeError: Aborted(invalid state: 1). Build with -sASSERTIONS for more info.
    at abort (/Users/k/asyncify-test/asyncify_test.js:418:11)
    at Object.handleSleep (/Users/k/asyncify-test/asyncify_test.js:1106:11)
    at _emscripten_sleep (/Users/k/asyncify-test/asyncify_test.js:751:23)
    at wasm://wasm/adf2820a:wasm-function[6]:0x38d
    at wasm://wasm/adf2820a:wasm-function[7]:0x39b
    at ret.<computed> (/Users/k/asyncify-test/asyncify_test.js:924:24)
    at Module._main (/Users/k/asyncify-test/asyncify_test.js:1130:90)
    at callMain (/Users/k/asyncify-test/asyncify_test.js:1169:15)
    at doRun (/Users/k/asyncify-test/asyncify_test.js:1208:23)
    at run (/Users/k/asyncify-test/asyncify_test.js:1221:5)

Node.js v20.11.1

It works as expected when built with -O0:

$ emcc -o asyncify_test.js asyncify_test.c \
       -O0 \
       -sASYNCIFY=1 \
       -sASYNCIFY_IGNORE_INDIRECT=1 \
       -sASYNCIFY_PROPAGATE_ADD=1 \
       -sASYNCIFY_ADD=my_sleep \
       -sASYNCIFY_ADVISE=1

[asyncify] emscripten_sleep is an import that can change the state
[asyncify] my_sleep is in the add-list, add
[asyncify] __original_main can change the state due to my_sleep
[asyncify] main can change the state due to __original_main

$ node asyncify_test.js
sleeping
awake
sleeping
awake

Removing -sASYNCIFY_IGNORE_INDIRECT=1 also fixes the issue.

curiousdannii commented 1 week ago

Asyncify runs after function inlining. You can use ASYNCIFY_ADVISE to tell you which functions would be instrumented without ASYNCIFY_IGNORE_INDIRECT=1 at the specified optimisation level, but it can't be relied upon for other optimisation levels.

I'd recommend running first with: -O1 -sASYNCIFY=1 -sASYNCIFY_ADVISE=1. That will tell you which functions have indirect function calls, which should include main if my_sleep is being inlined.

(I'm assuming that my_sleep can still be inlined despite it being defined with EMSCRIPTEN_KEEPALIVE. If that's not the case, then I don't have an explanation for why main isn't being propagated.)

kichikuou commented 6 days ago

Thank you for the explanation. Based on the documentation, this behavior still appears to deviate from what is expected, so I continue to believe this might be a bug.

curiousdannii commented 6 days ago

While unexpected, I don't think it's a bug. If main never calls my_sleep, then my_sleep's instrumentation status can't be propagated to it.

The docs do say:

You can enable the ASYNCIFY_ADVISE setting, which will tell the compiler to output which functions it is currently instrumenting and why. You can then determine whether you should add any functions to ASYNCIFY_REMOVE or whether it would be safe to enable ASYNCIFY_IGNORE_INDIRECT. Note that this phase of the compiler happens after many optimization phases, and several functions maybe be inlined already. To be safe, run it with -O0.

Which I think is misleading advice... because of inlining you really need to do the analysis on the final optimisation level you intend to do. (Haha, I just went digging and see that I wrote that paragraph of the docs! I should submit a PR to update it.)