Skip to content

[lld-macho] Add flag --keep-icf-stabs to LLD for MachO #93137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 28, 2024
Merged

[lld-macho] Add flag --keep-icf-stabs to LLD for MachO #93137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3033a2
[lld-macho] Add flag --keep-icf-stabs to LLD for MachO
alx32 May 23, 2024
92aa0ae
Address Feedback nr.1
alx32 May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lld/MachO/Config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ struct Configuration {
UndefinedSymbolTreatment undefinedSymbolTreatment =
UndefinedSymbolTreatment::error;
ICFLevel icfLevel = ICFLevel::none;
bool keepICFStabs = false;
ObjCStubsMode objcStubsMode = ObjCStubsMode::fast;
llvm::MachO::HeaderFileType outputType;
std::vector<llvm::StringRef> systemLibraryRoots;
Expand Down
1 change: 1 addition & 0 deletions lld/MachO/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,6 +1648,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
config->emitChainedFixups || args.hasArg(OPT_init_offsets);
config->emitRelativeMethodLists = shouldEmitRelativeMethodLists(args);
config->icfLevel = getICFLevel(args);
config->keepICFStabs = args.hasArg(OPT_keep_icf_stabs);
config->dedupStrings =
args.hasFlag(OPT_deduplicate_strings, OPT_no_deduplicate_strings, true);
config->deadStripDuplicates = args.hasArg(OPT_dead_strip_duplicates);
Expand Down
3 changes: 3 additions & 0 deletions lld/MachO/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ def icf_eq: Joined<["--"], "icf=">,
HelpText<"Set level for identical code folding (default: none)">,
MetaVarName<"[none,safe,all]">,
Group<grp_lld>;
def keep_icf_stabs: Joined<["--"], "keep-icf-stabs">,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stabs seems confusing as it appears a single word, but in fact it stands for symbol table?
Can we say keep-icf-symbol or something? Likewise, I'd use more descriptive word in the following, instead of using stabs entries below.

Copy link
Contributor Author

@alx32 alx32 May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag name comes from the entries that are created - STAB entries. Not sure of the origin of the name. @int3 - do you know the meaning of 'STABS' here ? Some code here:

llvm-project/lld/MachO/SyntheticSections.cpp

Line 1159 in e3033a2

StabsEntry stab(N_SO);
and some docs here: https://wiki.dwarfstd.org/Apple's_%22Lazy%22_DWARF_Scheme.md

It is not referring to the symbol table itself - the symbol table will (I think always) already contain the names of the symbols as they might need to be exported, etc.
It only affects STABS entries which are for debugging only.
I am not sure of the best name but keep-icf-symbol suggests that we would otherwise erase it from the symbol table - which is not the case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears a known common word for debug info. From your link, I'm seeing this.

we read the debug map (the stabs entries) out of the executable and create an address translation map for each .o file specifying what address the symbols landed at.

So, I think you can just expand the HelpText a bit on this flag.

HelpText<"Generate STABS entries for symbols folded by ICF. These entries can then be used by dsymutil to discover the address range where folded symbols are located.">,
Group<grp_lld>;
def lto_O: Joined<["--"], "lto-O">,
HelpText<"Set optimization level for LTO (default: 2)">,
MetaVarName<"<opt-level>">,
Expand Down
15 changes: 10 additions & 5 deletions lld/MachO/SyntheticSections.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1220,15 +1220,18 @@ void SymtabSection::emitStabs() {
continue;

// Constant-folded symbols go in the executable's symbol table, but don't
// get a stabs entry.
if (defined->wasIdenticalCodeFolded)
// get a stabs entry unless --keep-icf-stabs flag is specified
if (!config->keepICFStabs && defined->wasIdenticalCodeFolded)
continue;

ObjFile *file = defined->getObjectFile();
if (!file || !file->compileUnit)
continue;

symbolsNeedingStabs.emplace_back(defined, defined->isec()->getFile()->id);
// We use 'originalIsec' to get the file id of the symbol since 'isec()'
// might point to the merged ICF symbol's file
symbolsNeedingStabs.emplace_back(defined,
defined->originalIsec->getFile()->id);
}
}

