[PATCH] D44336: Implement --cref.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 14 12:48:12 PDT 2018


ruiu added a comment.

> Why B? Is there a --no-cref?

Yes, gold has that.

> GNU spec says "If a linker map file is being generated, the cross reference table is printed to the map file.", I checked that bfd do that.  Should we do that too?

I think we shouldn't. Our map file format is different from GNU, but this -cref format is compatible, and mixing the two produces an output that can't be parsed by existing tools.



================
Comment at: lld/ELF/MapFile.cpp:174
+      if (auto *Sym = dyn_cast<Defined>(B))
+        if (!Sym->isLocal() || (!Sym->Section || Sym->Section->Live))
+          Map[Sym].insert(File);
----------------
grimar wrote:
> Your test pass with this line removed.
Fixed.


================
Comment at: lld/ELF/MapFile.cpp:190
+    Symbol *Sym = KV.first;
+    SetVector<InputFile *> &Files = KV.second;
+    Print(toString(*Sym), toString(Sym->File));
----------------
grimar wrote:
> I would move `Files` below `Print`.
I wouldn't because I want to make the type explicit as the beginning of this loop.


================
Comment at: lld/ELF/MapFile.cpp:193
+
+    for (InputFile *File : Files)
+      if (File != Sym->File)
----------------
grimar wrote:
> Does it make sense to sort `Files` by name? Can be convenient probably. (BFD does **not ** do that FWIW).
I was thinking about that, but I decided to not. I can convinced the other way, but I feel like sorting files alphabetically doesn't make much sense in cref file.


https://reviews.llvm.org/D44336





More information about the llvm-commits mailing list