[PATCH] D79248: [lld][WebAssembly] Implement --unresolved-symbols

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D79248#2397526 <https://reviews.llvm.org/D79248#2397526>, @sbc100 wrote:

> In D79248#2396409 <https://reviews.llvm.org/D79248#2396409>, @MaskRay wrote:
>
>> In ELF, `--unresolved-symbols=` sets `--[no-]allow-shlib-undefined` and `-z (un)defs` simultaneously.
>>
>> - `--[no-]allow-shlib-undefined`: whether an unresolved undefined symbol from a shared object should be errored
>> - `-z (un)defs`:  whether an unresolved undefined symbol from a regular object should be errored.
>>
>> Does the wasm usage fit well in the existing practice?
>
> The support for shared libraries in wasm-ld, and indeed the entire PIC ABI,  is experimental (in fact, it will warn until you also pass `--experimental-pic`).
>
> In particular, support for linking against shared libraries does not yet
>
> In D79248#2396409 <https://reviews.llvm.org/D79248#2396409>, @MaskRay wrote:
>
>> In ELF, `--unresolved-symbols=` sets `--[no-]allow-shlib-undefined` and `-z (un)defs` simultaneously.
>>
>> - `--[no-]allow-shlib-undefined`: whether an unresolved undefined symbol from a shared object should be errored
>> - `-z (un)defs`:  whether an unresolved undefined symbol from a regular object should be errored.
>>
>> Does the wasm usage fit well in the existing practice?
>
> Yes.   While we don't currently support either of those flags I believe the implementation of `ignore-all` here will match `-z defs`, so I think that adding support for `-z (un)defs` would be trivial if decide do add that flag in the future.
>
> Regarding  `--[no-]allow-shlib-undefined`, our shared library support is still experimental is rather limited.  Using it currently requires the `--experimental-pic` flag.
>
> BTW, looking at the ELF code I don't see `--unresolved-symbols` currently effecting `--allow-shlib-undefined.
>
> In ELF/Driver.cpp we have:
>
>   config->allowShlibUndefined =                                                  
>         args.hasFlag(OPT_allow_shlib_undefined, OPT_no_allow_shlib_undefined,                        
>                      args.hasArg(OPT_shared));
>
> And this doesn't look like it gets updated based on `--unresolved-symbols`.

You are right. I knew this when investigating https://reviews.llvm.org/D67479#1667256 
It is of very low priority because `--unresolved-symbols=` has too few use cases. Now wasm is adding this option I'll make the ELF side semantics clearer: D91510 <https://reviews.llvm.org/D91510>


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79248/new/

https://reviews.llvm.org/D79248