[PATCH] D69607: Add a feature to explain why some file gets included to the linker's output

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

I agree with Nico that it would be nice if this could take symbol names as well. Maybe the `--explain`` output could be added to the output of `--trace`?



================
Comment at: lld/ELF/Explain.cpp:95
+
+  // Collect root objects.
+  for (InputFile *f : objectFiles)
----------------
Hmm, object files aren't really gc roots, so this could give misleading output (e.g. `ld a.o b.o --explain a` with a.o containing `a: ret` and b.o containing `b: call a` with only b in dynsym won't report b.o). Also, it misses archives included with `--whole-archive`.

Shouldn't this be more similar to MarkLive.cpp? In other words, include dynsym and don't treat object files as roots.


================
Comment at: lld/ELF/Explain.cpp:125
+
+      if (sec->areRelocsRela) {
+        for (const typename ELFT::Rela &rel : sec->template relas<ELFT>())
----------------
Shouldn't it also look at `dependentSections`?


================
Comment at: lld/ELF/Explain.cpp:167
+  auto *sym = dyn_cast_or_null<Defined>(symtab->find(symName));
+  if (!sym || sym->isWeak())
+    return;
----------------
I don't think the `isWeak` part here is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69607