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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 18 14:24:43 PST 2022


MaskRay added inline comments.


================
Comment at: lld/ELF/Driver.cpp:2415
+  // added at this point.
+  parallelForEach(objectFiles, [](ELFFileBase *file) {
+    switch (config->ekind) {
----------------
ikudrin wrote:
> 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?
> If I understand it right, the object files for LTO cannot introduce new duplicate symbols, no?

Ideally, yes. Due to some properties like isUsedInRegularObj, it needs more thoughts whether the loop can be omitted. Perhaps do this omission after the changes are proven to be stable.


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