[PATCH] D79300: [ELF] Demote lazy symbols relative to a discarded section to Undefined

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 12 06:24:53 PDT 2020


grimar added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:1124
+      // lazy symbol fetch. We should demote the lazy symbol to an Undefined.
+      if (sym->isLazy() && sym->file->symbols.empty())
+        sym->replace(und);
----------------
I am thinking about adding some flag like `isFetched` or alike to `InputFile` instead of using `symbols.empty()` this and in other places.
(Or perhaps only for ArchiveFile/LazyObjFile`)
It should look cleaner and avoid hacks like `push_back(nullptr)`. What do you think?


================
Comment at: lld/ELF/InputFiles.cpp:1590
+  // LazyObjFile has been processed.
+  symbols.clear();
 
----------------
You could use `std::swap(file->symbols, symbols)` instead I think.


================
Comment at: lld/test/ELF/comdat-discarded-lazy.s:11
+## Test the case when the symbol causing a "discarded section" is ordered
+## *before* the symbol fetching the lazy object.
+# RUN: echo '.globl f2, aa; f2: call aa; \
----------------
I think it should mention somewhere why it is ordered before/after.
I.e. that symbols are normally ordered by name in the symbol table.


================
Comment at: lld/test/ELF/comdat-discarded-lazy.s:18
+# RUN: rm -f %taa.a && llvm-ar rc %taa.a %taa.o
+# RUN: not ld.lld %t.o --start-lib %t1.o --end-lib %taa.a -o /dev/null 2>&1 | FileCheck --check-prefix=AA %s
+
----------------
Perhaps `AA`->`BEFORE`, `ZZ`->`AFTER`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79300





More information about the llvm-commits mailing list