Address review feedback

- LiveObject -> LiveItem
- nit: alive -> live
- Prefer "keeps live" to "(may) make live"
- Inline getParentSec lambda
- Always print symbol file
- reason.item guard clause
- Use /*offset=*/ and /*sym=*/ for enqueue
- Update ld.lld.1

Author: mysterymath

lld/ELF/MarkLive.cpp

@@ -48,11 +48,11 @@ namespace {
 using SecOffset = std::pair<InputSectionBase *, unsigned>;
 
 // Something that can have an independent reason for being live.
-using LiveObject = std::variant<InputSectionBase *, Symbol *, SecOffset>;
+using LiveItem = std::variant<InputSectionBase *, Symbol *, SecOffset>;
 
-// The most proximate reason that an object is live.
+// The most proximate reason that something is live.
 struct LiveReason {
-  std::optional<LiveObject> obj;
+  std::optional<LiveItem> item;
   StringRef desc;
 };
 
@@ -88,7 +88,7 @@ template <class ELFT, bool TrackWhyLive> class MarkLive {
   DenseMap<StringRef, SmallVector<InputSectionBase *, 0>> cNamedSections;
 
   // The most proximate reason that something is live.
-  DenseMap<LiveObject, LiveReason> whyLive;
+  DenseMap<LiveItem, LiveReason> whyLive;
 };
 } // namespace
 
@@ -164,7 +164,7 @@ void MarkLive<ELFT, TrackWhyLive>::resolveReloc(InputSectionBase &sec,
   }
 
   for (InputSectionBase *sec : cNamedSections.lookup(sym.getName()))
-    enqueue(sec, 0, nullptr, reason);
+    enqueue(sec, /*offset=*/0, /*sym=*/nullptr, reason);
 }
 
 // The .eh_frame section is an unfortunate special case.