Expand All @@ -1243,7 +1246,9 @@ void SymtabSection::emitStabs() {
InputFile *lastFile = nullptr;
for (SortingPair &pair : symbolsNeedingStabs) {
Defined *defined = pair.first;
InputSection *isec = defined->isec();
// We use 'originalIsec' of the symbol since we care about the actual origin
// of the symbol, not the canonical location returned by `isec()`.
InputSection *isec = defined->originalIsec;
ObjFile *file = cast<ObjFile>(isec->getFile());

if (lastFile == nullptr || lastFile != file) {
Expand All @@ -1256,7 +1261,7 @@ void SymtabSection::emitStabs() {
}

StabsEntry symStab;
symStab.sect = defined->isec()->parent->index;
symStab.sect = isec->parent->index;
symStab.strx = stringTableSection.addString(defined->getName());
symStab.value = defined->getVA();

Expand Down
27 changes: 27 additions & 0 deletions lld/test/MachO/stabs-icf.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
# RUN: %lld -lSystem --icf=all %t.o -o %t
# RUN: dsymutil -s %t | FileCheck %s -DDIR=%t -DSRC_PATH=%t.o

# RUN: %lld -lSystem --icf=all %t.o -o %t_icf_stabs --keep-icf-stabs
# RUN: dsymutil -s %t_icf_stabs | FileCheck %s -DDIR=%t_icf_stabs -DSRC_PATH=%t.o --check-prefixes=ICF_STABS

## This should include no N_FUN entry for _baz (which is ICF'd into _bar),
## but it does include a SECT EXT entry.
## NOTE: We do not omit the N_FUN entry for _bar even though it is of size zero.
Expand All @@ -27,6 +30,30 @@
# CHECK-DAG: ( {{.*}}) {{[0-9]+}} 0100 0000000000000000 'dyld_stub_binder'
# CHECK-EMPTY:


# ICF_STABS: (N_SO ) 00 0000 0000000000000000 '/tmp{{[/\\]}}test.cpp'
# ICF_STABS-NEXT: (N_OSO ) 03 0001 {{.*}} '[[SRC_PATH]]'
# ICF_STABS-NEXT: (N_FUN ) 01 0000 [[#%.16x,MAIN:]] '_main'
# ICF_STABS-NEXT: (N_FUN ) 00 0000 000000000000000b{{$}}
# ICF_STABS-NEXT: (N_FUN ) 01 0000 [[#%.16x,BAR:]] '_bar'
# ICF_STABS-NEXT: (N_FUN ) 00 0000 0000000000000000{{$}}
# ICF_STABS-NEXT: (N_FUN ) 01 0000 [[#BAR]] '_bar2'
# ICF_STABS-NEXT: (N_FUN ) 00 0000 0000000000000001{{$}}
# ICF_STABS-NEXT: (N_FUN ) 01 0000 [[#BAR]] '_baz'
# ICF_STABS-NEXT: (N_FUN ) 00 0000 0000000000000000{{$}}
# ICF_STABS-NEXT: (N_FUN ) 01 0000 [[#BAR]] '_baz2'
# ICF_STABS-NEXT: (N_FUN ) 00 0000 0000000000000001{{$}}
# ICF_STABS-NEXT: (N_SO ) 01 0000 0000000000000000{{$}}
# ICF_STABS-DAG: ( SECT EXT) 01 0000 [[#MAIN]] '_main'
# ICF_STABS-DAG: ( SECT EXT) 01 0000 [[#BAR]] '_bar'
# ICF_STABS-DAG: ( SECT EXT) 01 0000 [[#BAR]] '_bar2'
# ICF_STABS-DAG: ( SECT EXT) 01 0000 [[#BAR]] '_baz'
# ICF_STABS-DAG: ( SECT EXT) 01 0000 [[#BAR]] '_baz2'
# ICF_STABS-DAG: ( {{.*}}) {{[0-9]+}} 0010 {{[0-9a-f]+}} '__mh_execute_header'
# ICF_STABS-DAG: ( {{.*}}) {{[0-9]+}} 0100 0000000000000000 'dyld_stub_binder'
# ICF_STABS-EMPTY:


.text
.globl _bar, _bar2, _baz, _baz2, _main

Expand Down
Loading