WebAssembly/wasi-sdk

`__main_argc_argv` is not retained in main module linked with `libc.so`

Opened this issue · 8 comments

kateinoigakukun commented

I'm trying to add shared-everything support in WebAssembly port of CRuby, and found an issue.

Dependencies

  • wasm-tools: with my patch to fix _start issue
  • wasi-sdk: The latest main 3175485
    • gh run download 7227946508 --repo WebAssembly/wasi-sdk --name dist-ubuntu-bionic

Repro

$ curl -LO https://github.com/bytecodealliance/wasmtime/releases/download/v15.0.0/wasi_snapshot_preview1.command.wasm
$ cat main.c
int main(int argc, char **argv) {
    return 0;
}

$ $WASI_SDK_PATH/bin/clang main.c -fPIC -Xlinker -pie -Xlinker --import-memory -Xlinker --experimental-pic -o a.out
$ wasm-tools component link a.out $WASI_SDK_PATH/share/wasi-sysroot/lib/wasm32-wasi/libc.so --adapt wasi_snapshot_preview1.command.wasm -o a.out.wasm
$ wasmtime run --wasm component-model a.out.wasm
Error: failed to run main module `a.out.wasm`

Caused by:
    0: failed to invoke `run` function
    1: error while executing at wasm backtrace:
           0: 0x1417c - wit-component:stubs!<wasm function 0>
           1: 0x1f43a - libc.so!__main_void
           2: 0xae76c - a.out!_start
           3: 0xb05df - wit-component:shim!adapt-a.out-_start
           4: 0x5220 - wit-component:adapter:wasi_snapshot_preview1!wasi:cli/run@0.2.0-rc-2023-11-10#run
       note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable may show more debugging information
    2: wasm trap: wasm `unreachable` instruction executed

The main problem here is:

  1. __main_argc_argv is defined with hidden visibility
$ wasm-objdump -x main.o | grep "F <__main_argc_argv>"
 - 0: F <__main_argc_argv> func=1 [ binding=global vis=hidden ]
  1. __main_argc_argv is referenced from libc.so's __main_void (which is referenced from crt1-command.o's _start)
    but the use of __main_argc_argv in libc.so doesn't mark it live.
    With static-link, libc.a, crt1-command.o, and main.o are linked together into a single image, so symbols are fully resolved.

Options

There are several ways to resolve the issue. I prefer option 3 as an ideal solution, but I'd like to hear @dicej opinion before tackling it.

Option 1. Adding -Xlinker -export-if-defined=__main_argc_argv to clang command.

This is a tentative workaround without toolchain modification.

Option 2. Set __main_argc_argv's visibility as default in clang

  • Need to update other compilers that use wasi-libc like Swift
  • The symbol will be leaked to other images

Option 3. Move __main_void to crt1-command.o

  • Linking __main_void into the main module cleanly solves the liveness issue.
  • According to the comment, something wrong happens with --no-gc-sections.
sbc100 commented

I think option 3 makes the most sense.

  • 👍1
dicej commented

Option 3 sounds good to me, too, but see also WebAssembly/wasi-libc#429 (comment) for the discussion regarding --no-gc-sections. @yamt may have opinions about this.

sbc100 commented

Yes, we would have to find a way to not break --no-gc-sections of course.

yamt commented

i have been using Option 1 if it matters.
https://github.com/yamt/garbage/blob/fbc9f79d77243940251a8826f0eba67cf986bf76/c/shlib/wasi.sh#L64-L72

as you can see in the above url, i needed many non-default linker options to build a reasonable pie executable. --export-if-defined=__main_argc_argv was merely one of them. isn't it the case for you?

yamt commented

Option 2. Set __main_argc_argv's visibility as default in clang

* Need to update other compilers that use wasi-libc like Swift

does swift use llvm wasm-ld?

* The symbol will be leaked to other images

can you explain a bit?

kateinoigakukun commented

@yamt

i have been using Option 1 if it matters.

https://github.com/yamt/garbage/blob/fbc9f79d77243940251a8826f0eba67cf986bf76/c/shlib/wasi.sh#L64-L72

That's good to know, thanks for sharing.

as you can see in the above url, i needed many non-default linker options to build a reasonable pie executable. --export-if-defined=__main_argc_argv was merely one of them. isn't it the case for you?

Right, it's one of tweaks but I think it would be better if we can reduce the number of such tweaks that is needed due to toolchain internal implementation.

Also we actually don't need to "export" the entry point but just need to include it in the final linked module.
I don't think it causes any real problem at this moment but would be better to keep exported stuff minimal.

Option 2. Set __main_argc_argv's visibility as default in clang

* Need to update other compilers that use wasi-libc like Swift

does swift use llvm wasm-ld?

Yes, we are using LLVM and wasm-ld, and doing main entry point codegen like what clang does.
https://github.com/apple/swift/blob/a6c98879abefd42d67aeeb3394784281061d1647/lib/AST/ASTContext.cpp#L5989

* The symbol will be leaked to other images

can you explain a bit?

If we made __main_argc_argv's visibility as __attribute__((visibility(default))), the entry point would be visible from other shared libraries. As I said for Option 1, I don't think it will be a problem soon, but exposing a symbol is not an ideal side-effect just for retaining the symbol.

kateinoigakukun commented

Anyway, I will trace the problem of --no-gc-section to see if Option 3 is a realistic option

yamt commented

as you can see in the above url, i needed many non-default linker options to build a reasonable pie executable.

my memory was a bit wrong.
actually,
flags for pie executable: https://github.com/yamt/garbage/blob/6c69da7068c4f61d95b38a7642693c646985f550/c/shlib/wasi.sh#L59-L68
flags for non-pie executable: https://github.com/yamt/garbage/blob/6c69da7068c4f61d95b38a7642693c646985f550/c/shlib/wasi.sh#L75-L91