[lld] fcab06b - [lld-macho][nfc] Sort OutputSections based on explicit order of command-line inputs
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Tue May 25 11:58:45 PDT 2021
Author: Jez Ng
Date: 2021-05-25T14:58:29-04:00
New Revision: fcab06bd85d142eb715b5b96997a24247002e753
URL: https://github.com/llvm/llvm-project/commit/fcab06bd85d142eb715b5b96997a24247002e753
DIFF: https://github.com/llvm/llvm-project/commit/fcab06bd85d142eb715b5b96997a24247002e753.diff
LOG: [lld-macho][nfc] Sort OutputSections based on explicit order of command-line inputs
This diff paves the way for {D102964} which adds a new kind of
InputSection.
We previously maintained section ordering implicitly: we created
InputSections as we parsed each file in command-line order, and passed
on this ordering when we created OutputSections and OutputSegments by
iterating over these InputSections. The implicitness of the ordering
made it difficult to refactor the code to e.g. handle a new type of
InputSection. As such, I've codified the ordering explicitly via
`inputOrder` fields. This also allows us to use `sort` instead of
`stable_sort`.
Benchmarking chromium_framework on my 3.2 GHz 16-Core Intel Xeon W:
N Min Max Median Avg Stddev
x 20 4.23 4.35 4.27 4.274 0.030157481
+ 20 4.24 4.38 4.27 4.2815 0.033759989
No difference proven at 95.0% confidence
Reviewed By: #lld-macho, alexshap
Differential Revision: https://reviews.llvm.org/D102972
Added:
lld/test/MachO/section-order.s
Modified:
lld/MachO/OutputSection.h
lld/MachO/OutputSegment.cpp
lld/MachO/OutputSegment.h
lld/MachO/Writer.cpp
lld/test/MachO/load-command-sequence.s
lld/test/MachO/weak-binding.s
Removed:
################################################################################
diff --git a/lld/MachO/OutputSection.h b/lld/MachO/OutputSection.h
index 8110f756ab218..45a3b48b6eb71 100644
--- a/lld/MachO/OutputSection.h
+++ b/lld/MachO/OutputSection.h
@@ -12,6 +12,8 @@
#include "lld/Common/LLVM.h"
#include "llvm/ADT/DenseMap.h"
+#include <limits>
+
namespace lld {
namespace macho {
@@ -56,6 +58,10 @@ class OutputSection {
StringRef name;
OutputSegment *parent = nullptr;
+ // For output sections that don't have explicit ordering requirements, their
+ // output order should be based on the order of the input sections they
+ // contain.
+ int inputOrder = std::numeric_limits<int>::max();
uint32_t index = 0;
uint64_t addr = 0;
diff --git a/lld/MachO/OutputSegment.cpp b/lld/MachO/OutputSegment.cpp
index 8ed4c3c43b14e..3989d83d2a244 100644
--- a/lld/MachO/OutputSegment.cpp
+++ b/lld/MachO/OutputSegment.cpp
@@ -50,6 +50,8 @@ size_t OutputSegment::numNonHiddenSections() const {
}
void OutputSegment::addOutputSection(OutputSection *osec) {
+ inputOrder = std::min(inputOrder, osec->inputOrder);
+
osec->parent = this;
sections.push_back(osec);
diff --git a/lld/MachO/OutputSegment.h b/lld/MachO/OutputSegment.h
index 5593fb62e6055..80d2d73794b20 100644
--- a/lld/MachO/OutputSegment.h
+++ b/lld/MachO/OutputSegment.h
@@ -12,6 +12,8 @@
#include "OutputSection.h"
#include "lld/Common/LLVM.h"
+#include <limits>
+
namespace lld {
namespace macho {
@@ -42,7 +44,7 @@ class OutputSegment {
void addOutputSection(OutputSection *os);
void sortOutputSections(
llvm::function_ref<bool(OutputSection *, OutputSection *)> comparator) {
- llvm::stable_sort(sections, comparator);
+ llvm::sort(sections, comparator);
}
const std::vector<OutputSection *> &getSections() const { return sections; }
@@ -51,6 +53,7 @@ class OutputSegment {
uint64_t fileOff = 0;
uint64_t fileSize = 0;
uint64_t vmSize = 0;
+ int inputOrder = std::numeric_limits<int>::max();
StringRef name;
uint32_t maxProt = 0;
uint32_t initProt = 0;
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index c4cdf73be23dd..3dcf360bf6e53 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -765,7 +765,7 @@ static int segmentOrder(OutputSegment *seg) {
// Make sure __LINKEDIT is the last segment (i.e. all its hidden
// sections must be ordered after other sections).
.Case(segment_names::linkEdit, std::numeric_limits<int>::max())
- .Default(0);
+ .Default(seg->inputOrder);
}
static int sectionOrder(OutputSection *osec) {
@@ -779,7 +779,7 @@ static int sectionOrder(OutputSection *osec) {
.Case(section_names::stubHelper, -1)
.Case(section_names::unwindInfo, std::numeric_limits<int>::max() - 1)
.Case(section_names::ehFrame, std::numeric_limits<int>::max())
- .Default(0);
+ .Default(osec->inputOrder);
} else if (segname == segment_names::data ||
segname == segment_names::dataConst) {
// For each thread spawned, dyld will initialize its TLVs by copying the
@@ -798,9 +798,10 @@ static int sectionOrder(OutputSection *osec) {
return std::numeric_limits<int>::max();
default:
return StringSwitch<int>(osec->name)
+ .Case(section_names::got, -3)
.Case(section_names::lazySymbolPtr, -2)
- .Case(section_names::data, -1)
- .Default(0);
+ .Case(section_names::const_, -1)
+ .Default(osec->inputOrder);
}
} else if (segname == segment_names::linkEdit) {
return StringSwitch<int>(osec->name)
@@ -814,13 +815,13 @@ static int sectionOrder(OutputSection *osec) {
.Case(section_names::indirectSymbolTable, -2)
.Case(section_names::stringTable, -1)
.Case(section_names::codeSignature, std::numeric_limits<int>::max())
- .Default(0);
+ .Default(osec->inputOrder);
}
// ZeroFill sections must always be the at the end of their segments,
// otherwise subsequent sections may get overwritten with zeroes at runtime.
if (sectionType(osec->flags) == S_ZEROFILL)
return std::numeric_limits<int>::max();
- return 0;
+ return osec->inputOrder;
}
template <typename T, typename F>
@@ -834,8 +835,7 @@ static std::function<bool(T, T)> compareByOrder(F ord) {
static void sortSegmentsAndSections() {
TimeTraceScope timeScope("Sort segments and sections");
- llvm::stable_sort(outputSegments,
- compareByOrder<OutputSegment *>(segmentOrder));
+ llvm::sort(outputSegments, compareByOrder<OutputSegment *>(segmentOrder));
DenseMap<const InputSection *, size_t> isecPriorities =
buildInputSectionPriorities();
@@ -898,14 +898,17 @@ template <class LP> void Writer::createOutputSections() {
}
// Then add input sections to output sections.
- MapVector<NamePair, ConcatOutputSection *> concatOutputSections;
- for (InputSection *isec : inputSections) {
+ DenseMap<NamePair, ConcatOutputSection *> concatOutputSections;
+ for (const auto &p : enumerate(inputSections)) {
+ InputSection *isec = p.value();
if (isec->shouldOmitFromOutput())
continue;
NamePair names = maybeRenameSection({isec->segname, isec->name});
ConcatOutputSection *&osec = concatOutputSections[names];
- if (osec == nullptr)
+ if (osec == nullptr) {
osec = make<ConcatOutputSection>(names.second);
+ osec->inputOrder = p.index();
+ }
osec->addInput(isec);
}
diff --git a/lld/test/MachO/load-command-sequence.s b/lld/test/MachO/load-command-sequence.s
index 028b0ed8a7149..5805aec753b7b 100644
--- a/lld/test/MachO/load-command-sequence.s
+++ b/lld/test/MachO/load-command-sequence.s
@@ -28,10 +28,10 @@
# COMMON: segname __TEXT
# COMMON: cmd LC_SEGMENT_64
# COMMON: segname __DATA_CONST
-# COMMON: sectname __const
-# COMMON: segname __DATA_CONST
# COMMON: sectname __got
# COMMON: segname __DATA_CONST
+# COMMON: sectname __const
+# COMMON: segname __DATA_CONST
# COMMON: cmd LC_SEGMENT_64
# COMMON: segname __DATA
# COMMON: sectname __data
diff --git a/lld/test/MachO/section-order.s b/lld/test/MachO/section-order.s
new file mode 100644
index 0000000000000..c16300fdc1b22
--- /dev/null
+++ b/lld/test/MachO/section-order.s
@@ -0,0 +1,35 @@
+# REQUIRES: x86
+## Check that section ordering follows from input file ordering.
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/1.s -o %t/1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/2.s -o %t/2.o
+# RUN: %lld -dylib %t/1.o %t/2.o -o %t/12
+# RUN: %lld -dylib %t/2.o %t/1.o -o %t/21
+# RUN: llvm-objdump --macho --section-headers %t/12 | FileCheck %s --check-prefix=CHECK-12
+# RUN: llvm-objdump --macho --section-headers %t/21 | FileCheck %s --check-prefix=CHECK-21
+
+# CHECK-12: __text
+# CHECK-12-NEXT: foo
+# CHECK-12-NEXT: bar
+# CHECK-12-NEXT: __cstring
+
+# CHECK-21: __text
+# CHECK-21-NEXT: __cstring
+# CHECK-21-NEXT: bar
+# CHECK-21-NEXT: foo
+
+#--- 1.s
+.section __TEXT,foo
+ .space 1
+.section __TEXT,bar
+ .space 1
+.cstring
+ .asciz ""
+
+#--- 2.s
+.cstring
+ .asciz ""
+.section __TEXT,bar
+ .space 1
+.section __TEXT,foo
+ .space 1
diff --git a/lld/test/MachO/weak-binding.s b/lld/test/MachO/weak-binding.s
index 73c8733609975..2b39b0ed19502 100644
--- a/lld/test/MachO/weak-binding.s
+++ b/lld/test/MachO/weak-binding.s
@@ -4,8 +4,8 @@
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o
# RUN: %lld -dylib %t/libfoo.o -o %t/libfoo.dylib
# RUN: %lld %t/test.o -L%t -lfoo -o %t/test -lSystem
-# RUN: llvm-objdump -d --no-show-raw-insn --rebase --bind --lazy-bind --weak-bind --full-contents %t/test | \
-# RUN: FileCheck %s
+# RUN: llvm-objdump -d --no-show-raw-insn --rebase --bind --lazy-bind \
+# RUN: --weak-bind --full-contents %t/test | FileCheck %s
# CHECK: Contents of section __DATA_CONST,__got:
## Check that this section contains a nonzero pointer. It should point to
@@ -37,13 +37,15 @@
# CHECK-DAG: __DATA __la_symbol_ptr 0x[[#%x,WEAK_DY_FN:]] pointer 0 libfoo _weak_dysym_fn
# CHECK-DAG: __DATA __data 0x[[#%x,WEAK_DY:]] pointer 0 libfoo _weak_dysym
# CHECK-DAG: __DATA __thread_vars 0x{{[0-9a-f]*}} pointer 0 libSystem __tlv_bootstrap
+# CHECK-DAG: __DATA __thread_vars 0x{{[0-9a-f]*}} pointer 0 libSystem __tlv_bootstrap
# CHECK-DAG: __DATA __thread_ptrs 0x[[#WEAK_DY_TLV_ADDR]] pointer 0 libfoo _weak_dysym_tlv
## Check that we don't have any other bindings
-# CHECK-NOT: pointer
+# CHECK-EMPTY:
# CHECK-LABEL: Lazy bind table:
+# CHECK-NEXT: segment section address dylib symbol
## Verify that we have no lazy bindings
-# CHECK-NOT: pointer
+# CHECK-EMPTY:
# CHECK-LABEL: Weak bind table:
# CHECK-DAG: __DATA_CONST __got 0x[[#WEAK_DY_GOT_ADDR]] pointer 0 _weak_dysym_for_gotpcrel
@@ -55,7 +57,7 @@
# CHECK-DAG: __DATA __la_symbol_ptr 0x[[#WEAK_DY_FN]] pointer 0 _weak_dysym_fn
# CHECK-DAG: __DATA __la_symbol_ptr 0x[[#WEAK_EXT_FN]] pointer 0 _weak_external_fn
## Check that we don't have any other bindings
-# CHECK-NOT: pointer
+# CHECK-EMPTY:
## Weak internal symbols don't get bindings
# RUN: llvm-objdump --macho --bind --lazy-bind --weak-bind %t/test | FileCheck %s --check-prefix=WEAK-INTERNAL
More information about the llvm-commits
mailing list