[lld][WebAssembly] Do not merge comdat data segments

[sbc100]

Summary

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

[dschuff]

lld/test/wasm/relocatable-comdat.s

21

[sbc100]

• feedback

lld/test/wasm/Inputs/comdat-data.s

+.globl foo
+.section .data.foo,"G",@,foo,comdat
+foo:
+        .int32 42
+        .int32 43
+        .size foo, 8

lld/test/wasm/relocatable-comdat.s

+# 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:           [  ]

lld/wasm/Writer.cpp

private:
   void populateProducers();
   void populateTargetFeatures();
   void calculateInitFunctions();
   void calculateImports();
   void calculateExports();
   void calculateCustomSections();
   void calculateTypes();
   void createOutputSegments();
+  OutputSegment *createOutputSegment(StringRef name);
   void combineOutputSegments();
   void layoutMemory();
   void createHeader();
 
   void addSection(OutputSection *sec);
 
   void addSections();
 
if (name.startswith(".data."))
     return ".data";
   if (name.startswith(".bss."))
     return ".bss";
   if (name.startswith(".rodata."))
     return ".rodata";
   return name;
 }
 
-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 *Writer::createOutputSegment(StringRef name) {
+  OutputSegment *s = make<OutputSegment>(name);
+  if (!config->importMemory && !config->relocatable && name.startswith(".bss"))
+  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 = 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");
     }
   }
 
   // Sort segments by type, placing .bss last
   std::stable_sort(segments.begin(), segments.end(),

llvm/lib/Object/WasmObjectFile.cpp

 
 #define DEBUG_TYPE "wasm-object"
 
 using namespace llvm;
 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()) {
     Out << ", Segment=" << Info.DataRef.Segment;
     Out << ", Offset=" << Info.DataRef.Offset;
     Out << ", Size=" << Info.DataRef.Size;
   }
 }