[PATCH] D119908: [ELF] Move duplicate symbol check after input file parsing

Igor Kudrin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 17 02:14:04 PST 2022


ikudrin added inline comments.


================
Comment at: lld/ELF/Driver.cpp:2415
+  // added at this point.
+  parallelForEach(objectFiles, [](ELFFileBase *file) {
+    switch (config->ekind) {
----------------
If this is moved next to the loop for bitcode files, it should detect all the errors and avoid not-that-helpful error messages like
```
ld.lld: error: duplicate symbol: f
>>> defined at ld-temp.o
>>>            lto.tmp:(f)
>>> defined at ...test\ELF\lto\Output\duplicated.ll.tmp/b.o:(.text+0x0)
```
If I understand it right, the object files for LTO cannot introduce new duplicate symbols, no?

As for future extensions of the `postParse()` method, which should also be applied to the LTO-produced object files, we can have an additional list of them and run `postParse()` only for them, separately, after `compileBitcodeFiles()`.


================
Comment at: lld/ELF/Driver.cpp:2415
+  // added at this point.
+  parallelForEach(objectFiles, [](ELFFileBase *file) {
+    switch (config->ekind) {
----------------
ikudrin wrote:
> If this is moved next to the loop for bitcode files, it should detect all the errors and avoid not-that-helpful error messages like
> ```
> ld.lld: error: duplicate symbol: f
> >>> defined at ld-temp.o
> >>>            lto.tmp:(f)
> >>> defined at ...test\ELF\lto\Output\duplicated.ll.tmp/b.o:(.text+0x0)
> ```
> If I understand it right, the object files for LTO cannot introduce new duplicate symbols, no?
> 
> As for future extensions of the `postParse()` method, which should also be applied to the LTO-produced object files, we can have an additional list of them and run `postParse()` only for them, separately, after `compileBitcodeFiles()`.
Why not add a function like `postParseObjectFile()` instead of the lambda?


================
Comment at: lld/ELF/Symbols.cpp:653-656
+  // else if (cmp == 0)
+  //   reportDuplicate(this, other.file,
+  //                   dyn_cast_or_null<InputSectionBase>(other.section),
+  //                   other.value);
----------------
The code should be removed, not commented out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119908



More information about the llvm-commits mailing list