[lld] 16d7841 - [lld-macho] Don't fold subsections with symbols at nonzero offsets
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 18 14:22:24 PDT 2022
Author: Jez Ng
Date: 2022-10-18T17:22:09-04:00
New Revision: 16d784159f6873ff5ca860c0e490103127aa2e4e
URL: https://github.com/llvm/llvm-project/commit/16d784159f6873ff5ca860c0e490103127aa2e4e
DIFF: https://github.com/llvm/llvm-project/commit/16d784159f6873ff5ca860c0e490103127aa2e4e.diff
LOG: [lld-macho] Don't fold subsections with symbols at nonzero offsets
Symbols occur at non-zero offsets in a subsection if they are
`.alt_entry` symbols, or if `.subsections_via_symbols` is omitted.
It doesn't seem like ld64 supports folding those subsections either.
Moreover, supporting this it makes `foldIdentical` a lot more
complicated to implement. The existing implementation has some
questionable behavior around STABS omission -- if a section with an
non-zero offset symbol was folded into one without, we would omit the
STABS entry for the non-zero offset symbol.
I will be following up with a diff that makes `foldIdentical` zero out
the symbol sizes for folded symbols. Again, this is much easier to
implement if we don't have to worry about non-zero offsets.
Reviewed By: #lld-macho, oontvoo
Differential Revision: https://reviews.llvm.org/D136000
Added:
Modified:
lld/MachO/ICF.cpp
lld/MachO/InputFiles.cpp
lld/MachO/InputSection.cpp
lld/MachO/InputSection.h
lld/test/MachO/icf.s
Removed:
################################################################################
diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp
index 4f4b8fd8e2a4f..bc3dd99ccc459 100644
--- a/lld/MachO/ICF.cpp
+++ b/lld/MachO/ICF.cpp
@@ -425,8 +425,8 @@ void macho::foldIdenticalSections(bool onlyCfStrings) {
bool isFoldable = (!onlyCfStrings || isCfStringSection(isec)) &&
(isCodeSection(isec) || isFoldableWithAddendsRemoved ||
isGccExceptTabSection(isec)) &&
- !isec->keepUnique && !isec->shouldOmitFromOutput() &&
- hasFoldableFlags;
+ !isec->keepUnique && !isec->hasAltEntry &&
+ !isec->shouldOmitFromOutput() && hasFoldableFlags;
if (isFoldable) {
foldable.push_back(isec);
for (Defined *d : isec->symbols)
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index afd0e28a3dce4..e542fe24255fb 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -854,6 +854,7 @@ void ObjFile::parseSymbols(ArrayRef<typename LP::section> sectionHeaders,
// 4. If we have a literal section (e.g. __cstring and __literal4).
if (!subsectionsViaSymbols || symbolOffset == 0 ||
sym.n_desc & N_ALT_ENTRY || !isa<ConcatInputSection>(isec)) {
+ isec->hasAltEntry = symbolOffset != 0;
symbols[symIndex] = createDefined(sym, name, isec, symbolOffset,
symbolSize, forceHidden);
continue;
diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index ca5c7522d58e9..398a5c22a5c37 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -141,31 +141,15 @@ void ConcatInputSection::foldIdentical(ConcatInputSection *copy) {
for (auto ©Sym : copy->symbols)
copySym->wasIdenticalCodeFolded = true;
- // Merge the sorted vectors of symbols together.
- auto it = symbols.begin();
- for (auto copyIt = copy->symbols.begin(); copyIt != copy->symbols.end();) {
- if (it == symbols.end()) {
- symbols.push_back(*copyIt++);
- it = symbols.end();
- } else if ((*it)->value > (*copyIt)->value) {
- std::swap(*it++, *copyIt);
- } else {
- ++it;
- }
- }
+ symbols.insert(symbols.end(), copy->symbols.begin(), copy->symbols.end());
copy->symbols.clear();
// Remove duplicate compact unwind info for symbols at the same address.
if (symbols.empty())
return;
- it = symbols.begin();
- uint64_t v = (*it)->value;
- for (++it; it != symbols.end(); ++it) {
- Defined *d = *it;
- if (d->value == v)
- d->unwindEntry = nullptr;
- else
- v = d->value;
+ for (auto it = symbols.begin() + 1; it != symbols.end(); ++it) {
+ assert((*it)->value == 0);
+ (*it)->unwindEntry = nullptr;
}
}
diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 1a9706861c114..5a6a205f9047c 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -64,11 +64,12 @@ class InputSection {
protected:
InputSection(Kind kind, const Section §ion, ArrayRef<uint8_t> data,
uint32_t align)
- : sectionKind(kind), align(align), data(data), section(section) {}
+ : sectionKind(kind), keepUnique(false), hasAltEntry(false), align(align),
+ data(data), section(section) {}
InputSection(const InputSection &rhs)
- : sectionKind(rhs.sectionKind), align(rhs.align), data(rhs.data),
- section(rhs.section) {}
+ : sectionKind(rhs.sectionKind), keepUnique(false), hasAltEntry(false),
+ align(rhs.align), data(rhs.data), section(rhs.section) {}
Kind sectionKind;
@@ -77,7 +78,10 @@ class InputSection {
bool isFinal = false;
// keep the address of the symbol(s) in this section unique in the final
// binary ?
- bool keepUnique = false;
+ bool keepUnique : 1;
+ // Does this section have symbols at offsets other than zero? (NOTE: only
+ // applies to ConcatInputSections.)
+ bool hasAltEntry : 1;
uint32_t align = 1;
OutputSection *parent = nullptr;
diff --git a/lld/test/MachO/icf.s b/lld/test/MachO/icf.s
index 4325102e97232..03b474388f2eb 100644
--- a/lld/test/MachO/icf.s
+++ b/lld/test/MachO/icf.s
@@ -25,7 +25,7 @@
# CHECK: [[#%x,DYLIB_REF_4:]] l F __TEXT,__text _dylib_ref_4
# CHECK: [[#%x,ALT:]] l F __TEXT,__text _alt
# CHECK: [[#%x,WITH_ALT_ENTRY:]] l F __TEXT,__text _with_alt_entry
-# CHECK: [[#%x,WITH_ALT_ENTRY]] l F __TEXT,__text _no_alt_entry
+# CHECK: [[#%x,NO_ALT_ENTRY:]] l F __TEXT,__text _no_alt_entry
# CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_2:]] l F __TEXT,__text _defined_ref_with_addend_1
# CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_2]] l F __TEXT,__text _defined_ref_with_addend_2
# CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_3:]] l F __TEXT,__text _defined_ref_with_addend_3
@@ -74,7 +74,7 @@
# CHECK: callq 0x[[#%x,DYLIB_REF_4]] <_dylib_ref_4>
# CHECK: callq 0x[[#%x,ALT]] <_alt>
# CHECK: callq 0x[[#%x,WITH_ALT_ENTRY]] <_with_alt_entry>
-# CHECK: callq 0x[[#%x,WITH_ALT_ENTRY]] <_with_alt_entry>
+# CHECK: callq 0x[[#%x,NO_ALT_ENTRY]] <_no_alt_entry>
# CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_2]] <_defined_ref_with_addend_2>
# CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_2]] <_defined_ref_with_addend_2>
# CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_3]] <_defined_ref_with_addend_3>
@@ -161,8 +161,7 @@ _dylib_ref_4:
mov ___nan + 1 at GOTPCREL(%rip), %rax
callq ___inf + 1
-## We can merge two sections even if one of them has an alt entry. Just make
-## sure we don't merge the alt entry symbol with a regular symbol.
+## Sections with alt entries cannot be merged.
.alt_entry _alt
_with_alt_entry:
movq $3132, %rax
More information about the llvm-commits
mailing list