Skip to content

[WebAssembly] Fix issues combining coop threads and PIC - #208332

Merged
sbc100 merged 1 commit into
llvm:mainfrom
alexcrichton:wasm-ld-coop-pic-data-segments
Aug 13, 2026
Merged

[WebAssembly] Fix issues combining coop threads and PIC#208332
sbc100 merged 1 commit into
llvm:mainfrom
alexcrichton:wasm-ld-coop-pic-data-segments

Conversation

@alexcrichton

Copy link
Copy Markdown
Contributor

This commit fixes a few issues that have surfaced in wasm-ld's handling of --cooperative-threading with -shared. Two primary issues fixed are:

  • The __wasm_init_memory function was not valid as it referenced nonexistent locals. This was fixed by adjusting how locals are declared to make this a bit more flexible.
  • Combining data segments for PIC without extended-const is generalized to only operate on active data segments and is now orthogonal to threading. With coop threads there's a mixture of passive/active segments (TLS is passive, other data is active) and the active segments still need combining while TLS stays passive.

The latter fix ended up touching a few more areas. The first is that .tdata sections are now sorted just before .bss, the end section, rather than first. This is done to ensure that active segments when combined can indeed start at a relative address of 0 (as required without extended-const). This change resulted in a number of adjustments for tests as the data is now sorted differently. Additionally relocations in TLS segments needed adjusting since the previous implementation implicitly only worked if .tdata is first.

@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-lld-wasm

Author: Alex Crichton (alexcrichton)

Changes

This commit fixes a few issues that have surfaced in wasm-ld's handling of --cooperative-threading with -shared. Two primary issues fixed are:

  • The __wasm_init_memory function was not valid as it referenced nonexistent locals. This was fixed by adjusting how locals are declared to make this a bit more flexible.
  • Combining data segments for PIC without extended-const is generalized to only operate on active data segments and is now orthogonal to threading. With coop threads there's a mixture of passive/active segments (TLS is passive, other data is active) and the active segments still need combining while TLS stays passive.

The latter fix ended up touching a few more areas. The first is that .tdata sections are now sorted just before .bss, the end section, rather than first. This is done to ensure that active segments when combined can indeed start at a relative address of 0 (as required without extended-const). This change resulted in a number of adjustments for tests as the data is now sorted differently. Additionally relocations in TLS segments needed adjusting since the previous implementation implicitly only worked if .tdata is first.


Patch is 23.87 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208332.diff

10 Files Affected:

  • (modified) lld/test/wasm/compress-relocs.s (+6-6)
  • (modified) lld/test/wasm/compress-relocs64.s (+6-6)
  • (added) lld/test/wasm/cooperative-threading-pic.s (+77)
  • (modified) lld/test/wasm/data-segments.ll (+24-24)
  • (modified) lld/test/wasm/runtime-relocations-himem.s (+2-2)
  • (modified) lld/test/wasm/tls-non-shared-memory.s (+7-7)
  • (modified) lld/test/wasm/tls-relocations.s (+2-2)
  • (modified) lld/wasm/InputChunks.cpp (+8-2)
  • (modified) lld/wasm/OutputSections.cpp (+1-1)
  • (modified) lld/wasm/Writer.cpp (+59-31)
diff --git a/lld/test/wasm/compress-relocs.s b/lld/test/wasm/compress-relocs.s
index 37f1b3b170ff7..e872b941416af 100644
--- a/lld/test/wasm/compress-relocs.s
+++ b/lld/test/wasm/compress-relocs.s
@@ -47,16 +47,16 @@ test_memory_and_indirect_call_relocs:
   end_function
 
 # CHECK:    test_memory_and_indirect_call_relocs
-# CHECK:      41 90 80 84 80 00                 i32.const      65552
+# CHECK:      41 80 80 84 80 00                 i32.const      65536
 # CHECK:      11 80 80 80 80 00 80 80 80 80 00  call_indirect  0
-# CHECK:      28 02 94 80 84 80 00              i32.load       65556
+# CHECK:      28 02 84 80 84 80 00              i32.load       65540
 # CHECK:      11 81 80 80 80 00 80 80 80 80 00  call_indirect  1
 # CHECK:      41 81 80 80 80 00                 i32.const      1
 # CHECK:      11 80 80 80 80 00 80 80 80 80 00  call_indirect  0
 # COMPRESS: test_memory_and_indirect_call_relocs
-# COMPRESS:   41 90 80 04                       i32.const      65552
+# COMPRESS:   41 80 80 04                       i32.const      65536
 # COMPRESS:   11 00 00                          call_indirect  0
-# COMPRESS:   28 02 94 80 04                    i32.load       65556
+# COMPRESS:   28 02 84 80 04                    i32.load       65540
 # COMPRESS:   11 01 00                          call_indirect  1
 # COMPRESS:   41 01                             i32.const      1
 # COMPRESS:   11 00 00                          call_indirect  0
@@ -91,11 +91,11 @@ test_relative_relocs:
   end_function
 
 # CHECK:    test_relative_relocs
-# CHECK:      41 90 80 84 80 00  i32.const  65552
+# CHECK:      41 80 80 84 80 00  i32.const  65536
 # CHECK:      41 81 80 80 80 00  i32.const  1
 # CHECK:      41 83 80 80 80 00  i32.const  3
 # COMPRESS: test_relative_relocs
-# COMPRESS:   41 90 80 04        i32.const  65552
+# COMPRESS:   41 80 80 04        i32.const  65536
 # COMPRESS:   41 01              i32.const  1
 # COMPRESS:   41 03              i32.const  3
 
diff --git a/lld/test/wasm/compress-relocs64.s b/lld/test/wasm/compress-relocs64.s
index f3ff646cc3b1c..2dd18d604df41 100644
--- a/lld/test/wasm/compress-relocs64.s
+++ b/lld/test/wasm/compress-relocs64.s
@@ -36,12 +36,12 @@ test_memory_and_indirect_call_relocs:
   end_function
 
 # CHECK:    test_memory_and_indirect_call_relocs
-# CHECK:      42 90 80 84 80 80 80 80 80 80 00     i64.const 65552
-# CHECK:      29 03 98 80 84 80 80 80 80 80 80 00  i64.load  65560
+# CHECK:      42 80 80 84 80 80 80 80 80 80 00     i64.const 65536
+# CHECK:      29 03 88 80 84 80 80 80 80 80 80 00  i64.load  65544
 # CHECK:      42 81 80 80 80 80 80 80 80 80 00     i64.const 1
 # COMPRESS: test_memory_and_indirect_call_relocs
-# COMPRESS:   42 90 80 04                          i64.const 65552
-# COMPRESS:   29 03 98 80 04                       i64.load  65560
+# COMPRESS:   42 80 80 04                          i64.const 65536
+# COMPRESS:   29 03 88 80 04                       i64.load  65544
 # COMPRESS:   42 01                                i64.const 1
 
   .globl test_relative_relocs
@@ -56,11 +56,11 @@ test_relative_relocs:
   end_function
 
 # CHECK:    test_relative_relocs
-# CHECK:      42 90 80 84 80 80 80 80 80 80 00  i64.const 65552
+# CHECK:      42 80 80 84 80 80 80 80 80 80 00  i64.const 65536
 # CHECK:      42 81 80 80 80 80 80 80 80 80 00  i64.const 1
 # CHECK:      42 83 80 80 80 80 80 80 80 80 00  i64.const 3
 # COMPRESS: test_relative_relocs
-# COMPRESS:   42 90 80 04                       i64.const 65552
+# COMPRESS:   42 80 80 04                       i64.const 65536
 # COMPRESS:   42 01                             i64.const 1
 # COMPRESS:   42 03                             i64.const 3
 
diff --git a/lld/test/wasm/cooperative-threading-pic.s b/lld/test/wasm/cooperative-threading-pic.s
new file mode 100644
index 0000000000000..af2d366518b20
--- /dev/null
+++ b/lld/test/wasm/cooperative-threading-pic.s
@@ -0,0 +1,77 @@
+# Test --cooperative-threading combined with PIC output
+
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld -shared --cooperative-threading -no-gc-sections -o %t.so %t.o
+# RUN: obj2yaml %t.so | FileCheck %s
+# RUN: llvm-objdump --disassemble-symbols=__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.so | FileCheck %s --check-prefix=DIS
+
+.globl         __wasm_get_tls_base
+__wasm_get_tls_base:
+  .functype   __wasm_get_tls_base () -> (i32)
+  i32.const 0
+  end_function
+
+.globl get_tls1
+get_tls1:
+  .functype get_tls1 () -> (i32)
+  call __wasm_get_tls_base
+  i32.const tls1@TLSREL
+  i32.add
+  end_function
+
+.section  .rodata.ro_data,"",@
+.globl  ro_data
+.p2align  2
+ro_data:
+  .int32  2
+  .size ro_data, 4
+
+.section  .data.rw_data,"",@
+.globl  rw_data
+.p2align  2
+rw_data:
+  .int32  1
+  .size rw_data, 4
+
+.section  .tdata.tls1,"T",@
+.globl  tls1
+.p2align  2
+tls1:
+  .int32  43
+  .size tls1, 4
+
+.section  .custom_section.target_features,"",@
+  .int8 1
+  .int8 43
+  .int8 11
+  .ascii  "bulk-memory"
+
+# The active .rodata and .data segments are combined into a single active
+# segment at __memory_base; the TLS segment remains passive.
+# CHECK:       - Type:            DATACOUNT
+# CHECK-NEXT:    Count:           2
+# CHECK:       - Type:            DATA{{$}}
+# CHECK-NEXT:    Segments:
+# CHECK-NEXT:      - SectionOffset:   6
+# CHECK-NEXT:        InitFlags:       0
+# CHECK-NEXT:        Offset:
+# CHECK-NEXT:          Opcode:          GLOBAL_GET
+# CHECK-NEXT:          Index:           {{[0-9]+}}
+# CHECK-NEXT:        Content:         '0200000001000000'
+# CHECK-NEXT:      - SectionOffset:   {{[0-9]+}}
+# CHECK-NEXT:        InitFlags:       1
+# CHECK-NEXT:        Content:         2B000000
+# CHECK-NEXT:  - Type:            CUSTOM
+
+# DIS:      <__wasm_init_memory>:
+# DIS-NEXT:   .local i32
+# DIS-NEXT:   i32.const 8
+# DIS-NEXT:   global.get {{[0-9]+}}
+# DIS-NEXT:   i32.add
+# DIS-NEXT:   local.tee 0
+# DIS-NEXT:   call {{[0-9]+}}
+# DIS-NEXT:   local.get 0
+# DIS-NEXT:   i32.const 0
+# DIS-NEXT:   i32.const 4
+# DIS-NEXT:   memory.init 1, 0
+# DIS-NEXT:   end
diff --git a/lld/test/wasm/data-segments.ll b/lld/test/wasm/data-segments.ll
index 7a18fd5efb655..5073e9c4cc8d1 100644
--- a/lld/test/wasm/data-segments.ll
+++ b/lld/test/wasm/data-segments.ll
@@ -93,7 +93,7 @@
 ; ACTIVE-PIC-NEXT:        Offset:
 ; ACTIVE-PIC-NEXT:          Opcode:          GLOBAL_GET
 ; ACTIVE-PIC-NEXT:          Index:           1
-; ACTIVE-PIC-NEXT:        Content:         63000000636F6E7374616E74000000002B00000068656C6C6F00676F6F646279650000002A000000
+; ACTIVE-PIC-NEXT:        Content:         636F6E7374616E74000000002B00000068656C6C6F00676F6F646279650000002A00000063000000
 
 ; PASSIVE-LABEL: - Type:            START
 ; PASSIVE-NEXT:    StartFunction:   2
@@ -114,13 +114,13 @@
 ; PASSIVE-NEXT:    Segments:
 ; PASSIVE-NEXT:      - SectionOffset:   3
 ; PASSIVE-NEXT:        InitFlags:       1
-; PASSIVE-NEXT:        Content:         '63000000'
-; PASSIVE-NEXT:      - SectionOffset:   9
-; PASSIVE-NEXT:        InitFlags:       1
 ; PASSIVE-NEXT:        Content:         636F6E7374616E74000000002B
-; PASSIVE-NEXT:      - SectionOffset:   24
+; PASSIVE-NEXT:      - SectionOffset:   18
 ; PASSIVE-NEXT:        InitFlags:       1
 ; PASSIVE-NEXT:        Content:         68656C6C6F00676F6F646279650000002A000000
+; PASSIVE-NEXT:      - SectionOffset:   40
+; PASSIVE-NEXT:        InitFlags:       1
+; PASSIVE-NEXT:        Content:         '63000000'
 ; PASSIVE-NEXT:  - Type:            CUSTOM
 ; PASSIVE-NEXT:    Name:            name
 ; PASSIVE-NEXT:    FunctionNames:
@@ -153,13 +153,13 @@
 ; PASSIVE-PIC-NEXT:    Segments:
 ; PASSIVE-PIC-NEXT:      - SectionOffset:   3
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
-; PASSIVE-PIC-NEXT:        Content:         '63000000'
-; PASSIVE-PIC-NEXT:      - SectionOffset:   9
-; PASSIVE-PIC-NEXT:        InitFlags:       1
 ; PASSIVE-PIC-NEXT:        Content:         636F6E7374616E74000000002B
-; PASSIVE-PIC-NEXT:      - SectionOffset:   24
+; PASSIVE-PIC-NEXT:      - SectionOffset:   18
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
 ; PASSIVE-PIC-NEXT:        Content:         68656C6C6F00676F6F646279650000002A000000
+; PASSIVE-PIC-NEXT:      - SectionOffset:   40
+; PASSIVE-PIC-NEXT:        InitFlags:       1
+; PASSIVE-PIC-NEXT:        Content:         '63000000'
 ; PASSIVE-PIC-NEXT:  - Type:            CUSTOM
 ; PASSIVE-PIC-NEXT:    Name:            name
 ; PASSIVE-PIC-NEXT:    FunctionNames:
@@ -212,34 +212,34 @@
 ; DIS-NEXT:            end
 
 ; NOPIC-DIS-NEXT:      [[PTR]].const   65536
-; NOPIC-DIS-NEXT:      [[PTR]].const   65536
-; NOPIC-DIS-NEXT:      global.set      1
 ; PIC-DIS-NEXT:        [[PTR]].const   0
 ; PIC-DIS-NEXT:        global.get      1
 ; PIC-DIS-NEXT:        [[PTR]].add
-; PIC-DIS-NEXT:        local.tee       1
-; PIC-DIS-NEXT:        global.set      {{\d*}}
-; PIC-DIS-NEXT:        local.get       1
+
 ; DIS-NEXT:            i32.const       0
-; DIS-NEXT:            i32.const       4
-; DIS-NEXT:            memory.init  0, 0
+; DIS-NEXT:            i32.const       13
+; DIS-NEXT:            memory.init     0, 0
 
-; NOPIC-DIS-NEXT:      [[PTR]].const   65540
-; PIC-DIS-NEXT:        [[PTR]].const   4
+; NOPIC-DIS-NEXT:      [[PTR]].const   65552
+; PIC-DIS-NEXT:        [[PTR]].const   16
 ; PIC-DIS-NEXT:        global.get      1
 ; PIC-DIS-NEXT:        [[PTR]].add
 
 ; DIS-NEXT:            i32.const       0
-; DIS-NEXT:            i32.const       13
+; DIS-NEXT:            i32.const       20
 ; DIS-NEXT:            memory.init     1, 0
 
-; NOPIC-DIS-NEXT:      [[PTR]].const   65556
-; PIC-DIS-NEXT:        [[PTR]].const   20
+; NOPIC-DIS-NEXT:      [[PTR]].const   65572
+; NOPIC-DIS-NEXT:      [[PTR]].const   65572
+; NOPIC-DIS-NEXT:      global.set      1
+; PIC-DIS-NEXT:        [[PTR]].const   36
 ; PIC-DIS-NEXT:        global.get      1
 ; PIC-DIS-NEXT:        [[PTR]].add
-
+; PIC-DIS-NEXT:        local.tee       1
+; PIC-DIS-NEXT:        global.set      {{\d*}}
+; PIC-DIS-NEXT:        local.get       1
 ; DIS-NEXT:            i32.const       0
-; DIS-NEXT:            i32.const       20
+; DIS-NEXT:            i32.const       4
 ; DIS-NEXT:            memory.init     2, 0
 ; NOPIC-DIS-NEXT:      [[PTR]].const   65576
 ; PIC-DIS-NEXT:        [[PTR]].const   40
@@ -272,6 +272,6 @@
 ; DIS-NEXT:           memory.atomic.wait32    0
 ; DIS-NEXT:           drop
 ; DIS-NEXT:          end
+; DIS-NEXT:          data.drop       0
 ; DIS-NEXT:          data.drop       1
-; DIS-NEXT:          data.drop       2
 ; DIS-NEXT:         end
diff --git a/lld/test/wasm/runtime-relocations-himem.s b/lld/test/wasm/runtime-relocations-himem.s
index 2d39a204c7904..2026e62a5af86 100644
--- a/lld/test/wasm/runtime-relocations-himem.s
+++ b/lld/test/wasm/runtime-relocations-himem.s
@@ -47,14 +47,14 @@ data_sym:
 
 # CHECK: <__wasm_apply_data_relocs>:
 # CHECK-EMPTY:
-# CHECK-NEXT:  i32.const -2147483636
+# CHECK-NEXT:  i32.const -2147483644
 # CHECK-NEXT:  global.get 0
 # CHECK-NEXT:  i32.store 0
 # CHECK-NEXT:  end
 
 # CHECK: <__wasm_apply_tls_relocs>:
 # CHECK-EMPTY:
-# CHECK-NEXT:  i32.const -2147483644
+# CHECK-NEXT:  i32.const -2147483636
 # CHECK-NEXT:  global.get 0
 # CHECK-NEXT:  i32.store 0
 # CHECK-NEXT:  end
diff --git a/lld/test/wasm/tls-non-shared-memory.s b/lld/test/wasm/tls-non-shared-memory.s
index 0a87ade7efb2e..21082a5603e7f 100644
--- a/lld/test/wasm/tls-non-shared-memory.s
+++ b/lld/test/wasm/tls-non-shared-memory.s
@@ -70,14 +70,14 @@ tls1:
 # CHECK-NEXT:         Mutable:         false
 # CHECK-NEXT:         InitExpr:
 # CHECK-NEXT:           Opcode:          I32_CONST
-# CHECK-NEXT:           Value:           65536
+# CHECK-NEXT:           Value:           65540
 # GOT.data.internal.tls1
 # CHECK-NEXT:       - Index:           2
 # CHECK-NEXT:         Type:            I32
 # CHECK-NEXT:         Mutable:         false
 # CHECK-NEXT:         InitExpr:
 # CHECK-NEXT:           Opcode:          I32_CONST
-# CHECK-NEXT:           Value:           65536
+# CHECK-NEXT:           Value:           65540
 # CHECK-NEXT:   - Type:            EXPORT
 
 #      CHECK:  - Type:            DATA
@@ -88,14 +88,14 @@ tls1:
 # CHECK-NEXT:        Offset:
 # CHECK-NEXT:          Opcode:          I32_CONST
 # CHECK-NEXT:          Value:           65536
-# CHECK-NEXT:        Content:         2B000000
+# CHECK-NEXT:        Content:         2A000000
 # .tdata
 # CHECK-NEXT:      - SectionOffset:   19
 # CHECK-NEXT:        InitFlags:       0
 # CHECK-NEXT:        Offset:
 # CHECK-NEXT:          Opcode:          I32_CONST
 # CHECK-NEXT:          Value:           65540
-# CHECK-NEXT:        Content:         2A000000
+# CHECK-NEXT:        Content:         2B000000
 # CHECK-NEXT:  - Type:            CUSTOM
 
 # The constant value here which we add to `__tls_base` should not be absolute
@@ -146,7 +146,7 @@ tls1:
 # PIC-NEXT:        Offset:
 # PIC-NEXT:          Opcode:          GLOBAL_GET
 # PIC-NEXT:          Index:           {{\d*}}
-# PIC-NEXT:        Content:         2B0000002A000000
+# PIC-NEXT:        Content:         2A0000002B000000
 # PIC-NEXT:  - Type:            CUSTOM
 
 # Unless we have extended-const, in which case the merging is not needed.
@@ -160,7 +160,7 @@ tls1:
 # EXT-CONST-NEXT:        Offset:
 # EXT-CONST-NEXT:          Opcode:          GLOBAL_GET
 # EXT-CONST-NEXT:          Index:           1
-# EXT-CONST-NEXT:        Content:         2B000000
+# EXT-CONST-NEXT:        Content:         2A000000
 # EXT-CONST-NEXT:      - SectionOffset:   18
 # EXT-CONST-NEXT:        InitFlags:       0
 # EXT-CONST-NEXT:        Offset:
@@ -168,4 +168,4 @@ tls1:
 # This instruction sequence decodes to:
 # (global.get[0x23] 0x1 i32.const[0x41] 0x04 i32.add[0x6A] end[0x0b])
 # EXT-CONST-NEXT:          Body:            230141046A0B
-# EXT-CONST-NEXT:        Content:         2A000000
+# EXT-CONST-NEXT:        Content:         2B000000
diff --git a/lld/test/wasm/tls-relocations.s b/lld/test/wasm/tls-relocations.s
index 9679074d6a0db..b5d1304177744 100644
--- a/lld/test/wasm/tls-relocations.s
+++ b/lld/test/wasm/tls-relocations.s
@@ -64,7 +64,7 @@ tls_sym:
 # ASM-NEXT:                 local.get 0
 # ASM-NEXT:                 i32.const 0
 # ASM-NEXT:                 i32.const 16
-# ASM-NEXT:                 memory.init 0, 0
+# ASM-NEXT:                 memory.init 1, 0
 # call to __wasm_apply_tls_relocs
 # ASM-NEXT:                 call  3
 # ASM-NEXT:                 end
@@ -75,7 +75,7 @@ tls_sym:
 # ASM-NEXT:                 global.get  3
 # ASM-NEXT:                 i32.add
 # ASM-NEXT:                 global.get  1
