Skip to content

Commit 2772323

Browse files
committed
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
1 parent 4e52983 commit 2772323

File tree

4 files changed

+66
-71
lines changed

4 files changed

+66
-71
lines changed

lld/ELF/MarkLive.cpp

Lines changed: 39 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ namespace {
4848
using SecOffset = std::pair<InputSectionBase *, unsigned>;
4949

5050
// Something that can have an independent reason for being live.
51-
using LiveObject = std::variant<InputSectionBase *, Symbol *, SecOffset>;
51+
using LiveItem = std::variant<InputSectionBase *, Symbol *, SecOffset>;
5252

53-
// The most proximate reason that an object is live.
53+
// The most proximate reason that something is live.
5454
struct LiveReason {
55-
std::optional<LiveObject> obj;
55+
std::optional<LiveItem> item;
5656
StringRef desc;
5757
};
5858

@@ -88,7 +88,7 @@ template <class ELFT, bool TrackWhyLive> class MarkLive {
8888
DenseMap<StringRef, SmallVector<InputSectionBase *, 0>> cNamedSections;
8989

9090
// The most proximate reason that something is live.
91-
DenseMap<LiveObject, LiveReason> whyLive;
91+
DenseMap<LiveItem, LiveReason> whyLive;
9292
};
9393
} // namespace
9494

@@ -164,7 +164,7 @@ void MarkLive<ELFT, TrackWhyLive>::resolveReloc(InputSectionBase &sec,
164164
}
165165

166166
for (InputSectionBase *sec : cNamedSections.lookup(sym.getName()))
167-
enqueue(sec, 0, nullptr, reason);
167+
enqueue(sec, /*offset=*/0, /*sym=*/nullptr, reason);
168168
}
169169

