[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

diff --git a/lld/test/wasm/Inputs/comdat-data.s b/lld/test/wasm/Inputs/comdat-data.s
new file mode 100644 (file)
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 (file)
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 (file)
--- 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<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");

diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index b7bf770..946721a 100644 (file)
--- 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()) {