@@ -240,12 +240,12 @@ void MarkLive<ELFT, TrackWhyLive>::enqueue(InputSectionBase *sec,
 
   if (TrackWhyLive) {
     if (sym) {
-      // If a specific symbol is referenced, that makes it alive. It may in turn
-      // make its section alive.
+      // If a specific symbol is referenced, that keeps it live. The symbol then
+      // keeps its section live.
       whyLive.try_emplace(sym, reason);
       whyLive.try_emplace(sec, LiveReason{sym, "contained live symbol"});
     } else {
-      // Otherwise, the reference generically makes the section live.
+      // Otherwise, the reference generically keeps the section live.
       whyLive.try_emplace(sec, reason);
     }
   }
@@ -268,57 +268,49 @@ void MarkLive<ELFT, TrackWhyLive>::printWhyLive(Symbol *s) const {
   auto msg = Msg(ctx);
 
   const auto printSymbol = [&](Symbol *s) {
-    if (s->isLocal())
-      msg << s->file << ":(" << s << ')';
-    else
-      msg << s;
+    msg << s->file << ":(" << s << ')';
   };
 
   msg << "live symbol: ";
   printSymbol(s);
 
-  LiveObject cur = s;
+  LiveItem cur = s;
   while (true) {
     auto it = whyLive.find(cur);
     LiveReason reason;
-    // If there is a specific reason this object is live...
+    // If there is a specific reason this item is live...
     if (it != whyLive.end()) {
       reason = it->second;
     } else {
-      // This object is live, but it has no tracked reason. It must be an
+      // This item is live, but it has no tracked reason. It must be an
       // unreferenced symbol in a live section or a symbol with no section.
-      const auto getParentSec = [&]() -> InputSectionBase * {
-        auto *d = dyn_cast<Defined>(std::get<Symbol *>(cur));
-        if (!d)
-          return nullptr;
-        return dyn_cast_or_null<InputSectionBase>(d->section);
-      };
-      InputSectionBase *sec = getParentSec();
+      InputSectionBase *sec = nullptr;
+      if (auto *d = dyn_cast<Defined>(std::get<Symbol *>(cur)))
+        sec = dyn_cast_or_null<InputSectionBase>(d->section);
       reason = sec ? LiveReason{sec, "in live section"}
                    : LiveReason{std::nullopt, "no section"};
     }
 
-    if (reason.obj) {
-      msg << "\n>>> " << reason.desc << ": ";
-      // The reason may not yet have been resolved to a symbol; do so now.
-      if (std::holds_alternative<SecOffset>(*reason.obj)) {
-        const auto &so = std::get<SecOffset>(*reason.obj);
-        InputSectionBase *sec = so.first;
-        Defined *sym = sec->getEnclosingSymbol(so.second);
-        cur = sym ? LiveObject(sym) : LiveObject(sec);
-      } else {
-        cur = *reason.obj;
-      }
-
-      if (std::holds_alternative<Symbol *>(cur))
-        printSymbol(std::get<Symbol *>(cur));
-      else
-        msg << std::get<InputSectionBase *>(cur);
-    }
-    if (!reason.obj) {
+    if (!reason.item) {
       msg << " (" << reason.desc << ')';
       break;
     }
+
+    msg << "\n>>> " << reason.desc << ": ";
+    // The reason may not yet have been resolved to a symbol; do so now.
+    if (std::holds_alternative<SecOffset>(*reason.item)) {
+      const auto &so = std::get<SecOffset>(*reason.item);
+      InputSectionBase *sec = so.first;
+      Defined *sym = sec->getEnclosingSymbol(so.second);
+      cur = sym ? LiveItem(sym) : LiveItem(sec);
+    } else {
+      cur = *reason.item;
+    }
+
+    if (std::holds_alternative<Symbol *>(cur))
+      printSymbol(std::get<Symbol *>(cur));
+    else
+      msg << std::get<InputSectionBase *>(cur);
   }
 }
 
@@ -374,7 +366,7 @@ void MarkLive<ELFT, TrackWhyLive>::run() {
   }
   for (InputSectionBase *sec : ctx.inputSections) {
     if (sec->flags & SHF_GNU_RETAIN) {
-      enqueue(sec, 0, nullptr, {std::nullopt, "retained"});
+      enqueue(sec, /*offset=*/0, /*sym=*/nullptr, {std::nullopt, "retained"});
       continue;
     }
     if (sec->flags & SHF_LINK_ORDER)
@@ -413,9 +405,10 @@ void MarkLive<ELFT, TrackWhyLive>::run() {
     // Preserve special sections and those which are specified in linker
     // script KEEP command.
     if (isReserved(sec)) {
-      enqueue(sec, 0, nullptr, {std::nullopt, "reserved"});
+      enqueue(sec, /*offset=*/0, /*sym=*/nullptr, {std::nullopt, "reserved"});
     } else if (ctx.script->shouldKeep(sec)) {
-      enqueue(sec, 0, nullptr, {std::nullopt, "KEEP in linker script"});
+      enqueue(sec, /*offset=*/0, /*sym=*/nullptr,
+              {std::nullopt, "KEEP in linker script"});
     } else if ((!ctx.arg.zStartStopGC || sec->name.starts_with("__libc_")) &&
                isValidCIdentifier(sec->name)) {
       // As a workaround for glibc libc.a before 2.34
@@ -460,11 +453,11 @@ void MarkLive<ELFT, TrackWhyLive>::mark() {
       resolveReloc(sec, rel, false);
 
     for (InputSectionBase *isec : sec.dependentSections)
-      enqueue(isec, 0, nullptr, {&sec, "dependent section"});
+      enqueue(isec, /*offset=*/0, /*sym=*/nullptr, {&sec, "dependent section"});
 
     // Mark the next group member.
     if (sec.nextInSectionGroup)
-      enqueue(sec.nextInSectionGroup, 0, nullptr,
+      enqueue(sec.nextInSectionGroup, /*offset=*/0, /*sym=*/nullptr,
               {&sec, "next in section group"});
   }
 }
@@ -492,7 +485,7 @@ void MarkLive<ELFT, TrackWhyLive>::moveToMain() {
       continue;
     if (ctx.symtab->find(("__start_" + sec->name).str()) ||
         ctx.symtab->find(("__stop_" + sec->name).str()))
-      enqueue(sec, 0, nullptr, {});
+      enqueue(sec, /*offset=*/0, /*sym=*/nullptr, /*reason=*/{});
   }
 
   mark();

lld/ELF/Options.td

@@ -568,16 +568,16 @@ defm whole_archive: B<"whole-archive",
 
 def why_extract: JJ<"why-extract=">, HelpText<"Print to a file about why archive members are extracted">;
 
-defm wrap : Eq<"wrap", "Redirect symbol references to __wrap_symbol and "
-                       "__real_symbol references to symbol">,
-            MetaVarName<"<symbol>">;
-
 defm why_live
     : EEq<"why-live",
           "Report a chain of references preventing garbage collection for "
           "each symbol matching <glob>">,
       MetaVarName<"<glob>">;
 
+defm wrap : Eq<"wrap", "Redirect symbol references to __wrap_symbol and "
+                       "__real_symbol references to symbol">,
+            MetaVarName<"<symbol>">;
+
 def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
   HelpText<"Linker option extensions">;
 

lld/docs/ld.lld.1

@@ -755,6 +755,8 @@ Report unresolved symbols as warnings.
 Force load of all members in a static library.
 .It Fl -why-extract Ns = Ns Ar file
 Print to a file about why archive members are extracted.
+.It Fl --why-live Ns = Ns Ar glob
+Report a chain of references preventing garbage collection for each symbol matching the glob.
 .It Fl -wrap Ns = Ns Ar symbol
 Redirect
 .Ar symbol

lld/test/ELF/why-live.s

@@ -24,8 +24,8 @@ test_simple:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_simple | FileCheck %s --check-prefix=SIMPLE
 
-# SIMPLE:      live symbol: test_simple
-# SIMPLE-NEXT: >>> referenced by: _start (entry point)
+# SIMPLE:      live symbol: {{.*}}.o:(test_simple)
+# SIMPLE-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # SIMPLE-NOT:  >>>
 
 ## Live only by being a member of .test_simple
@@ -35,10 +35,10 @@ test_incidental:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_incidental | FileCheck %s --check-prefix=INCIDENTAL
 
-# INCIDENTAL:      live symbol: test_incidental
+# INCIDENTAL:      live symbol: {{.*}}.o:(test_incidental)
 # INCIDENTAL-NEXT: >>> in live section: {{.*}}.o:(.test_simple)
-# INCIDENTAL-NEXT: >>> contained live symbol: test_simple
-# INCIDENTAL-NEXT: >>> referenced by: _start (entry point)
+# INCIDENTAL-NEXT: >>> contained live symbol: {{.*}}.o:(test_simple)
+# INCIDENTAL-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # INCIDENTAL-NOT:  >>>
 
 ## Reached from a reference in section .test_simple directly, since test_simple is an unsized symbol.
@@ -49,10 +49,10 @@ test_from_unsized:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_from_unsized | FileCheck %s --check-prefix=FROM-UNSIZED
 
-# FROM-UNSIZED:      live symbol: test_from_unsized
+# FROM-UNSIZED:      live symbol: {{.*}}.o:(test_from_unsized)
 # FROM-UNSIZED-NEXT: >>> referenced by: {{.*}}.o:(.test_simple)
-# FROM-UNSIZED-NEXT: >>> contained live symbol: test_simple
-# FROM-UNSIZED-NEXT: >>> referenced by: _start (entry point)
+# FROM-UNSIZED-NEXT: >>> contained live symbol: {{.*}}.o:(test_simple)
+# FROM-UNSIZED-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # FROM-UNSIZED-NOT:  >>>
 
 ## Symbols in dead sections are dead and not reported.
@@ -67,7 +67,7 @@ test_dead:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_undef -u test_undef | FileCheck %s --check-prefix=UNDEFINED
 
-# UNDEFINED:     live symbol: test_undef (no section)
+# UNDEFINED:     live symbol: <internal>:(test_undef) (no section)
 # UNDEFINED-NOT: >>>
 
 ## Defined symbols without input section parents are live.
@@ -76,7 +76,7 @@ test_absolute = 1234
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_absolute | FileCheck %s --check-prefix=ABSOLUTE
 
-# ABSOLUTE:     live symbol: test_absolute (no section)
+# ABSOLUTE:     live symbol: {{.*}}.o:(test_absolute) (no section)
 # ABSOLUTE-NOT: >>>
 
 ## Retained sections are intrinsically live, and they make contained symbols live.
@@ -87,7 +87,7 @@ test_retained:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_retained | FileCheck %s --check-prefix=RETAINED
 
-# RETAINED:      live symbol: test_retained
+# RETAINED:      live symbol: {{.*}}.o:(test_retained)
 # RETAINED-NEXT: >>> in live section: {{.*}}:(.test_retained) (retained)
 # RETAINED-NOT:  >>>
 
@@ -102,9 +102,9 @@ test_section_offset:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_section_offset | FileCheck %s --check-prefix=SECTION-OFFSET
 
-# SECTION-OFFSET:      live symbol: test_section_offset
+# SECTION-OFFSET:      live symbol: {{.*}}.o:(test_section_offset)
 # SECTION-OFFSET-NEXT: >>> in live section: {{.*}}:(.test_section_offset)
-# SECTION-OFFSET-NEXT: >>> referenced by: _start (entry point)
+# SECTION-OFFSET-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # SECTION-OFFSET-NOT:  >>>
 
 ## Relocs that reference offsets from sections (e.g., from anonymous symbols) are considered to point to the enclosing symbol if one exists.
@@ -119,8 +119,8 @@ test_section_offset_within_symbol:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_section_offset_within_symbol | FileCheck %s --check-prefix=SECTION-OFFSET-WITHIN-SYMBOL
 
-# SECTION-OFFSET-WITHIN-SYMBOL:      live symbol: test_section_offset_within_symbol
-# SECTION-OFFSET-WITHIN-SYMBOL-NEXT: >>> referenced by: _start (entry point)
+# SECTION-OFFSET-WITHIN-SYMBOL:      live symbol: {{.*}}.o:(test_section_offset_within_symbol)
+# SECTION-OFFSET-WITHIN-SYMBOL-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # SECTION-OFFSET-WITHIN-SYMBOL-NOT:  >>>
 
 ## Local symbols can be queried just like global symbols.
@@ -132,22 +132,22 @@ test_local:
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_local | FileCheck %s --check-prefix=LOCAL
 
-# LOCAL:      live symbol: {{.*}}:(test_local)
-# LOCAL-NEXT: >>> referenced by: _start (entry point)
+# LOCAL:      live symbol: {{.*}}.o:(test_local)
+# LOCAL-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # LOCAL-NOT:  >>>
 
 ## Shared symbols
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections %t.so --why-live=test_shared | FileCheck %s --check-prefix=SHARED
 
-# SHARED:      live symbol: test_shared
-# SHARED-NEXT: >>> referenced by: _start (entry point)
+# SHARED:      live symbol: {{.*}}.so:(test_shared)
+# SHARED-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
 # SHARED-NOT:  >>>
 
 ## Globs match multiple cases. Multiple --why-live flags union.
 
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections %t.so --why-live=test_s* | FileCheck %s --check-prefix=MULTIPLE
 # RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections %t.so --why-live=test_simple --why-live=test_shared | FileCheck %s --check-prefix=MULTIPLE
 
-# MULTIPLE-DAG: live symbol: test_simple
-# MULTIPLE-DAG: live symbol: test_shared
+# MULTIPLE-DAG: live symbol: {{.*}}.o:(test_simple)
+# MULTIPLE-DAG: live symbol: {{.*}}.so:(test_shared)