[PATCH] D120640: [ELF] Set the priority of STB_GNU_UNIQUE the same as STB_WEAK
Peter Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 28 10:07:09 PST 2022
peter.smith added inline comments.
================
Comment at: lld/ELF/Symbols.cpp:511
// Compare two symbols. Return true if the new symbol should win.
bool Symbol::compare(const Defined &other) const {
if (LLVM_UNLIKELY(isCommon())) {
----------------
Compare is probably a bit generic a name. I think it is only called from resolveDefined(). Perhaps `shouldReplace`.
================
Comment at: lld/ELF/Symbols.cpp:536
+ // the the incoming copy in a non-prevailing COMDAT is selected.
+ return !isGlobal() && other.isGlobal();
}
----------------
Presumably an existing `STB_GNU_UNIQUE` is also preferred over an incoming `STB_WEAK` because if it exists it must have come from a prevailing COMDAT?
May be worth adding that to the comment?
================
Comment at: lld/test/ELF/comdat-binding.s:12
+# RUN: ld.lld -e 0 %t/w.o %t/g.o -o %t/w
+# RUN: llvm-readelf -s %t/w | FileCheck %s --check-prefix=WEAK
+
----------------
If I've understood this run, we've got a prevailing group from w.o so we'd expect to see the STB_WEAK symbol and not the STB_GLOBAL. Although looking at reason the code was added (to stop a STB_GNU_UNIQUE from a non prevailing group replacing a STB_WEAK), I'm wondering why as `return !isGlobal() && other.isGlobal();` would imply that it should. Perhaps I need to understand https://reviews.llvm.org/D120626 first?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120640/new/
https://reviews.llvm.org/D120640
More information about the llvm-commits
mailing list