[lld][WebAssembly] Fix for PIC output + TLS + non-shared-memory

[sbc100]

Summary

Prior to this change build with -shared/-pie and using TLS (but
without -shared-memory) would hit this assert:

"Currenly only a single data segment is supported in PIC mode"

This is because we were not including TLS data when merging data
segments. However, when we build without shared-memory (i.e. without
threads) we effectively lower away TLS into a normal active data
segment.. so we were ending up with two active data segments: the merged
data, and the lowered TLS data.

To fix this problem we can instead avoid combining data segments at
all when running in shared memory mode (because in this case all
segment initialization is passive). And then in non-shared memory
mode we know that TLS has been lowered and therefore we can can
and should combine all segments.

So with this new behavior we have two different modes:

1. With shared memory / mutli-threaded: Never combine data segments since it is not necessary. (All data segments as passive already).

2. Wihout shared memory / single-threaded: Combine *all* data segments since we treat TLS as normal data. (We end up with a single active data segment).

[dschuff]

Does avoiding combing data segments in shared-memory mode mean that there will have to be a lot more passive segment initializations run at load time, instead of just one before?

[sbc100]

In D102937#2774353, @dschuff wrote:

Does avoiding combing data segments in shared-memory mode mean that there will have to be a lot more passive segment initializations run at load time, instead of just one before?

Not really.. most programs only have .data and .rodata and so they are mostly combined already. The normal merging of .data.* etc is still performed either way. That is controlled by --no-/merge-data-segments which is on by default. This second level of merging was never about performance and we would not have it all if we could do arithmetic in the const exprs that place the segments.

[dschuff]

Ah, got it. So is it the case that we have e.g. a single .tdata section that might be separate now, but just 1 or 2 and not scaling with the size of the program.
Mostly I just want to make sure we don't slow down loading too much, especially in the dylib case where it will already be slower than the non-library case.

lld/test/wasm/data-segments.ll

123–124

are these just data and rodata sections then?

lld/test/wasm/tls-non-shared-memory.s

102

I guess checking the content will ensure that the segments are merged. Does it also make sense to check that there are no additional segments?

[sbc100]

• feedback

[dschuff]

lld/wasm/Writer.cpp

1428

[sbc100]

lld/test/wasm/data-segments.ll

123–124

Yes, with passive (shared memory) segments we no longer combine them so they are separate. (as they would be for normal non-PIC code).

Added more checks below regarding the actual data.

lld/test/wasm/tls-non-shared-memory.s

102

I think this matches the above tests that checks for the two separate segments. I added a CHECK-NEXT to both of them to make sure there are no subsequent segments. I also added a comment here.

lld/test/wasm/data-segments.ll

 ; PASSIVE-NEXT:      - Index:           1
 ; PASSIVE-NEXT:        Name:            __wasm_init_tls
 ; PASSIVE-NEXT:      - Index:           2
 ; PASSIVE-NEXT:        Name:            __wasm_init_memory
 
 ;      PASSIVE-PIC:  - Type:            START
 ; PASSIVE-PIC-NEXT:    StartFunction:   2
 ; PASSIVE-PIC-NEXT:  - Type:            DATACOUNT
-; PASSIVE-PIC-NEXT:    Count:           1
+; PASSIVE-PIC-NEXT:    Count:           2
 ; PASSIVE-PIC-NEXT:  - Type:            CODE
 ; PASSIVE-PIC-NEXT:    Functions:
 ; PASSIVE-PIC-NEXT:      - Index:           0
 ; PASSIVE-PIC-NEXT:        Locals:          []
 ; PASSIVE-PIC-NEXT:        Body:            10030B
 ; PASSIVE-PIC-NEXT:      - Index:           1
 ; PASSIVE-PIC-NEXT:        Locals:          []
 ; PASSIVE-PIC-NEXT:        Body:            0B
 ; PASSIVE-PIC-NEXT:      - Index:           2
 ; PASSIVE-PIC-NEXT:        Locals:
 ; PASSIVE32-PIC-NEXT:          - Type:            I32
 ; PASSIVE64-PIC-NEXT:          - Type:            I64
 ; PASSIVE-PIC-NEXT:            Count:           1
