Skip to content

Commit

Permalink
[lld][WebAssembly] Do not merge comdat data segments
Browse files Browse the repository at this point in the history
When running in relocatable mode any input data segments that are part
of a comdat group should not be merged with other segments of the same
name.  This is because the final linker needs to keep the separate so
they can be included/excluded individually.

Often this is not a problem since normally only one section with a given
name `foo` ends up in the output object file.  However, the problem
occurs when one input contains `foo` which part of a comdat and another
object contains a local symbol `foo` we were attempting to merge them.

This behaviour matches (I believe) that of the ELF linker.  See
`LinkerScript.cpp:addInputSec`.

Fixes: emscripten-core/emscripten#9726

Differential Revision: https://reviews.llvm.org/D101703
  • Loading branch information
sbc100 committed May 3, 2021
1 parent e38ccb7 commit 73332d7
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 16 deletions.
6 changes: 6 additions & 0 deletions lld/test/wasm/Inputs/comdat-data.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.globl foo
.section .data.foo,"G",@,foo,comdat
foo:
.int32 42
.int32 43
.size foo, 8
46 changes: 46 additions & 0 deletions lld/test/wasm/relocatable-comdat.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# RUN: llvm-mc -triple=wasm32 -filetype=obj %p/Inputs/comdat-data.s -o %t1.o
# RUN: llvm-mc -triple=wasm32 -filetype=obj %s -o %t.o
# RUN: wasm-ld --relocatable -o %t.wasm %t.o %t1.o
# RUN: obj2yaml %t.wasm | FileCheck %s


.globl _start
.type _start,@function
_start:
.functype _start () -> ()
i32.load foo
end_function


.section .data.foo,"",@
foo:
.int32 42
.size foo, 4

# Verify that .data.foo in this file is not merged with comdat .data.foo
# section in Inputs/comdat-data.s.

# CHECK: - Type: DATA
# CHECK-NEXT: Segments:
# CHECK-NEXT: - SectionOffset: 6
# CHECK-NEXT: InitFlags: 0
# CHECK-NEXT: Offset:
# CHECK-NEXT: Opcode: I32_CONST
# CHECK-NEXT: Value: 0
# CHECK-NEXT: Content: 2A000000
# CHECK-NEXT: - SectionOffset: 15
# CHECK-NEXT: InitFlags: 0
# CHECK-NEXT: Offset:
# CHECK-NEXT: Opcode: I32_CONST
# CHECK-NEXT: Value: 4
# CHECK-NEXT: Content: 2A0000002B000000

# CHECK: SegmentInfo:
# CHECK-NEXT: - Index: 0
# CHECK-NEXT: Name: .data.foo
# CHECK-NEXT: Alignment: 0
# CHECK-NEXT: Flags: [ ]
# CHECK-NEXT: - Index: 1
# CHECK-NEXT: Name: .data.foo
# CHECK-NEXT: Alignment: 0
# CHECK-NEXT: Flags: [ ]
40 changes: 26 additions & 14 deletions lld/wasm/Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class Writer {
void calculateCustomSections();
void calculateTypes();
void createOutputSegments();
OutputSegment *createOutputSegment(StringRef name);
void combineOutputSegments();
void layoutMemory();
void createHeader();
Expand Down Expand Up @@ -830,26 +831,37 @@ static StringRef getOutputDataSegmentName(StringRef name) {
return name;
}

OutputSegment *Writer::createOutputSegment(StringRef name) {
LLVM_DEBUG(dbgs() << "new segment: " << name << "\n");
OutputSegment *s = make<OutputSegment>(name);
if (config->sharedMemory)
s->initFlags = WASM_DATA_SEGMENT_IS_PASSIVE;
// Exported memories are guaranteed to be zero-initialized, so no need
// to emit data segments for bss sections.
// TODO: consider initializing bss sections with memory.fill
// instructions when memory is imported and bulk-memory is available.
if (!config->importMemory && !config->relocatable && name.startswith(".bss"))
s->isBss = true;
segments.push_back(s);
return s;
}

void Writer::createOutputSegments() {
for (ObjFile *file : symtab->objectFiles) {
for (InputSegment *segment : file->segments) {
if (!segment->live)
continue;
StringRef name = getOutputDataSegmentName(segment->getName());
OutputSegment *&s = segmentMap[name];
if (s == nullptr) {
LLVM_DEBUG(dbgs() << "new segment: " << name << "\n");
s = make<OutputSegment>(name);
if (config->sharedMemory)
s->initFlags = WASM_DATA_SEGMENT_IS_PASSIVE;
// Exported memories are guaranteed to be zero-initialized, so no need
// to emit data segments for bss sections.
// TODO: consider initializing bss sections with memory.fill
// instructions when memory is imported and bulk-memory is available.
if (!config->importMemory && !config->relocatable &&
name.startswith(".bss"))
s->isBss = true;
segments.push_back(s);
OutputSegment *s = nullptr;
// When running in relocatable mode we can't merge segments that are part
// of comdat groups since the ultimate linker needs to be able exclude or
// include them individually.
if (config->relocatable && !segment->getComdatName().empty()) {
s = createOutputSegment(name);
} else {
if (segmentMap.count(name) == 0)
segmentMap[name] = createOutputSegment(name);
s = segmentMap[name];
}
s->addInputSegment(segment);
LLVM_DEBUG(dbgs() << "added data: " << name << ": " << s->size << "\n");
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Object/WasmObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ using namespace object;

void WasmSymbol::print(raw_ostream &Out) const {
Out << "Name=" << Info.Name
<< ", Kind=" << toString(wasm::WasmSymbolType(Info.Kind))
<< ", Flags=" << Info.Flags;
<< ", Kind=" << toString(wasm::WasmSymbolType(Info.Kind)) << ", Flags=0x"
<< Twine::utohexstr(Info.Flags);
if (!isTypeData()) {
Out << ", ElemIndex=" << Info.ElementIndex;
} else if (isDefined()) {
Expand Down

0 comments on commit 73332d7

Please sign in to comment.