[PATCH] D100624: [ELF] Don't set versionId on undefined weak lazy symbols

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D100624#2701483 <https://reviews.llvm.org/D100624#2701483>, @peter.smith wrote:

> LGTM.

Thanks for the review.

> If I've got this right:
>
> - We assign versionid before LTO code generation
> - We assign versionid to lazy symbols from script in case LTO fetches the symbol
> - If the lazy symbol is not referenced we want to give it VER_NDX_GLOBAL so we have to do this at write time.
>
> I'm trying to think if there is any reason we can't delay scanVersionScript() till after LTO code generation. If we could do that then we'd know which symbols were lazy and we wouldn't need to assign them a versionid. I don't know off the top of my head how much work it would be to check that though.

If `scanVersionScript()` is before LTO code generation, these tests will fail.

  Failed Tests (6):
    lld :: ELF/a.s
    lld :: ELF/lto/devirt_vcall_vis_export_dynamic.ll
    lld :: ELF/lto/devirt_vcall_vis_localize.ll
    lld :: ELF/lto/dynamic-list.ll
    lld :: ELF/lto/internalize-version-script.ll
    lld :: ELF/lto/version-libcall.ll

We handle dynamic lists in `scanVersionScript`, so the `includeInDynsym` and `inDynamicList` properties can be incorrect in `LTO.cpp BitcodeCompiler::add`.
A bit recently, I added `sym->computeBinding() != STB_LOCAL` in D98220 <https://reviews.llvm.org/D98220> so the whole program devirtualization pass will lose information as well.

So we need a `scanVersionScript` before `compileBitcodeFiles`. If we simply call `scanVersionScript` another time after `compileBitcodeFiles`, we will get `error: attempt to reassign symbol 'foo' of version 'VER2' to version 'VER1'` from `version-script-symver2.s`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100624