-; PASSIVE32-PIC-NEXT:        Body:            230141B4CE006A2100200041004101FE480200044020004101427FFE0102001A05410023016A410041B4CE00FC08000020004102FE1702002000417FFE0002001A0BFC09000B
-; PASSIVE64-PIC-NEXT:        Body:            230142B4CE006A2100200041004101FE480200044020004101427FFE0102001A05420023016A410041B4CE00FC08000020004102FE1702002000417FFE0002001A0BFC09000B
+; PASSIVE32-PIC-NEXT:        Body:            230141B4CE006A2100200041004101FE480200044020004101427FFE0102001A05410023016A4100410DFC080000411023016A41004114FC08010020004102FE1702002000417FFE0002001A0BFC0900FC09010B
+; PASSIVE64-PIC-NEXT:        Body:            230142B4CE006A2100200041004101FE480200044020004101427FFE0102001A05420023016A4100410DFC080000421023016A41004114FC08010020004102FE1702002000417FFE0002001A0BFC0900FC09010B
 ; PASSIVE-PIC-NEXT:      - Index:           3
 ; PASSIVE-PIC-NEXT:        Locals:          []
 ; PASSIVE-PIC-NEXT:        Body:            0B
 ; PASSIVE-PIC-NEXT:  - Type:            DATA
 ; PASSIVE-PIC-NEXT:    Segments:
-; PASSIVE-PIC-NEXT:      - SectionOffset:   4
+; PASSIVE-PIC-NEXT:      - SectionOffset:   3
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
-
-;      PASSIVE-PIC:  - Type:            CUSTOM
+; PASSIVE-PIC-NEXT:        Content:         636F6E7374616E74000000002B
+; PASSIVE-PIC-NEXT:      - SectionOffset:   18
+; PASSIVE-PIC-NEXT:        InitFlags:       1
+; PASSIVE-PIC-NEXT:        Content:         68656C6C6F00676F6F646279650000002A000000
+; PASSIVE-PIC-NEXT:  - Type:            CUSTOM
 ; PASSIVE-PIC-NEXT:    Name:            name
 ; PASSIVE-PIC-NEXT:    FunctionNames:
 ; PASSIVE-PIC-NEXT:      - Index:           0
 ; PASSIVE-PIC-NEXT:        Name:            __wasm_call_ctors
 ; PASSIVE-PIC-NEXT:      - Index:           1
 ; PASSIVE-PIC-NEXT:        Name:            __wasm_init_tls
 ; PASSIVE-PIC-NEXT:      - Index:           2
 ; PASSIVE-PIC-NEXT:        Name:            __wasm_init_memory
 ; PASSIVE-PIC-NEXT:      - Index:           3
 ; PASSIVE-PIC-NEXT:        Name:            __wasm_apply_data_relocs

lld/test/wasm/relocation-bad-tls.s

 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
-# RUN: not wasm-ld %t.o -o %t.wasm 2>&1 | FileCheck %s
+# RUN: not wasm-ld --shared-memory %t.o -o %t.wasm 2>&1 | FileCheck %s
 
 .globl _start
 _start:
   .functype _start () -> ()
   i32.const foo@TLSREL
   i32.const bar@TLSREL
   end_function
 

lld/test/wasm/tls-non-shared-memory.s

 # Test that linking without shared memory causes __tls_base to be
 # internalized
 
 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
 
 .globaltype __tls_base, i32
 
 .globl get_tls1
 get_tls1:
   .functype get_tls1 () -> (i32)
   global.get __tls_base
-  i32.const tls1
+  i32.const tls1@TLSREL
   i32.add
   end_function
 
 .section  .data.no_tls,"",@
 .globl  no_tls
 .p2align  2
 no_tls:
   .int32  42
.section  .custom_section.target_features,"",@
   .ascii  "atomics"
   .int8 43
   .int8 11
   .ascii  "bulk-memory"
 
 # RUN: wasm-ld --no-gc-sections --no-entry -o %t.wasm %t.o
 # RUN: obj2yaml %t.wasm | FileCheck %s
 
+# RUN: wasm-ld --experimental-pic -shared -o %t.so %t.o
+# RUN: obj2yaml %t.so | FileCheck %s --check-prefix=PIC
+
 #      CHECK:   - Type:            GLOBAL
 # __stack_pointer
 # CHECK-NEXT:     Globals:
 # CHECK-NEXT:       - Index:           0
 # CHECK-NEXT:         Type:            I32
 # CHECK-NEXT:         Mutable:         true
 # CHECK-NEXT:         InitExpr:
 # CHECK-NEXT:           Opcode:          I32_CONST

 # CHECK-NEXT:        Content:         2B000000
 # .tdata
 # CHECK-NEXT:      - SectionOffset:   17
 # CHECK-NEXT:        InitFlags:       0
 # CHECK-NEXT:        Offset:
 # CHECK-NEXT:          Opcode:          I32_CONST
 # CHECK-NEXT:          Value:           1028
 # CHECK-NEXT:        Content:         2A000000
+# CHECK-NEXT:  - Type:            CUSTOM
+
+
+# In PIC mode we expect TLS data and non-TLS data to be merged into
+# a single segment which is initialized via the  __memory_base import
+
+#      PIC:  - Type:            IMPORT
+# PIC-NEXT:    Imports:
+# PIC-NEXT:      - Module:          env
+# PIC-NEXT:        Field:           memory
+# PIC-NEXT:        Kind:            MEMORY
+# PIC-NEXT:        Memory:
+# PIC-NEXT:          Minimum:         0x1
+# PIC-NEXT:      - Module:          env
+# PIC-NEXT:        Field:           __memory_base
+# PIC-NEXT:        Kind:            GLOBAL
+# PIC-NEXT:        GlobalType:      I32
+
+# .tdata and .data are combined into single segment in PIC mode.
+#      PIC:  - Type:            DATA
+# PIC-NEXT:    Segments:
+# PIC-NEXT:      - SectionOffset:   6
+# PIC-NEXT:        InitFlags:       0
+# PIC-NEXT:        Offset:
+# PIC-NEXT:          Opcode:          GLOBAL_GET
+# PIC-NEXT:          Index:           0
+# PIC-NEXT:        Content:         2B0000002A000000
+# PIC-NEXT:  - Type:            CUSTOM

lld/wasm/OutputSections.cpp

 
 #ifndef NDEBUG
   unsigned activeCount = std::count_if(
       segments.begin(), segments.end(), [](OutputSegment *segment) {
         return (segment->initFlags & WASM_DATA_SEGMENT_IS_PASSIVE) == 0;
       });
 #endif
 
-  assert((!config->isPic || activeCount <= 1) &&
-         "Currenly only a single data segment is supported in PIC mode");
+  assert((config->sharedMemory || !config->isPic || activeCount <= 1) &&
+         "output segments should have been combined by now");
 
   writeUleb128(os, segmentCount, "data segment count");
   os.flush();
   bodySize = dataSectionHeader.size();
 
   for (OutputSegment *segment : segments) {
     if (segment->isBss)
       continue;

lld/wasm/Relocations.cpp

case R_WASM_TABLE_INDEX_REL_SLEB64:
       out.elemSec->addEntry(cast<FunctionSymbol>(sym));
       break;
     case R_WASM_GLOBAL_INDEX_LEB:
     case R_WASM_GLOBAL_INDEX_I32:
       if (!isa<GlobalSymbol>(sym))
         addGOTEntry(sym);
       break;
     case R_WASM_MEMORY_ADDR_TLS_SLEB:
+      // In single-threaded builds TLS is lowered away and TLS data can be
+      // merged with normal data and allowing TLS relocation in non-TLS
+      // segments.
+      if (config->sharedMemory) {
         if (auto *D = dyn_cast<DefinedData>(sym)) {
           if (!D->segment->outputSeg->isTLS()) {
             error(toString(file) + ": relocation " +
                   relocTypeToString(reloc.Type) + " cannot be used against `" +
                   toString(*sym) +
                   "` in non-TLS section: " + D->segment->outputSeg->name);
           }
         }
+      }
       break;
     }
 
     if (config->isPic) {
       switch (reloc.Type) {
       case R_WASM_TABLE_INDEX_SLEB:
       case R_WASM_TABLE_INDEX_SLEB64:
       case R_WASM_MEMORY_ADDR_SLEB:

lld/wasm/Writer.cpp

void Writer::createOutputSegments() {
 
   // Merge MergeInputSections into a single MergeSyntheticSection.
   LLVM_DEBUG(dbgs() << "-- finalize input semgments\n");
   for (OutputSegment *seg : segments)
     seg->finalizeInputSegments();
 }
 
 void Writer::combineOutputSegments() {
-  // With PIC code we currently only support a single data segment since
-  // we only have a single __memory_base to use as our base address.
-  // This pass combines all non-TLS data segments into a single .data
-  // segment.
+  // With PIC code we currently only support a single active data segment since
+  // we only have a single __memory_base to use as our base address.  This pass
+  // combines all data segments into a single .data segment.
   // This restructions can be relaxed once we have extended constant
   // expressions available:
   // https://github.com/WebAssembly/extended-const
-  assert(config->isPic);
+  assert(config->isPic && !config->sharedMemory);
   if (segments.size() <= 1)
     return;
-  OutputSegment *combined = nullptr;
-  std::vector<OutputSegment *> new_segments;
+  OutputSegment *combined = make<OutputSegment>(".data");
+  combined->startVA = segments[0]->startVA;
   for (OutputSegment *s : segments) {
-    if (s->isTLS()) {
-      new_segments.push_back(s);
-    } else {
-      if (!combined) {
-        LLVM_DEBUG(dbgs() << "created combined output segment: .data\n");
-        combined = make<OutputSegment>(".data");
-        combined->startVA = s->startVA;
-        if (config->sharedMemory)
-          combined->initFlags = WASM_DATA_SEGMENT_IS_PASSIVE;
-      }
-      bool first = true;
-      for (InputChunk *inSeg : s->inputSegments) {
-        if (first)
-          inSeg->alignment = std::max(inSeg->alignment, s->alignment);
-        first = false;
 #ifndef NDEBUG
-        uint64_t oldVA = inSeg->getVA();
 #endif
-        combined->addInputSegment(inSeg);
 #ifndef NDEBUG
-        uint64_t newVA = inSeg->getVA();
-        LLVM_DEBUG(dbgs() << "added input segment. name=" << inSeg->getName()
-                          << " oldVA=" << oldVA << " newVA=" << newVA << "\n");
-        assert(oldVA == newVA);
 #endif
-      }
-    }
-  }
-  if (combined) {
-    new_segments.push_back(combined);
-    segments = new_segments;
-    for (size_t i = 0; i < segments.size(); ++i)
-      segments[i]->index = i;
-  }
+
+  segments = {combined};
 }
 
 static void createFunction(DefinedFunction *func, StringRef bodyContent) {
   std::string functionBody;
   {
     raw_string_ostream os(functionBody);
     writeUleb128(os, bodyContent.size(), "function size");
     os << bodyContent;
for (auto &name : config->requiredExports) {
     if (!sym || !sym->isDefined()) {
       if (config->unresolvedSymbols == UnresolvedPolicy::ReportError)
         error(Twine("symbol exported via --export not found: ") + name);
       if (config->unresolvedSymbols == UnresolvedPolicy::Warn)
         warn(Twine("symbol exported via --export not found: ") + name);
     }
   }
 
-  if (config->isPic) {
+  if (config->isPic && !config->sharedMemory) {
+    // In shared memory mode all data segments are passive and initilized
+    // via __wasm_init_memory.
     log("-- combineOutputSegments");
     combineOutputSegments();
   }
 
   log("-- createSyntheticSectionsPostLayout");
   createSyntheticSectionsPostLayout();
   log("-- populateProducers");
   populateProducers();