Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kateinoigakukun opened this issue Dec 17, 2023 · 8 comments
Open

Comments

@kateinoigakukun
Copy link
Contributor

kateinoigakukun commented Dec 17, 2023

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

Dependencies

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/[email protected]#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
Copy link
Member

sbc100 commented Dec 18, 2023

I think option 3 makes the most sense.

@dicej
Copy link
Contributor

dicej commented Dec 18, 2023

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
Copy link
Member

sbc100 commented Dec 18, 2023

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

@yamt
Copy link
Contributor

yamt commented Dec 19, 2023

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
Copy link
Contributor

yamt commented Dec 19, 2023

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
Copy link
Contributor Author

kateinoigakukun commented Dec 19, 2023

@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
Copy link
Contributor Author

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

@yamt
Copy link
Contributor

yamt commented Dec 21, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants