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

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Fri May 21 15:18:23 PDT 2021


Author: Sam Clegg
Date: 2021-05-21T15:16:47-07:00
New Revision: 8544b40b6e1d4e34650af66e25102ebcdf360e62

URL: https://github.com/llvm/llvm-project/commit/8544b40b6e1d4e34650af66e25102ebcdf360e62
DIFF: https://github.com/llvm/llvm-project/commit/8544b40b6e1d4e34650af66e25102ebcdf360e62.diff

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

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).

Differential Revision: https://reviews.llvm.org/D102937

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

Modified: 
    lld/test/wasm/data-segments.ll
    lld/test/wasm/relocation-bad-tls.s
    lld/wasm/OutputSections.cpp
    lld/wasm/Relocations.cpp
    lld/wasm/Writer.cpp

Removed: 
    lld/test/wasm/tls-no-shared.s


################################################################################
diff  --git a/lld/test/wasm/data-segments.ll b/lld/test/wasm/data-segments.ll
index 9c0382c693d2a..72c4d9117bbcc 100644
--- a/lld/test/wasm/data-segments.ll
+++ b/lld/test/wasm/data-segments.ll
@@ -107,7 +107,7 @@
 ;      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
@@ -121,17 +121,20 @@
 ; 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

diff  --git a/lld/test/wasm/relocation-bad-tls.s b/lld/test/wasm/relocation-bad-tls.s
index 93fa2bbd25919..fad9a264173bf 100644
--- a/lld/test/wasm/relocation-bad-tls.s
+++ b/lld/test/wasm/relocation-bad-tls.s
@@ -1,5 +1,5 @@
 # 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:

diff  --git a/lld/test/wasm/tls-no-shared.s b/lld/test/wasm/tls-non-shared-memory.s
similarity index 62%
rename from lld/test/wasm/tls-no-shared.s
rename to lld/test/wasm/tls-non-shared-memory.s
index d93a19c843382..38f95e5310b77 100644
--- a/lld/test/wasm/tls-no-shared.s
+++ b/lld/test/wasm/tls-non-shared-memory.s
@@ -9,7 +9,7 @@
 get_tls1:
   .functype get_tls1 () -> (i32)
   global.get __tls_base
-  i32.const tls1
+  i32.const tls1 at TLSREL
   i32.add
   end_function
 
@@ -39,6 +39,9 @@ tls1:
 # 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:
@@ -73,3 +76,31 @@ tls1:
 # 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

diff  --git a/lld/wasm/OutputSections.cpp b/lld/wasm/OutputSections.cpp
index 829fd97b2cf2d..44e6cdcaf68cb 100644
--- a/lld/wasm/OutputSections.cpp
+++ b/lld/wasm/OutputSections.cpp
@@ -144,8 +144,8 @@ void DataSection::finalizeContents() {
       });
 #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();

diff  --git a/lld/wasm/Relocations.cpp b/lld/wasm/Relocations.cpp
index ea0f6a0635068..eedd7d7e1be2c 100644
--- a/lld/wasm/Relocations.cpp
+++ b/lld/wasm/Relocations.cpp
@@ -115,12 +115,17 @@ void scanRelocations(InputChunk *chunk) {
         addGOTEntry(sym);
       break;
     case R_WASM_MEMORY_ADDR_TLS_SLEB:
-      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);
+      // 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;

diff  --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 90970ef326b9a..ce6171519202e 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -895,53 +895,37 @@ void Writer::createOutputSegments() {
 }
 
 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;
+    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();
+      uint64_t oldVA = inSeg->getVA();
 #endif
-        combined->addInputSegment(inSeg);
+      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);
+      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) {
@@ -1440,7 +1424,9 @@ void Writer::run() {
     }
   }
 
-  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();
   }


        


More information about the llvm-commits mailing list