170170
// The .eh_frame section is an unfortunate special case.
@@ -240,12 +240,12 @@ void MarkLive<ELFT, TrackWhyLive>::enqueue(InputSectionBase *sec,
240240

241241
if (TrackWhyLive) {
242242
if (sym) {
243-
// If a specific symbol is referenced, that makes it alive. It may in turn
244-
// make its section alive.
243+
// If a specific symbol is referenced, that keeps it live. The symbol then
244+
// keeps its section live.
245245
whyLive.try_emplace(sym, reason);
246246
whyLive.try_emplace(sec, LiveReason{sym, "contained live symbol"});
247247
} else {
248-
// Otherwise, the reference generically makes the section live.
248+
// Otherwise, the reference generically keeps the section live.
249249
whyLive.try_emplace(sec, reason);
250250
}
251251
}
@@ -268,57 +268,49 @@ void MarkLive<ELFT, TrackWhyLive>::printWhyLive(Symbol *s) const {
268268
auto msg = Msg(ctx);
269269

270270
const auto printSymbol = [&](Symbol *s) {
271-
if (s->isLocal())
272-
msg << s->file << ":(" << s << ')';
273-
else
274-
msg << s;
271+
msg << s->file << ":(" << s << ')';
275272
};
276273

277274
msg << "live symbol: ";
278275
printSymbol(s);
279276

280-
LiveObject cur = s;
277+
LiveItem cur = s;
281278
while (true) {
282279
auto it = whyLive.find(cur);
283280
LiveReason reason;
284-
// If there is a specific reason this object is live...
281+
// If there is a specific reason this item is live...
285282
if (it != whyLive.end()) {
286283
reason = it->second;
287284
} else {
288-
// This object is live, but it has no tracked reason. It must be an
285+
// This item is live, but it has no tracked reason. It must be an
289286
// unreferenced symbol in a live section or a symbol with no section.
290-
const auto getParentSec = [&]() -> InputSectionBase * {
291-
auto *d = dyn_cast<Defined>(std::get<Symbol *>(cur));
292-
if (!d)
293-
return nullptr;
294-
return dyn_cast_or_null<InputSectionBase>(d->section);
295-
};
296-
InputSectionBase *sec = getParentSec();
287+
InputSectionBase *sec = nullptr;
288+
if (auto *d = dyn_cast<Defined>(std::get<Symbol *>(cur)))
289+
sec = dyn_cast_or_null<InputSectionBase>(d->section);
297290
reason = sec ? LiveReason{sec, "in live section"}
298291
: LiveReason{std::nullopt, "no section"};
299292
}
300293

301-
if (reason.obj) {
302-
msg << "\n>>> " << reason.desc << ": ";
303-
// The reason may not yet have been resolved to a symbol; do so now.
304-
if (std::holds_alternative<SecOffset>(*reason.obj)) {
305-
const auto &so = std::get<SecOffset>(*reason.obj);
306-
InputSectionBase *sec = so.first;
307-
Defined *sym = sec->getEnclosingSymbol(so.second);
308-
cur = sym ? LiveObject(sym) : LiveObject(sec);
309-
} else {
310-
cur = *reason.obj;
311-
}
312-
313-
if (std::holds_alternative<Symbol *>(cur))
314-
printSymbol(std::get<Symbol *>(cur));
315-
else
316-
msg << std::get<InputSectionBase *>(cur);
317-
}
318-
if (!reason.obj) {
294+
if (!reason.item) {
319295
msg << " (" << reason.desc << ')';
320296
break;
321297
}
298+
299+
msg << "\n>>> " << reason.desc << ": ";
300+
// The reason may not yet have been resolved to a symbol; do so now.
301+
if (std::holds_alternative<SecOffset>(*reason.item)) {
302+
const auto &so = std::get<SecOffset>(*reason.item);
303+
InputSectionBase *sec = so.first;
304+
Defined *sym = sec->getEnclosingSymbol(so.second);
305+
cur = sym ? LiveItem(sym) : LiveItem(sec);
306+
} else {
307+
cur = *reason.item;
308+
}
309+
310+
if (std::holds_alternative<Symbol *>(cur))
311+
printSymbol(std::get<Symbol *>(cur));
312+
else
313+
msg << std::get<InputSectionBase *>(cur);
322314
}
323315
}
324316

@@ -374,7 +366,7 @@ void MarkLive<ELFT, TrackWhyLive>::run() {
374366
}
375367
for (InputSectionBase *sec : ctx.inputSections) {
376368
if (sec->flags & SHF_GNU_RETAIN) {
377-
enqueue(sec, 0, nullptr, {std::nullopt, "retained"});
369+
enqueue(sec, /*offset=*/0, /*sym=*/nullptr, {std::nullopt, "retained"});
378370
continue;
379371
}
380372
if (sec->flags & SHF_LINK_ORDER)
@@ -413,9 +405,10 @@ void MarkLive<ELFT, TrackWhyLive>::run() {
413405
// Preserve special sections and those which are specified in linker
414406
// script KEEP command.
415407
if (isReserved(sec)) {
416-
enqueue(sec, 0, nullptr, {std::nullopt, "reserved"});
408+
enqueue(sec, /*offset=*/0, /*sym=*/nullptr, {std::nullopt, "reserved"});
417409
} else if (ctx.script->shouldKeep(sec)) {
418-
enqueue(sec, 0, nullptr, {std::nullopt, "KEEP in linker script"});
410+
enqueue(sec, /*offset=*/0, /*sym=*/nullptr,
411+
{std::nullopt, "KEEP in linker script"});
419412
} else if ((!ctx.arg.zStartStopGC || sec->name.starts_with("__libc_")) &&
420413
isValidCIdentifier(sec->name)) {
421414
// As a workaround for glibc libc.a before 2.34
@@ -460,11 +453,11 @@ void MarkLive<ELFT, TrackWhyLive>::mark() {
460453
resolveReloc(sec, rel, false);
461454

462455
for (InputSectionBase *isec : sec.dependentSections)
463-
enqueue(isec, 0, nullptr, {&sec, "dependent section"});
456+
enqueue(isec, /*offset=*/0, /*sym=*/nullptr, {&sec, "dependent section"});
464457

465458
// Mark the next group member.
466459
if (sec.nextInSectionGroup)
467-
enqueue(sec.nextInSectionGroup, 0, nullptr,
460+
enqueue(sec.nextInSectionGroup, /*offset=*/0, /*sym=*/nullptr,
468461
{&sec, "next in section group"});
469462
}
470463
}
@@ -492,7 +485,7 @@ void MarkLive<ELFT, TrackWhyLive>::moveToMain() {
492485
continue;
493486
if (ctx.symtab->find(("__start_" + sec->name).str()) ||
494487
ctx.symtab->find(("__stop_" + sec->name).str()))
495-
enqueue(sec, 0, nullptr, {});
488+
enqueue(sec, /*offset=*/0, /*sym=*/nullptr, /*reason=*/{});
496489
}
497490

498491
mark();

lld/ELF/Options.td

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -555,16 +555,16 @@ defm whole_archive: B<"whole-archive",
555555

556556
def why_extract: JJ<"why-extract=">, HelpText<"Print to a file about why archive members are extracted">;
557557

558-
defm wrap : Eq<"wrap", "Redirect symbol references to __wrap_symbol and "
559-
"__real_symbol references to symbol">,
560-
MetaVarName<"<symbol>">;
561-
562558
defm why_live
563559
: EEq<"why-live",
564560
"Report a chain of references preventing garbage collection for "
565561
"each symbol matching <glob>">,
566562
MetaVarName<"<glob>">;
567563

564+
defm wrap : Eq<"wrap", "Redirect symbol references to __wrap_symbol and "
565+
"__real_symbol references to symbol">,
566+
MetaVarName<"<symbol>">;
567+
568568
def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
569569
HelpText<"Linker option extensions">;
570570

lld/docs/ld.lld.1

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,8 @@ Report unresolved symbols as warnings.
751751
Force load of all members in a static library.
752752
.It Fl -why-extract Ns = Ns Ar file
753753
Print to a file about why archive members are extracted.
754+
.It Fl --why-live Ns = Ns Ar glob
755+
Report a chain of references preventing garbage collection for each symbol matching the glob.
754756
.It Fl -wrap Ns = Ns Ar symbol
755757
Redirect
756758
.Ar symbol

lld/test/ELF/why-live.s

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ test_simple:
2424

2525
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_simple | FileCheck %s --check-prefix=SIMPLE
2626

27-
# SIMPLE: live symbol: test_simple
28-
# SIMPLE-NEXT: >>> referenced by: _start (entry point)
27+
# SIMPLE: live symbol: {{.*}}.o:(test_simple)
28+
# SIMPLE-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
2929
# SIMPLE-NOT: >>>
3030

3131
## Live only by being a member of .test_simple
@@ -35,10 +35,10 @@ test_incidental:
3535

3636
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_incidental | FileCheck %s --check-prefix=INCIDENTAL
3737

38-
# INCIDENTAL: live symbol: test_incidental
38+
# INCIDENTAL: live symbol: {{.*}}.o:(test_incidental)
3939
# INCIDENTAL-NEXT: >>> in live section: {{.*}}.o:(.test_simple)
40-
# INCIDENTAL-NEXT: >>> contained live symbol: test_simple
41-
# INCIDENTAL-NEXT: >>> referenced by: _start (entry point)
40+
# INCIDENTAL-NEXT: >>> contained live symbol: {{.*}}.o:(test_simple)
41+
# INCIDENTAL-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
4242
# INCIDENTAL-NOT: >>>
4343

4444
## Reached from a reference in section .test_simple directly, since test_simple is an unsized symbol.
@@ -49,10 +49,10 @@ test_from_unsized:
4949

5050
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_from_unsized | FileCheck %s --check-prefix=FROM-UNSIZED
5151

52-
# FROM-UNSIZED: live symbol: test_from_unsized
52+
# FROM-UNSIZED: live symbol: {{.*}}.o:(test_from_unsized)
5353
# FROM-UNSIZED-NEXT: >>> referenced by: {{.*}}.o:(.test_simple)
54-
# FROM-UNSIZED-NEXT: >>> contained live symbol: test_simple
55-
# FROM-UNSIZED-NEXT: >>> referenced by: _start (entry point)
54+
# FROM-UNSIZED-NEXT: >>> contained live symbol: {{.*}}.o:(test_simple)
55+
# FROM-UNSIZED-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
5656
# FROM-UNSIZED-NOT: >>>
5757

5858
## Symbols in dead sections are dead and not reported.
@@ -67,7 +67,7 @@ test_dead:
6767

6868
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_undef -u test_undef | FileCheck %s --check-prefix=UNDEFINED
6969

70-
# UNDEFINED: live symbol: test_undef (no section)
70+
# UNDEFINED: live symbol: <internal>:(test_undef) (no section)
7171
# UNDEFINED-NOT: >>>
7272

7373
## Defined symbols without input section parents are live.
@@ -76,7 +76,7 @@ test_absolute = 1234
7676

7777
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_absolute | FileCheck %s --check-prefix=ABSOLUTE
7878

79-
# ABSOLUTE: live symbol: test_absolute (no section)
79+
# ABSOLUTE: live symbol: {{.*}}.o:(test_absolute) (no section)
8080
# ABSOLUTE-NOT: >>>
8181

8282
## Retained sections are intrinsically live, and they make contained symbols live.
@@ -87,7 +87,7 @@ test_retained:
8787

8888
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_retained | FileCheck %s --check-prefix=RETAINED
8989

90-
# RETAINED: live symbol: test_retained
90+
# RETAINED: live symbol: {{.*}}.o:(test_retained)
9191
# RETAINED-NEXT: >>> in live section: {{.*}}:(.test_retained) (retained)
9292
# RETAINED-NOT: >>>
9393

@@ -102,9 +102,9 @@ test_section_offset:
102102

103103
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_section_offset | FileCheck %s --check-prefix=SECTION-OFFSET
104104

105-
# SECTION-OFFSET: live symbol: test_section_offset
105+
# SECTION-OFFSET: live symbol: {{.*}}.o:(test_section_offset)
106106
# SECTION-OFFSET-NEXT: >>> in live section: {{.*}}:(.test_section_offset)
107-
# SECTION-OFFSET-NEXT: >>> referenced by: _start (entry point)
107+
# SECTION-OFFSET-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
108108
# SECTION-OFFSET-NOT: >>>
109109

110110
## 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:
119119

120120
# 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
121121

122-
# SECTION-OFFSET-WITHIN-SYMBOL: live symbol: test_section_offset_within_symbol
123-
# SECTION-OFFSET-WITHIN-SYMBOL-NEXT: >>> referenced by: _start (entry point)
122+
# SECTION-OFFSET-WITHIN-SYMBOL: live symbol: {{.*}}.o:(test_section_offset_within_symbol)
123+
# SECTION-OFFSET-WITHIN-SYMBOL-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
124124
# SECTION-OFFSET-WITHIN-SYMBOL-NOT: >>>
125125

126126
## Local symbols can be queried just like global symbols.
@@ -132,22 +132,22 @@ test_local:
132132

133133
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections --why-live=test_local | FileCheck %s --check-prefix=LOCAL
134134

135-
# LOCAL: live symbol: {{.*}}:(test_local)
136-
# LOCAL-NEXT: >>> referenced by: _start (entry point)
135+
# LOCAL: live symbol: {{.*}}.o:(test_local)
136+
# LOCAL-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
137137
# LOCAL-NOT: >>>
138138

139139
## Shared symbols
140140

141141
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections %t.so --why-live=test_shared | FileCheck %s --check-prefix=SHARED
142142

143-
# SHARED: live symbol: test_shared
144-
# SHARED-NEXT: >>> referenced by: _start (entry point)
143+
# SHARED: live symbol: {{.*}}.so:(test_shared)
144+
# SHARED-NEXT: >>> referenced by: {{.*}}.o:(_start) (entry point)
145145
# SHARED-NOT: >>>
146146

147147
## Globs match multiple cases. Multiple --why-live flags union.
148148

149149
# RUN: ld.lld %t.o %t.so -o /dev/null --gc-sections %t.so --why-live=test_s* | FileCheck %s --check-prefix=MULTIPLE
150150
# 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
151151

152-
# MULTIPLE-DAG: live symbol: test_simple
153-
# MULTIPLE-DAG: live symbol: test_shared
152+
# MULTIPLE-DAG: live symbol: {{.*}}.o:(test_simple)
153+
# MULTIPLE-DAG: live symbol: {{.*}}.so:(test_shared)

0 commit comments

Comments
 (0)