-# ASM-NEXT:                 i32.const 20
+# ASM-NEXT:                 i32.const 4
 # ASM-NEXT:                 i32.add
 # ASM-NEXT:                 i32.store 0
 # ASM-NEXT:                 i32.const 12
diff --git a/lld/wasm/InputChunks.cpp b/lld/wasm/InputChunks.cpp
index 99623e7c9aefe..e2d900a43ab19 100644
--- a/lld/wasm/InputChunks.cpp
+++ b/lld/wasm/InputChunks.cpp
@@ -445,12 +445,18 @@ bool InputChunk::generateRelocationCode(raw_ostream &os) const {
       continue;
     }
 
-    uint64_t offset = getVA(rel.Offset) - getInputSectionOffset();
+    // Calculate the address at which to apply the relocation. Note that PIC TLS
+    // segments are handled slightly differently here because at runtime they're
+    // a relative offset from `__tls_base`.
+    uint64_t offset;
+    if (ctx.isPic && isTLS())
+      offset = getChunkOffset(rel.Offset) - getInputSectionOffset();
+    else
+      offset = getVA(rel.Offset) - getInputSectionOffset();
     LLVM_DEBUG(dbgs() << "gen reloc: type=" << relocTypeToString(rel.Type)
                       << " addend=" << rel.Addend << " index=" << rel.Index
                       << " output offset=" << offset << "\n");
 
-    // Calculate the address at which to apply the relocation
     writePtrConst(os, offset, is64, "offset");
 
     // In PIC mode we need to add the __memory_base
diff --git a/lld/wasm/OutputSections.cpp b/lld/wasm/OutputSections.cpp
index 22c001eaa43b7..0b2dba4b25a7f 100644
--- a/lld/wasm/OutputSections.cpp
+++ b/lld/wasm/OutputSections.cpp
@@ -109,7 +109,7 @@ void DataSection::finalizeContents() {
   });
 #endif
 
-  assert((ctx.arg.sharedMemory || !ctx.isPic || ctx.arg.extendedConst ||
+  assert((!ctx.isPic || ctx.arg.extendedConst ||
           activeCount <= 1) &&
          "output segments should have been combined by now");
 
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 6ff5fbe6d9c0d..6f5c39ddec7fb 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -85,7 +85,7 @@ class Writer {
   void calculateTypes();
   void createOutputSegments();
   OutputSegment *createOutputSegment(StringRef name);
-  void combineOutputSegments();
+  void combineActiveOutputSegments();
   void layoutMemory();
   void createHeader();
 
@@ -1095,16 +1095,20 @@ void Writer::createOutputSegments() {
     }
   }
 
-  // Sort segments by type, placing .bss last
+  // Sort segments by type, placing .bss last. Note that one requirement of
+  // this sort is that all eventually-active segments must come first in
+  // case `combineActiveOutputSegments` is used. When combined the relative
+  // address of the data segment must be 0 (to be compatible with PIC and a
+  // lack of extended-const).
   llvm::stable_sort(segments,
                     [](const OutputSegment *a, const OutputSegment *b) {
                       auto order = [](StringRef name) {
                         return StringSwitch<int>(name)
-                            .StartsWith(".tdata", 0)
-                            .StartsWith(".rodata", 1)
-                            .StartsWith(".data", 2)
+                            .StartsWith(".rodata", 0)
+                            .StartsWith(".data", 1)
+                            .StartsWith(".tdata", 3)
                             .StartsWith(".bss", 4)
-                            .Default(3);
+                            .Default(2);
                       };
                       return order(a->name) < order(b->name);
                     });
@@ -1118,24 +1122,29 @@ void Writer::createOutputSegments() {
     seg->finalizeInputSegments();
 }
 
-void Writer::combineOutputSegments() {
+void Writer::combineActiveOutputSegments() {
   // 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.
+  // combines all active data segments into a single .data segment.
   // This restriction does not apply when the extended const extension is
   // available: https://github.com/WebAssembly/extended-const
   assert(!ctx.arg.extendedConst);
-  assert(ctx.isPic && !ctx.arg.isMultithreaded());
-  if (segments.size() <= 1)
+  assert(ctx.isPic);
+  auto isActive = [](const OutputSegment *s) {
+    return s->requiredInBinary() &&
+           (s->initFlags & WASM_DATA_SEGMENT_IS_PASSIVE) == 0;
+  };
+  if (llvm::count_if(segments, isActive) <= 1)
     return;
   OutputSegment *combined = make<OutputSegment>(".data");
-  combined->startVA = segments[0]->startVA;
   std::vector<OutputSegment *> newSegments = {combined};
   for (OutputSegment *s : segments) {
-    if (!s->requiredInBinary()) {
+    if (!isActive(s)) {
       newSegments.push_back(s);
       continue;
     }
+    if (combined->inputSegments.empty())
+      combined->startVA = s->startVA;
     bool first = true;
     for (InputChunk *inSeg : s->inputSegments) {
       if (first)
@@ -1155,6 +1164,10 @@ void Writer::combineOutputSegments() {
   }
 
   segments = std::move(newSegments);
+
+  // Fixup indices for any segments that have moved around.
+  for (size_t i = 0; i < segments.size(); ++i)
+    segments[i]->index = i;
 }
 
 static void createFunction(DefinedFunction *func, StringRef bodyContent) {
@@ -1318,29 +1331,48 @@ void Writer::createInitMemoryFunction() {
     //    (i32.const $__init_memory_flag)
     //    (i32.const 1)
 
+    // First figure out what locals need to be emitted for this function. Locals
+    // aren't always needed, though. Map them out here where they're allocated
+    // based on the same conditions that they're used in various situations
+    // below. For now all locals have the same type which makes the declaration
+    // side a bit simpler, and this'll have to get fancier if multiple types of
+    // locals are ever needed in t...
[truncated]

@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-lld

Author: Alex Crichton (alexcrichton)

Changes

This commit fixes a few issues that have surfaced in wasm-ld's handling of --cooperative-threading with -shared. Two primary issues fixed are:

  • The __wasm_init_memory function was not valid as it referenced nonexistent locals. This was fixed by adjusting how locals are declared to make this a bit more flexible.
  • Combining data segments for PIC without extended-const is generalized to only operate on active data segments and is now orthogonal to threading. With coop threads there's a mixture of passive/active segments (TLS is passive, other data is active) and the active segments still need combining while TLS stays passive.

The latter fix ended up touching a few more areas. The first is that .tdata sections are now sorted just before .bss, the end section, rather than first. This is done to ensure that active segments when combined can indeed start at a relative address of 0 (as required without extended-const). This change resulted in a number of adjustments for tests as the data is now sorted differently. Additionally relocations in TLS segments needed adjusting since the previous implementation implicitly only worked if .tdata is first.


Patch is 23.87 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208332.diff

10 Files Affected:

  • (modified) lld/test/wasm/compress-relocs.s (+6-6)
  • (modified) lld/test/wasm/compress-relocs64.s (+6-6)
  • (added) lld/test/wasm/cooperative-threading-pic.s (+77)
  • (modified) lld/test/wasm/data-segments.ll (+24-24)
  • (modified) lld/test/wasm/runtime-relocations-himem.s (+2-2)
  • (modified) lld/test/wasm/tls-non-shared-memory.s (+7-7)
  • (modified) lld/test/wasm/tls-relocations.s (+2-2)
  • (modified) lld/wasm/InputChunks.cpp (+8-2)
  • (modified) lld/wasm/OutputSections.cpp (+1-1)
  • (modified) lld/wasm/Writer.cpp (+59-31)
diff --git a/lld/test/wasm/compress-relocs.s b/lld/test/wasm/compress-relocs.s
index 37f1b3b170ff7..e872b941416af 100644
--- a/lld/test/wasm/compress-relocs.s
+++ b/lld/test/wasm/compress-relocs.s
@@ -47,16 +47,16 @@ test_memory_and_indirect_call_relocs:
   end_function
 
 # CHECK:    test_memory_and_indirect_call_relocs
-# CHECK:      41 90 80 84 80 00                 i32.const      65552
+# CHECK:      41 80 80 84 80 00                 i32.const      65536
 # CHECK:      11 80 80 80 80 00 80 80 80 80 00  call_indirect  0
-# CHECK:      28 02 94 80 84 80 00              i32.load       65556
+# CHECK:      28 02 84 80 84 80 00              i32.load       65540
 # CHECK:      11 81 80 80 80 00 80 80 80 80 00  call_indirect  1
 # CHECK:      41 81 80 80 80 00                 i32.const      1
 # CHECK:      11 80 80 80 80 00 80 80 80 80 00  call_indirect  0
 # COMPRESS: test_memory_and_indirect_call_relocs
-# COMPRESS:   41 90 80 04                       i32.const      65552
+# COMPRESS:   41 80 80 04                       i32.const      65536
 # COMPRESS:   11 00 00                          call_indirect  0
-# COMPRESS:   28 02 94 80 04                    i32.load       65556
+# COMPRESS:   28 02 84 80 04                    i32.load       65540
 # COMPRESS:   11 01 00                          call_indirect  1
 # COMPRESS:   41 01                             i32.const      1
 # COMPRESS:   11 00 00                          call_indirect  0
@@ -91,11 +91,11 @@ test_relative_relocs:
   end_function
 
 # CHECK:    test_relative_relocs
-# CHECK:      41 90 80 84 80 00  i32.const  65552
+# CHECK:      41 80 80 84 80 00  i32.const  65536
 # CHECK:      41 81 80 80 80 00  i32.const  1
 # CHECK:      41 83 80 80 80 00  i32.const  3
 # COMPRESS: test_relative_relocs
-# COMPRESS:   41 90 80 04        i32.const  65552
+# COMPRESS:   41 80 80 04        i32.const  65536
 # COMPRESS:   41 01              i32.const  1
 # COMPRESS:   41 03              i32.const  3
 
diff --git a/lld/test/wasm/compress-relocs64.s b/lld/test/wasm/compress-relocs64.s
index f3ff646cc3b1c..2dd18d604df41 100644
--- a/lld/test/wasm/compress-relocs64.s
+++ b/lld/test/wasm/compress-relocs64.s
@@ -36,12 +36,12 @@ test_memory_and_indirect_call_relocs:
   end_function
 
 # CHECK:    test_memory_and_indirect_call_relocs
-# CHECK:      42 90 80 84 80 80 80 80 80 80 00     i64.const 65552
-# CHECK:      29 03 98 80 84 80 80 80 80 80 80 00  i64.load  65560
+# CHECK:      42 80 80 84 80 80 80 80 80 80 00     i64.const 65536
+# CHECK:      29 03 88 80 84 80 80 80 80 80 80 00  i64.load  65544
 # CHECK:      42 81 80 80 80 80 80 80 80 80 00     i64.const 1
 # COMPRESS: test_memory_and_indirect_call_relocs
-# COMPRESS:   42 90 80 04                          i64.const 65552
-# COMPRESS:   29 03 98 80 04                       i64.load  65560
+# COMPRESS:   42 80 80 04                          i64.const 65536
+# COMPRESS:   29 03 88 80 04                       i64.load  65544
 # COMPRESS:   42 01                                i64.const 1
 
   .globl test_relative_relocs
@@ -56,11 +56,11 @@ test_relative_relocs:
   end_function
 
 # CHECK:    test_relative_relocs
-# CHECK:      42 90 80 84 80 80 80 80 80 80 00  i64.const 65552
+# CHECK:      42 80 80 84 80 80 80 80 80 80 00  i64.const 65536
 # CHECK:      42 81 80 80 80 80 80 80 80 80 00  i64.const 1
 # CHECK:      42 83 80 80 80 80 80 80 80 80 00  i64.const 3
 # COMPRESS: test_relative_relocs
-# COMPRESS:   42 90 80 04                       i64.const 65552
+# COMPRESS:   42 80 80 04                       i64.const 65536
 # COMPRESS:   42 01                             i64.const 1
 # COMPRESS:   42 03                             i64.const 3
 
diff --git a/lld/test/wasm/cooperative-threading-pic.s b/lld/test/wasm/cooperative-threading-pic.s
new file mode 100644
index 0000000000000..af2d366518b20
--- /dev/null
+++ b/lld/test/wasm/cooperative-threading-pic.s
@@ -0,0 +1,77 @@
+# Test --cooperative-threading combined with PIC output
+
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: wasm-ld -shared --cooperative-threading -no-gc-sections -o %t.so %t.o
+# RUN: obj2yaml %t.so | FileCheck %s
+# RUN: llvm-objdump --disassemble-symbols=__wasm_init_memory --no-show-raw-insn --no-leading-addr %t.so | FileCheck %s --check-prefix=DIS
+
+.globl         __wasm_get_tls_base
+__wasm_get_tls_base:
+  .functype   __wasm_get_tls_base () -> (i32)
+  i32.const 0
+  end_function
+
+.globl get_tls1
+get_tls1:
+  .functype get_tls1 () -> (i32)
+  call __wasm_get_tls_base
+  i32.const tls1@TLSREL
+  i32.add
+  end_function
+
+.section  .rodata.ro_data,"",@
+.globl  ro_data
+.p2align  2
+ro_data:
+  .int32  2
+  .size ro_data, 4
+
+.section  .data.rw_data,"",@
+.globl  rw_data
+.p2align  2
+rw_data:
+  .int32  1
+  .size rw_data, 4
+
+.section  .tdata.tls1,"T",@
+.globl  tls1
+.p2align  2
+tls1:
+  .int32  43
+  .size tls1, 4
+
+.section  .custom_section.target_features,"",@
+  .int8 1
+  .int8 43
+  .int8 11
+  .ascii  "bulk-memory"
+
+# The active .rodata and .data segments are combined into a single active
+# segment at __memory_base; the TLS segment remains passive.
+# CHECK:       - Type:            DATACOUNT
+# CHECK-NEXT:    Count:           2
+# CHECK:       - Type:            DATA{{$}}
+# CHECK-NEXT:    Segments:
+# CHECK-NEXT:      - SectionOffset:   6
+# CHECK-NEXT:        InitFlags:       0
+# CHECK-NEXT:        Offset:
+# CHECK-NEXT:          Opcode:          GLOBAL_GET
+# CHECK-NEXT:          Index:           {{[0-9]+}}
+# CHECK-NEXT:        Content:         '0200000001000000'
+# CHECK-NEXT:      - SectionOffset:   {{[0-9]+}}
+# CHECK-NEXT:        InitFlags:       1
+# CHECK-NEXT:        Content:         2B000000
+# CHECK-NEXT:  - Type:            CUSTOM
+
+# DIS:      <__wasm_init_memory>:
+# DIS-NEXT:   .local i32
+# DIS-NEXT:   i32.const 8
+# DIS-NEXT:   global.get {{[0-9]+}}
+# DIS-NEXT:   i32.add
+# DIS-NEXT:   local.tee 0
+# DIS-NEXT:   call {{[0-9]+}}
+# DIS-NEXT:   local.get 0
+# DIS-NEXT:   i32.const 0
+# DIS-NEXT:   i32.const 4
+# DIS-NEXT:   memory.init 1, 0
+# DIS-NEXT:   end
diff --git a/lld/test/wasm/data-segments.ll b/lld/test/wasm/data-segments.ll
index 7a18fd5efb655..5073e9c4cc8d1 100644
--- a/lld/test/wasm/data-segments.ll
+++ b/lld/test/wasm/data-segments.ll
@@ -93,7 +93,7 @@
 ; ACTIVE-PIC-NEXT:        Offset:
 ; ACTIVE-PIC-NEXT:          Opcode:          GLOBAL_GET
 ; ACTIVE-PIC-NEXT:          Index:           1
-; ACTIVE-PIC-NEXT:        Content:         63000000636F6E7374616E74000000002B00000068656C6C6F00676F6F646279650000002A000000
+; ACTIVE-PIC-NEXT:        Content:         636F6E7374616E74000000002B00000068656C6C6F00676F6F646279650000002A00000063000000
 
 ; PASSIVE-LABEL: - Type:            START
 ; PASSIVE-NEXT:    StartFunction:   2
@@ -114,13 +114,13 @@
 ; PASSIVE-NEXT:    Segments:
 ; PASSIVE-NEXT:      - SectionOffset:   3
 ; PASSIVE-NEXT:        InitFlags:       1
-; PASSIVE-NEXT:        Content:         '63000000'
-; PASSIVE-NEXT:      - SectionOffset:   9
-; PASSIVE-NEXT:        InitFlags:       1
 ; PASSIVE-NEXT:        Content:         636F6E7374616E74000000002B
-; PASSIVE-NEXT:      - SectionOffset:   24
+; PASSIVE-NEXT:      - SectionOffset:   18
 ; PASSIVE-NEXT:        InitFlags:       1
 ; PASSIVE-NEXT:        Content:         68656C6C6F00676F6F646279650000002A000000
+; PASSIVE-NEXT:      - SectionOffset:   40
+; PASSIVE-NEXT:        InitFlags:       1
+; PASSIVE-NEXT:        Content:         '63000000'
 ; PASSIVE-NEXT:  - Type:            CUSTOM
 ; PASSIVE-NEXT:    Name:            name
 ; PASSIVE-NEXT:    FunctionNames:
@@ -153,13 +153,13 @@
 ; PASSIVE-PIC-NEXT:    Segments:
 ; PASSIVE-PIC-NEXT:      - SectionOffset:   3
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
-; PASSIVE-PIC-NEXT:        Content:         '63000000'
-; PASSIVE-PIC-NEXT:      - SectionOffset:   9
-; PASSIVE-PIC-NEXT:        InitFlags:       1
 ; PASSIVE-PIC-NEXT:        Content:         636F6E7374616E74000000002B
-; PASSIVE-PIC-NEXT:      - SectionOffset:   24
+; PASSIVE-PIC-NEXT:      - SectionOffset:   18
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
 ; PASSIVE-PIC-NEXT:        Content:         68656C6C6F00676F6F646279650000002A000000
+; PASSIVE-PIC-NEXT:      - SectionOffset:   40
+; PASSIVE-PIC-NEXT:        InitFlags:       1
+; PASSIVE-PIC-NEXT:        Content:         '63000000'
 ; PASSIVE-PIC-NEXT:  - Type:            CUSTOM
 ; PASSIVE-PIC-NEXT:    Name:            name
 ; PASSIVE-PIC-NEXT:    FunctionNames:
@@ -212,34 +212,34 @@
 ; DIS-NEXT:            end
 
 ; NOPIC-DIS-NEXT:      [[PTR]].const   65536
-; NOPIC-DIS-NEXT:      [[PTR]].const   65536
-; NOPIC-DIS-NEXT:      global.set      1
 ; PIC-DIS-NEXT:        [[PTR]].const   0
 ; PIC-DIS-NEXT:        global.get      1
 ; PIC-DIS-NEXT:        [[PTR]].add
-; PIC-DIS-NEXT:        local.tee       1
-; PIC-DIS-NEXT:        global.set      {{\d*}}
-; PIC-DIS-NEXT:        local.get       1
+
 ; DIS-NEXT:            i32.const       0
-; DIS-NEXT:            i32.const       4
-; DIS-NEXT:            memory.init  0, 0
+; DIS-NEXT:            i32.const       13
+; DIS-NEXT:            memory.init     0, 0
 
-; NOPIC-DIS-NEXT:      [[PTR]].const   65540
-; PIC-DIS-NEXT:        [[PTR]].const   4
+; NOPIC-DIS-NEXT:      [[PTR]].const   65552
+; PIC-DIS-NEXT:        [[PTR]].const   16
 ; PIC-DIS-NEXT:        global.get      1
 ; PIC-DIS-NEXT:        [[PTR]].add
 
 ; DIS-NEXT:            i32.const       0
-; DIS-NEXT:            i32.const       13
+; DIS-NEXT:            i32.const       20
 ; DIS-NEXT:            memory.init     1, 0
 
-; NOPIC-DIS-NEXT:      [[PTR]].const   65556
-; PIC-DIS-NEXT:        [[PTR]].const   20
+; NOPIC-DIS-NEXT:      [[PTR]].const   65572
+; NOPIC-DIS-NEXT:      [[PTR]].const   65572
+; NOPIC-DIS-NEXT:      global.set      1
+; PIC-DIS-NEXT:        [[PTR]].const   36
 ; PIC-DIS-NEXT:        global.get      1
 ; PIC-DIS-NEXT:        [[PTR]].add
-
+; PIC-DIS-NEXT:        local.tee       1
+; PIC-DIS-NEXT:        global.set      {{\d*}}
+; PIC-DIS-NEXT:        local.get       1
 ; DIS-NEXT:            i32.const       0
-; DIS-NEXT:            i32.const       20
+; DIS-NEXT:            i32.const       4
 ; DIS-NEXT:            memory.init     2, 0
 ; NOPIC-DIS-NEXT:      [[PTR]].const   65576
 ; PIC-DIS-NEXT:        [[PTR]].const   40
@@ -272,6 +272,6 @@
 ; DIS-NEXT:           memory.atomic.wait32    0
 ; DIS-NEXT:           drop
 ; DIS-NEXT:          end
+; DIS-NEXT:          data.drop       0
 ; DIS-NEXT:          data.drop       1
-; DIS-NEXT:          data.drop       2
 ; DIS-NEXT:         end
diff --git a/lld/test/wasm/runtime-relocations-himem.s b/lld/test/wasm/runtime-relocations-himem.s
index 2d39a204c7904..2026e62a5af86 100644
--- a/lld/test/wasm/runtime-relocations-himem.s
+++ b/lld/test/wasm/runtime-relocations-himem.s
@@ -47,14 +47,14 @@ data_sym:
 
 # CHECK: <__wasm_apply_data_relocs>:
 # CHECK-EMPTY:
-# CHECK-NEXT:  i32.const -2147483636
+# CHECK-NEXT:  i32.const -2147483644
 # CHECK-NEXT:  global.get 0
 # CHECK-NEXT:  i32.store 0
 # CHECK-NEXT:  end
 
 # CHECK: <__wasm_apply_tls_relocs>:
 # CHECK-EMPTY:
-# CHECK-NEXT:  i32.const -2147483644
+# CHECK-NEXT:  i32.const -2147483636
 # CHECK-NEXT:  global.get 0
 # CHECK-NEXT:  i32.store 0
 # CHECK-NEXT:  end
diff --git a/lld/test/wasm/tls-non-shared-memory.s b/lld/test/wasm/tls-non-shared-memory.s
index 0a87ade7efb2e..21082a5603e7f 100644
--- a/lld/test/wasm/tls-non-shared-memory.s
+++ b/lld/test/wasm/tls-non-shared-memory.s
@@ -70,14 +70,14 @@ tls1:
 # CHECK-NEXT:         Mutable:         false
 # CHECK-NEXT:         InitExpr:
 # CHECK-NEXT:           Opcode:          I32_CONST
-# CHECK-NEXT:           Value:           65536
+# CHECK-NEXT:           Value:           65540
 # GOT.data.internal.tls1
 # CHECK-NEXT:       - Index:           2
 # CHECK-NEXT:         Type:            I32
 # CHECK-NEXT:         Mutable:         false
 # CHECK-NEXT:         InitExpr:
 # CHECK-NEXT:           Opcode:          I32_CONST
-# CHECK-NEXT:           Value:           65536
+# CHECK-NEXT:           Value:           65540
 # CHECK-NEXT:   - Type:            EXPORT
 
 #      CHECK:  - Type:            DATA
@@ -88,14 +88,14 @@ tls1:
 # CHECK-NEXT:        Offset:
 # CHECK-NEXT:          Opcode:          I32_CONST
 # CHECK-NEXT:          Value:           65536
-# CHECK-NEXT:        Content:         2B000000
+# CHECK-NEXT:        Content:         2A000000
 # .tdata
 # CHECK-NEXT:      - SectionOffset:   19
 # CHECK-NEXT:        InitFlags:       0
 # CHECK-NEXT:        Offset:
 # CHECK-NEXT:          Opcode:          I32_CONST
 # CHECK-NEXT:          Value:           65540
-# CHECK-NEXT:        Content:         2A000000
+# CHECK-NEXT:        Content:         2B000000
 # CHECK-NEXT:  - Type:            CUSTOM
 
 # The constant value here which we add to `__tls_base` should not be absolute
@@ -146,7 +146,7 @@ tls1:
 # PIC-NEXT:        Offset:
 # PIC-NEXT:          Opcode:          GLOBAL_GET
 # PIC-NEXT:          Index:           {{\d*}}
-# PIC-NEXT:        Content:         2B0000002A000000
+# PIC-NEXT:        Content:         2A0000002B000000
 # PIC-NEXT:  - Type:            CUSTOM
 
 # Unless we have extended-const, in which case the merging is not needed.
@@ -160,7 +160,7 @@ tls1:
 # EXT-CONST-NEXT:        Offset:
 # EXT-CONST-NEXT:          Opcode:          GLOBAL_GET
 # EXT-CONST-NEXT:          Index:           1
-# EXT-CONST-NEXT:        Content:         2B000000
+# EXT-CONST-NEXT:        Content:         2A000000
 # EXT-CONST-NEXT:      - SectionOffset:   18
 # EXT-CONST-NEXT:        InitFlags:       0
 # EXT-CONST-NEXT:        Offset:
@@ -168,4 +168,4 @@ tls1:
 # This instruction sequence decodes to:
 # (global.get[0x23] 0x1 i32.const[0x41] 0x04 i32.add[0x6A] end[0x0b])
 # EXT-CONST-NEXT:          Body:            230141046A0B
-# EXT-CONST-NEXT:        Content:         2A000000
+# EXT-CONST-NEXT:        Content:         2B000000
diff --git a/lld/test/wasm/tls-relocations.s b/lld/test/wasm/tls-relocations.s
index 9679074d6a0db..b5d1304177744 100644
--- a/lld/test/wasm/tls-relocations.s
+++ b/lld/test/wasm/tls-relocations.s
@@ -64,7 +64,7 @@ tls_sym:
 # ASM-NEXT:                 local.get 0
 # ASM-NEXT:                 i32.const 0
 # ASM-NEXT:                 i32.const 16
-# ASM-NEXT:                 memory.init 0, 0
+# ASM-NEXT:                 memory.init 1, 0
 # call to __wasm_apply_tls_relocs
 # ASM-NEXT:                 call  3
 # ASM-NEXT:                 end
@@ -75,7 +75,7 @@ tls_sym:
 # ASM-NEXT:                 global.get  3
 # ASM-NEXT:                 i32.add
 # ASM-NEXT:                 global.get  1
-# ASM-NEXT:                 i32.const 20
+# ASM-NEXT:                 i32.const 4
 # ASM-NEXT:                 i32.add
 # ASM-NEXT:                 i32.store 0
 # ASM-NEXT:                 i32.const 12
diff --git a/lld/wasm/InputChunks.cpp b/lld/wasm/InputChunks.cpp
index 99623e7c9aefe..e2d900a43ab19 100644
--- a/lld/wasm/InputChunks.cpp
+++ b/lld/wasm/InputChunks.cpp
@@ -445,12 +445,18 @@ bool InputChunk::generateRelocationCode(raw_ostream &os) const {
       continue;
     }
 
-    uint64_t offset = getVA(rel.Offset) - getInputSectionOffset();
+    // Calculate the address at which to apply the relocation. Note that PIC TLS
+    // segments are handled slightly differently here because at runtime they're
+    // a relative offset from `__tls_base`.
+    uint64_t offset;
+    if (ctx.isPic && isTLS())
+      offset = getChunkOffset(rel.Offset) - getInputSectionOffset();
+    else
+      offset = getVA(rel.Offset) - getInputSectionOffset();
     LLVM_DEBUG(dbgs() << "gen reloc: type=" << relocTypeToString(rel.Type)
                       << " addend=" << rel.Addend << " index=" << rel.Index
                       << " output offset=" << offset << "\n");
 
-    // Calculate the address at which to apply the relocation
     writePtrConst(os, offset, is64, "offset");
 
     // In PIC mode we need to add the __memory_base
diff --git a/lld/wasm/OutputSections.cpp b/lld/wasm/OutputSections.cpp
index 22c001eaa43b7..0b2dba4b25a7f 100644
--- a/lld/wasm/OutputSections.cpp
+++ b/lld/wasm/OutputSections.cpp
@@ -109,7 +109,7 @@ void DataSection::finalizeContents() {
   });
 #endif
 
-  assert((ctx.arg.sharedMemory || !ctx.isPic || ctx.arg.extendedConst ||
+  assert((!ctx.isPic || ctx.arg.extendedConst ||
           activeCount <= 1) &&
          "output segments should have been combined by now");
 
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 6ff5fbe6d9c0d..6f5c39ddec7fb 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -85,7 +85,7 @@ class Writer {
   void calculateTypes();
   void createOutputSegments();
   OutputSegment *createOutputSegment(StringRef name);
-  void combineOutputSegments();
+  void combineActiveOutputSegments();
   void layoutMemory();
   void createHeader();
 
@@ -1095,16 +1095,20 @@ void Writer::createOutputSegments() {
     }
   }
 
-  // Sort segments by type, placing .bss last
+  // Sort segments by type, placing .bss last. Note that one requirement of
+  // this sort is that all eventually-active segments must come first in
+  // case `combineActiveOutputSegments` is used. When combined the relative
+  // address of the data segment must be 0 (to be compatible with PIC and a
+  // lack of extended-const).
   llvm::stable_sort(segments,
                     [](const OutputSegment *a, const OutputSegment *b) {
                       auto order = [](StringRef name) {
                         return StringSwitch<int>(name)
-                            .StartsWith(".tdata", 0)
-                            .StartsWith(".rodata", 1)
-                            .StartsWith(".data", 2)
+                            .StartsWith(".rodata", 0)
+                            .StartsWith(".data", 1)
+                            .StartsWith(".tdata", 3)
                             .StartsWith(".bss", 4)
-                            .Default(3);
+                            .Default(2);
                       };
                       return order(a->name) < order(b->name);
                     });
@@ -1118,24 +1122,29 @@ void Writer::createOutputSegments() {
     seg->finalizeInputSegments();
 }
 
-void Writer::combineOutputSegments() {
+void Writer::combineActiveOutputSegments() {
   // 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.
+  // combines all active data segments into a single .data segment.
   // This restriction does not apply when the extended const extension is
   // available: https://github.com/WebAssembly/extended-const
   assert(!ctx.arg.extendedConst);
-  assert(ctx.isPic && !ctx.arg.isMultithreaded());
-  if (segments.size() <= 1)
+  assert(ctx.isPic);
+  auto isActive = [](const OutputSegment *s) {
+    return s->requiredInBinary() &&
+           (s->initFlags & WASM_DATA_SEGMENT_IS_PASSIVE) == 0;
+  };
+  if (llvm::count_if(segments, isActive) <= 1)
     return;
   OutputSegment *combined = make<OutputSegment>(".data");
-  combined->startVA = segments[0]->startVA;
   std::vector<OutputSegment *> newSegments = {combined};
   for (OutputSegment *s : segments) {
-    if (!s->requiredInBinary()) {
+    if (!isActive(s)) {
       newSegments.push_back(s);
       continue;
     }
+    if (combined->inputSegments.empty())
+      combined->startVA = s->startVA;
     bool first = true;
     for (InputChunk *inSeg : s->inputSegments) {
       if (first)
@@ -1155,6 +1164,10 @@ void Writer::combineOutputSegments() {
   }
 
   segments = std::move(newSegments);
+
+  // Fixup indices for any segments that have moved around.
+  for (size_t i = 0; i < segments.size(); ++i)
+    segments[i]->index = i;
 }
 
 static void createFunction(DefinedFunction *func, StringRef bodyContent) {
@@ -1318,29 +1331,48 @@ void Writer::createInitMemoryFunction() {
     //    (i32.const $__init_memory_flag)
     //    (i32.const 1)
 
+    // First figure out what locals need to be emitted for this function. Locals
+    // aren't always needed, though. Map them out here where they're allocated
+    // based on the same conditions that they're used in various situations
+    // below. For now all locals have the same type which makes the declaration
+    // side a bit simpler, and this'll have to get fancier if multiple types of
+    // locals are ever needed in t...
[truncated]

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

✅ With the latest revision this PR passed the C/C++ code formatter.

@alexcrichton
alexcrichton force-pushed the wasm-ld-coop-pic-data-segments branch 2 times, most recently from 1cdfdd7 to 698640d Compare July 10, 2026 00:11
Comment thread lld/wasm/InputChunks.cpp Outdated
Comment thread lld/wasm/Writer.cpp
Comment thread lld/wasm/Writer.cpp Outdated
@alexcrichton
alexcrichton force-pushed the wasm-ld-coop-pic-data-segments branch 2 times, most recently from 1f95382 to e41db8b Compare July 10, 2026 15:21
@alexcrichton
alexcrichton force-pushed the wasm-ld-coop-pic-data-segments branch from e41db8b to 83511ed Compare August 7, 2026 18:36
Comment thread lld/wasm/OutputSegment.h Outdated
}

/// Returns whether this is destined to become a active data segment in the
/// final output.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding comments like this? I'm not sure they add anything.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, removed

Comment thread lld/wasm/Writer.cpp
}

void Writer::combineOutputSegments() {
void Writer::combineActiveOutputSegments() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how long before we can remove this code completely because extended-const is available everywhere we care about.. probably a while

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is something I've talked about with some folks from time to time, or rather the more general issue of enabling/disabling features. Alas I've also never settled on a great answer of when to turn something on-by-default :(

This commit fixes a few issues that have surfaced in `wasm-ld`'s
handling of `--cooperative-threading` with `-shared`. Two primary issues
fixed are:

* The `__wasm_init_memory` function was not valid as it referenced
  nonexistent locals. This was fixed by adjusting how locals are
  declared to make this a bit more flexible.
* Combining data segments for PIC without extended-const is generalized
  to only operate on active data segments and is now orthogonal to
  threading. With coop threads there's a mixture of passive/active
  segments (TLS is passive, other data is active) and the active
  segments still need combining while TLS stays passive.

The latter fix ended up touching a few more areas. The first is that
`.tdata` sections are now sorted just before `.bss`, the end section,
rather than first. This is done to ensure that active segments when
combined can indeed start at a relative address of 0 (as required
without extended-const). This change resulted in a number of adjustments
for tests as the data is now sorted differently. Additionally
relocations in TLS segments needed adjusting since the previous
implementation implicitly only worked if `.tdata` is first.
@alexcrichton
alexcrichton force-pushed the wasm-ld-coop-pic-data-segments branch from 83511ed to 16ee109 Compare August 13, 2026 13:57
@sbc100
sbc100 merged commit 662246d into llvm:main Aug 13, 2026
11 of 12 checks passed
@alexcrichton
alexcrichton deleted the wasm-ld-coop-pic-data-segments branch August 13, 2026 15:01
zhangweize9-cyber pushed a commit to zhangweize9-cyber/llvm-project that referenced this pull request Aug 16, 2026
This commit fixes a few issues that have surfaced in `wasm-ld`'s
handling of `--cooperative-threading` with `-shared`. Two primary issues
fixed are:

* The `__wasm_init_memory` function was not valid as it referenced
nonexistent locals. This was fixed by adjusting how locals are declared
to make this a bit more flexible.
* Combining data segments for PIC without extended-const is generalized
to only operate on active data segments and is now orthogonal to
threading. With coop threads there's a mixture of passive/active
segments (TLS is passive, other data is active) and the active segments
still need combining while TLS stays passive.

The latter fix ended up touching a few more areas. The first is that
`.tdata` sections are now sorted just before `.bss`, the end section,
rather than first. This is done to ensure that active segments when
combined can indeed start at a relative address of 0 (as required
without extended-const). This change resulted in a number of adjustments
for tests as the data is now sorted differently. Additionally
relocations in TLS segments needed adjusting since the previous
implementation implicitly only worked if `.tdata` is first.
dyung pushed a commit that referenced this pull request Aug 20, 2026
This commit fixes a few issues that have surfaced in `wasm-ld`'s
handling of `--cooperative-threading` with `-shared`. Two primary issues
fixed are:

* The `__wasm_init_memory` function was not valid as it referenced
nonexistent locals. This was fixed by adjusting how locals are declared
to make this a bit more flexible.
* Combining data segments for PIC without extended-const is generalized
to only operate on active data segments and is now orthogonal to
threading. With coop threads there's a mixture of passive/active
segments (TLS is passive, other data is active) and the active segments
still need combining while TLS stays passive.

The latter fix ended up touching a few more areas. The first is that
`.tdata` sections are now sorted just before `.bss`, the end section,
rather than first. This is done to ensure that active segments when
combined can indeed start at a relative address of 0 (as required
without extended-const). This change resulted in a number of adjustments
for tests as the data is now sorted differently. Additionally
relocations in TLS segments needed adjusting since the previous
implementation implicitly only worked if `.tdata` is first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants