jhuber6 marked an inline comment as done. jhuber6 added a comment. In D142484#4077979 <https://reviews.llvm.org/D142484#4077979>, @tra wrote:
> We could also use more test cases. E.g. weak symbols (should not cause object > extraction) Yeah, I'll try to add a reasonable test here. It's a little difficult due to the indirect nature of this however. ================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1217-1218 +/// 1) It defines an undefined symbol in a regular object filie. +/// 2) It defines a global symbol without hidden visibility that has not +/// yet been defined. +Expected<bool> getSymbols(StringRef Image, StringSaver &Saver, ---------------- tra wrote: > jhuber6 wrote: > > tra wrote: > > > jhuber6 wrote: > > > > tra wrote: > > > > > How do we handle conflicting symbols defined more than once? > > > > > > > > > I just assume that will be caught by the actual linker, it's not > > > > relevant for knowing which symbols to pull out. > > > That would be the case when we were loading all libraries at once. > > > However, now that we load them as needed one by one, we may end up > > > loading only the first file which provides such symbol and would > > > potentially ignore others, if no other symbol requires loading them. > > > Given that we're checking the symbols anyways, it would be useful to > > > diagnose it, IMO. > > This is the expected behavior of static libraries right? `lld` has some > > option to print out why a certain library was extracted, we could do > > something similar? > Yes, it is expected. It's also a common source of surprises. I'm not sure > whether lld diagnoses it these days. We should follow whatever it does. > I just tested it with the same library defining a global `x` for `main.c`. ``` > clang main.c libx.a libx.a libx.a -Wl,--why-extract=- reference extracted symbol /tmp/main-8a5566.o libx.a(x.o) x ``` I could maybe add a similar option here to at least let us know if it extracts. Maybe for some tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142484/new/ https://reviews.llvm.org/D142484 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
