From 73332d73e15f4fdf8e4240c585d0a334f23926f3 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Sat, 1 May 2021 15:37:40 -0700 Subject: [PATCH] [lld][WebAssembly] Do not merge comdat data segments 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: https://github.com/emscripten-core/emscripten/issues/9726 Differential Revision: https://reviews.llvm.org/D101703 --- lld/test/wasm/Inputs/comdat-data.s | 6 +++++ lld/test/wasm/relocatable-comdat.s | 46 ++++++++++++++++++++++++++++++++++++++ lld/wasm/Writer.cpp | 40 +++++++++++++++++++++------------ llvm/lib/Object/WasmObjectFile.cpp | 4 ++-- 4 files changed, 80 insertions(+), 16 deletions(-) create mode 100644 lld/test/wasm/Inputs/comdat-data.s create mode 100644 lld/test/wasm/relocatable-comdat.s diff --git a/lld/test/wasm/Inputs/comdat-data.s b/lld/test/wasm/Inputs/comdat-data.s new file mode 100644 index 0000000..d25258c --- /dev/null +++ b/lld/test/wasm/Inputs/comdat-data.s @@ -0,0 +1,6 @@ +.globl foo +.section .data.foo,"G",@,foo,comdat +foo: + .int32 42 + .int32 43 + .size foo, 8 diff --git a/lld/test/wasm/relocatable-comdat.s b/lld/test/wasm/relocatable-comdat.s new file mode 100644 index 0000000..e2b45d4 --- /dev/null +++ b/lld/test/wasm/relocatable-comdat.s @@ -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: [ ] diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp index 62df038..af30e13 100644 --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -77,6 +77,7 @@ private: void calculateCustomSections(); void calculateTypes(); void createOutputSegments(); + OutputSegment *createOutputSegment(StringRef name); void combineOutputSegments(); void layoutMemory(); void createHeader(); @@ -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(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(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"); diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp index b7bf770..946721a 100644 --- a/llvm/lib/Object/WasmObjectFile.cpp +++ b/llvm/lib/Object/WasmObjectFile.cpp @@ -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()) { -- 2.7.4