[lld] a8a6e5b - [lld-macho] Preserve alignment for non-deduplicated cstrings
Nico Weber via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 28 19:26:58 PDT 2021
Author: Leonard Grey
Date: 2021-06-28T22:26:43-04:00
New Revision: a8a6e5b094aac642f436390294ec837400c521bb
URL: https://github.com/llvm/llvm-project/commit/a8a6e5b094aac642f436390294ec837400c521bb
DIFF: https://github.com/llvm/llvm-project/commit/a8a6e5b094aac642f436390294ec837400c521bb.diff
LOG: [lld-macho] Preserve alignment for non-deduplicated cstrings
Fixes PR50637.
Downstream bug: https://crbug.com/1218958
Currently, we split __cstring along symbol boundaries with .subsections_via_symbols
when not deduplicating, and along null bytes when deduplicating. This change splits
along null bytes unconditionally, and preserves original alignment in the non-
deduplicated case.
Removing subsections-section-relocs.s because with this change, __cstring
is never reordered based on the order file.
Differential Revision: https://reviews.llvm.org/D104919
Added:
lld/test/MachO/dead-strip-align.s
Modified:
lld/MachO/InputFiles.cpp
lld/MachO/InputSection.cpp
lld/MachO/InputSection.h
lld/MachO/SyntheticSections.cpp
lld/MachO/SyntheticSections.h
lld/MachO/Writer.cpp
Removed:
lld/test/MachO/subsections-section-relocs.s
################################################################################
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 32527c7f4c7b8..4025a4c55ab1d 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -262,10 +262,9 @@ void ObjFile::parseSections(ArrayRef<Section> sections) {
uint32_t align = 1 << sec.align;
uint32_t flags = sec.flags;
- if (config->dedupLiterals &&
- (sectionType(sec.flags) == S_CSTRING_LITERALS ||
- isWordLiteralSection(sec.flags))) {
- if (sec.nreloc)
+ if (sectionType(sec.flags) == S_CSTRING_LITERALS ||
+ (config->dedupLiterals && isWordLiteralSection(sec.flags))) {
+ if (sec.nreloc && config->dedupLiterals)
fatal(toString(this) + " contains relocations in " + sec.segname + "," +
sec.sectname +
", so LLD cannot deduplicate literals. Try re-running without "
diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index a961807abd230..740eea6d8fd41 100644
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "InputSection.h"
+#include "Config.h"
#include "InputFiles.h"
#include "OutputSegment.h"
#include "Symbols.h"
@@ -156,7 +157,8 @@ void CStringInputSection::splitIntoPieces() {
if (end == StringRef::npos)
fatal(toString(this) + ": string is not null terminated");
size_t size = end + 1;
- pieces.emplace_back(off, xxHash64(s.substr(0, size)));
+ uint32_t hash = config->dedupLiterals ? xxHash64(s.substr(0, size)) : 0;
+ pieces.emplace_back(off, hash);
s = s.substr(size);
off += size;
}
diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 3dd31d27be919..9eea39105f147 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -145,6 +145,7 @@ struct StringPiece {
// Offset from the start of the containing input section.
uint32_t inSecOff;
uint32_t live : 1;
+ // Only set if deduplicating literals
uint32_t hash : 31;
// Offset from the start of the containing output section.
uint64_t outSecOff = 0;
@@ -180,14 +181,20 @@ class CStringInputSection final : public InputSection {
// Split at each null byte.
void splitIntoPieces();
- // Returns i'th piece as a CachedHashStringRef. This function is very hot when
- // string merging is enabled, so we want to inline.
LLVM_ATTRIBUTE_ALWAYS_INLINE
- llvm::CachedHashStringRef getCachedHashStringRef(size_t i) const {
+ StringRef getStringRef(size_t i) const {
size_t begin = pieces[i].inSecOff;
size_t end =
(pieces.size() - 1 == i) ? data.size() : pieces[i + 1].inSecOff;
- return {toStringRef(data.slice(begin, end - begin)), pieces[i].hash};
+ return toStringRef(data.slice(begin, end - begin));
+ }
+
+ // Returns i'th piece as a CachedHashStringRef. This function is very hot when
+ // string merging is enabled, so we want to inline.
+ LLVM_ATTRIBUTE_ALWAYS_INLINE
+ llvm::CachedHashStringRef getCachedHashStringRef(size_t i) const {
+ assert(config->dedupLiterals);
+ return {getStringRef(i), pieces[i].hash};
}
static bool classof(const InputSection *isec) {
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 81fec04275295..c5d77142fda06 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -1152,6 +1152,45 @@ void BitcodeBundleSection::writeTo(uint8_t *buf) const {
remove(xarPath);
}
+CStringSection::CStringSection()
+ : SyntheticSection(segment_names::text, section_names::cString) {
+ flags = S_CSTRING_LITERALS;
+}
+
+void CStringSection::addInput(CStringInputSection *isec) {
+ isec->parent = this;
+ inputs.push_back(isec);
+ if (isec->align > align)
+ align = isec->align;
+}
+
+void CStringSection::writeTo(uint8_t *buf) const {
+ for (const CStringInputSection *isec : inputs) {
+ for (size_t i = 0, e = isec->pieces.size(); i != e; ++i) {
+ if (!isec->pieces[i].live)
+ continue;
+ StringRef string = isec->getStringRef(i);
+ memcpy(buf + isec->pieces[i].outSecOff, string.data(), string.size());
+ }
+ }
+}
+
+void CStringSection::finalizeContents() {
+ uint64_t offset = 0;
+ for (CStringInputSection *isec : inputs) {
+ for (size_t i = 0, e = isec->pieces.size(); i != e; ++i) {
+ if (!isec->pieces[i].live)
+ continue;
+ uint32_t pieceAlign = MinAlign(isec->pieces[i].inSecOff, align);
+ offset = alignTo(offset, pieceAlign);
+ isec->pieces[i].outSecOff = offset;
+ isec->isFinal = true;
+ StringRef string = isec->getStringRef(i);
+ offset += string.size();
+ }
+ }
+ size = offset;
+}
// Mergeable cstring literals are found under the __TEXT,__cstring section. In
// contrast to ELF, which puts strings that need
diff erent alignments into
//
diff erent sections, clang's Mach-O backend puts them all in one section.
@@ -1176,19 +1215,10 @@ void BitcodeBundleSection::writeTo(uint8_t *buf) const {
// deduplication of
diff erently-aligned strings. Finally, the overhead is not
// huge: using 16-byte alignment (vs no alignment) is only a 0.5% size overhead
// when linking chromium_framework on x86_64.
-CStringSection::CStringSection()
- : SyntheticSection(segment_names::text, section_names::cString),
- builder(StringTableBuilder::RAW, /*Alignment=*/16) {
- align = 16;
- flags = S_CSTRING_LITERALS;
-}
+DeduplicatedCStringSection::DeduplicatedCStringSection()
+ : builder(StringTableBuilder::RAW, /*Alignment=*/16) {}
-void CStringSection::addInput(CStringInputSection *isec) {
- isec->parent = this;
- inputs.push_back(isec);
-}
-
-void CStringSection::finalizeContents() {
+void DeduplicatedCStringSection::finalizeContents() {
// Add all string pieces to the string table builder to create section
// contents.
for (const CStringInputSection *isec : inputs)
diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index a5f6ea9a6e1f4..95a09a010e274 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -518,17 +518,28 @@ class BitcodeBundleSection final : public SyntheticSection {
uint64_t xarSize;
};
-class CStringSection final : public SyntheticSection {
+class CStringSection : public SyntheticSection {
public:
CStringSection();
void addInput(CStringInputSection *);
- uint64_t getSize() const override { return builder.getSize(); }
- void finalizeContents();
+ uint64_t getSize() const override { return size; }
+ virtual void finalizeContents();
bool isNeeded() const override { return !inputs.empty(); }
- void writeTo(uint8_t *buf) const override { builder.write(buf); }
+ void writeTo(uint8_t *buf) const override;
std::vector<CStringInputSection *> inputs;
+private:
+ uint64_t size;
+};
+
+class DeduplicatedCStringSection final : public CStringSection {
+public:
+ DeduplicatedCStringSection();
+ uint64_t getSize() const override { return builder.getSize(); }
+ void finalizeContents() override;
+ void writeTo(uint8_t *buf) const override { builder.write(buf); }
+
private:
llvm::StringTableBuilder builder;
};
diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index ffe5668a877bd..5520e65d881f2 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -1149,7 +1149,11 @@ template <class LP> void macho::writeResult() { Writer().run<LP>(); }
void macho::createSyntheticSections() {
in.header = make<MachHeaderSection>();
- in.cStringSection = config->dedupLiterals ? make<CStringSection>() : nullptr;
+ if (config->dedupLiterals) {
+ in.cStringSection = make<DeduplicatedCStringSection>();
+ } else {
+ in.cStringSection = make<CStringSection>();
+ }
in.wordLiteralSection =
config->dedupLiterals ? make<WordLiteralSection>() : nullptr;
in.rebase = make<RebaseSection>();
diff --git a/lld/test/MachO/dead-strip-align.s b/lld/test/MachO/dead-strip-align.s
new file mode 100644
index 0000000000000..46452079cb365
--- /dev/null
+++ b/lld/test/MachO/dead-strip-align.s
@@ -0,0 +1,46 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: %lld -lSystem -o %t.out %t.o -dead_strip
+# RUN: llvm-otool -l %t.out | FileCheck --check-prefix=SECT %s
+# RUN: llvm-otool -vs __TEXT __cstring %t.out | FileCheck %s
+
+# SECT: sectname __cstring
+# SECT-NEXT: segname __TEXT
+# SECT-NEXT: addr
+# SECT-NEXT: size
+# SECT-NEXT: offset
+# SECT-NEXT: align 2^4 (16)
+
+# CHECK: 0 \303Q043\005\376\334\272\230vT2\020\001
+# CHECK: 8 def
+
+.section __TEXT,__cstring,cstring_literals
+.globl _foo
+_foo: # Dead. External, has symbol table entry, gets stripped.
+ .asciz "asdf"
+
+.globl _hi
+_hi:
+ .asciz "hi" # External, has symbol table entry.
+
+.p2align 4
+L_internal_aligned_16: # Has no symbol table entry.
+ .asciz "\303Q043\005\376\334\272\230vT2\020\001"
+
+L_internal_nonaligned:
+ .asciz "abc"
+
+.p2align 3
+L_internal_aligned_8:
+ .asciz "def"
+
+.text
+.globl _main
+_main:
+ movq _hi(%rip), %rax
+ movq L_internal_nonaligned(%rip), %rax
+ movq L_internal_aligned_8(%rip), %rax
+ movaps L_internal_aligned_16(%rip), %xmm0
+ retq
+
+.subsections_via_symbols
diff --git a/lld/test/MachO/subsections-section-relocs.s b/lld/test/MachO/subsections-section-relocs.s
deleted file mode 100644
index 84baa4784202a..0000000000000
--- a/lld/test/MachO/subsections-section-relocs.s
+++ /dev/null
@@ -1,52 +0,0 @@
-# REQUIRES: x86
-# RUN: rm -rf %t; split-file %s %t
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
-
-# RUN: %lld -o %t/test %t/test.o -order_file %t/order-file
-# RUN: llvm-objdump --section-headers -d --no-show-raw-insn %t/test | FileCheck %s
-# CHECK-LABEL: Sections:
-# CHECK: __cstring {{[^ ]*}} {{0*}}[[#%x, CSTRING_ADDR:]]
-# CHECK-LABEL: Disassembly of section __TEXT,__text:
-## L._str should end up at CSTRING_ADDR + 4, and leaq is 7 bytes long so we
-## have RIP = ADDR + 7
-# CHECK: [[#%x, ADDR:]]: leaq
-# CHECK-SAME: [[#%u, CSTRING_ADDR + 4 - ADDR - 7]](%rip), %rsi {{.*}} <_bar_str+0x4>
-
-# RUN: llvm-readobj --string-dump=__cstring %t/test | FileCheck %s --check-prefix=STRINGS
-# STRINGS: bar
-# STRINGS: Private symbol
-# STRINGS: foo
-
-#--- order-file
-_bar_str
-_foo_str
-
-#--- test.s
-.text
-.globl _main, _foo_str, _bar_str
-
-_main:
- leaq L_.str(%rip), %rsi
- mov $0, %rax
- ret
-
-.section __TEXT,__cstring
-_foo_str:
- .asciz "foo"
-
-_bar_str:
- .asciz "bar"
-
-## References to this generate a section relocation
-## N.B.: ld64 doesn't actually reorder symbols in __cstring based on the order
-## file. Our implementation only does does so if --no-literal-merge is
-## specified. I'm not sure how else to test section relocations that
-## target an address inside a relocated symbol: using a non-__cstring
-## section would cause llvm-mc to emit a symbol relocation instead using
-## the nearest symbol. It might be more consistent for LLD to disable
-## symbol-based cstring reordering altogether and leave this functionality
-## untested, at least until we find a real-world use case...
-L_.str:
- .asciz "Private symbol"
-
-.subsections_via_symbols
More information about the llvm-commits
mailing list