[lld][WebAssembly] Perform data relocations during start function

[sbc100]

Summary

We already perform memory initialization and apply global relocations
during start. It makes sense to performs data relocations too. I think
the reason we were not doing this already is solely historical.

[sbc100]

• update

[sbc100]

• update

[sbc100]

• rebase

[tlively]

Who was previously responsible for applying the data relocs? If it was Emscripten runtime code, does that mean this change will have to land alongside the corresponding Emscripten change? Is there anyone who would be broken by this ABI break who we should notify?

lld/wasm/Writer.cpp

1261–1268

Can we add a test that exercises this case?

[sbc100]

In D117412#3250155, @tlively wrote:

Who was previously responsible for applying the data relocs? If it was Emscripten runtime code, does that mean this change will have to land alongside the corresponding Emscripten change? Is there anyone who would be broken by this ABI break who we should notify?

Previously it was treated as the very first ctor in __wasm_call_ctors.. (which emscripten take care of calling). Now it happens earlier, removing that window where the relocations have not yet been applied.

[sbc100]

• add testing

[sbc100]

• add tests

[sbc100]

lld/wasm/Writer.cpp

1261–1268

I think we have coverage of all three branches above now.

[dschuff]

This change (probably) broke the SAFE_HEAP tests on the emscripten waterfall: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8824622360264175889/+/u/Emscripten_testsuite__SAFE_HEAP_/stdout
There are other changes in the LLVM roll (https://chromium.googlesource.com/emscripten-releases/+/cd22f208c7dda41d695549e17f8ddc4ae533769d) but this is the only wasm-related one.

[sbc100]

In D117412#3256093, @dschuff wrote:

This change (probably) broke the SAFE_HEAP tests on the emscripten waterfall: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8824622360264175889/+/u/Emscripten_testsuite__SAFE_HEAP_/stdout
There are other changes in the LLVM roll (https://chromium.googlesource.com/emscripten-releases/+/cd22f208c7dda41d695549e17f8ddc4ae533769d) but this is the only wasm-related one.

Yup.. I noticed that too. Fix is in https://github.com/WebAssembly/binaryen/pull/4463

lld/test/wasm/data-segments.ll

 ; PASSIVE-NEXT:      - Index:           0
 ; PASSIVE-NEXT:        Name:            __wasm_call_ctors
 ; 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:    StartFunction:   4
 ; PASSIVE-PIC-NEXT:  - Type:            DATACOUNT
 ; 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
 ; PASSIVE-PIC-NEXT:        Body:            {{.*}}
 ; PASSIVE-PIC-NEXT:      - Index:           3
 ; PASSIVE-PIC-NEXT:        Locals:          []
 ; PASSIVE-PIC-NEXT:        Body:            0B
+; PASSIVE-PIC-NEXT:      - Index:           4
+; PASSIVE-PIC-NEXT:        Locals:          []
+; PASSIVE-PIC-NEXT:        Body:            100210030B
 ; PASSIVE-PIC-NEXT:  - Type:            DATA
 ; PASSIVE-PIC-NEXT:    Segments:
 ; PASSIVE-PIC-NEXT:      - SectionOffset:   3
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
 ; PASSIVE-PIC-NEXT:        Content:         636F6E7374616E74000000002B
 ; PASSIVE-PIC-NEXT:      - SectionOffset:   18
 ; PASSIVE-PIC-NEXT:        InitFlags:       1
 ; PASSIVE-PIC-NEXT:        Content:         68656C6C6F00676F6F646279650000002A000000

lld/test/wasm/pie.ll

 
 ; RUN: llc -relocation-model=pic -mattr=+mutable-globals,+atomics,+bulk-memory -filetype=obj %s -o %t.shmem.o
 ; RUN: wasm-ld --no-gc-sections --shared-memory --allow-undefined --experimental-pic -pie -o %t.shmem.wasm %t.shmem.o
 ; RUN: obj2yaml %t.shmem.wasm | FileCheck %s --check-prefix=SHMEM
 
 ; SHMEM:         - Type:            CODE
 ; SHMEM:           - Index:           6
 ; SHMEM-NEXT:        Locals:          []
-; SHMEM-NEXT:        Body:            100310050B
+; SHMEM-NEXT:        Body:            1003100510040B
 
 ; SHMEM:         FunctionNames:
 ; SHMEM-NEXT:      - Index:           0
 ; SHMEM-NEXT:        Name:            external_func
 ; SHMEM-NEXT:      - Index:           1
 ; SHMEM-NEXT:        Name:            __wasm_call_ctors
 ; SHMEM-NEXT:      - Index:           2
 ; SHMEM-NEXT:        Name:            __wasm_init_tls

lld/wasm/Writer.cpp

if (config->isPic) {
     if (out.globalSec->needsRelocations()) {
       WasmSym::applyGlobalRelocs = symtab->addSyntheticFunction(
           "__wasm_apply_global_relocs", WASM_SYMBOL_VISIBILITY_HIDDEN,
           make<SyntheticFunction>(nullSignature, "__wasm_apply_global_relocs"));
       WasmSym::applyGlobalRelocs->markLive();
     }
   }
 
-  if (WasmSym::applyGlobalRelocs && WasmSym::initMemory) {
+  int startCount = 0;
+  if (WasmSym::applyGlobalRelocs)
+    startCount++;
+  if (WasmSym::WasmSym::initMemory)
+    startCount++;
+  if (WasmSym::applyDataRelocs)
+    startCount++;
+
+  // If there is only one start function we can just use that function
+  // itself as the Wasm start function, otherwise we need to synthesize
+  // a new function to call them in sequence.
+  if (startCount > 1) {
     WasmSym::startFunction = symtab->addSyntheticFunction(
         "__wasm_start", WASM_SYMBOL_VISIBILITY_HIDDEN,
         make<SyntheticFunction>(nullSignature, "__wasm_start"));
     WasmSym::startFunction->markLive();
   }
 }
 
 void Writer::createInitMemoryFunction() {
std::string bodyContent;
     // End the function
     writeU8(os, WASM_OPCODE_END, "END");
   }
 
   createFunction(WasmSym::initMemory, bodyContent);
 }
 
 void Writer::createStartFunction() {
+  // If that start function exists when we have more than one function to call.
   if (WasmSym::startFunction) {
     std::string bodyContent;
     {
       raw_string_ostream os(bodyContent);
       writeUleb128(os, 0, "num locals");
+      if (WasmSym::initMemory) {
         writeU8(os, WASM_OPCODE_CALL, "CALL");
         writeUleb128(os, WasmSym::initMemory->getFunctionIndex(),
                      "function index");
+      }
+      if (WasmSym::applyGlobalRelocs) {
         writeU8(os, WASM_OPCODE_CALL, "CALL");
         writeUleb128(os, WasmSym::applyGlobalRelocs->getFunctionIndex(),
                      "function index");
+      }
+      if (WasmSym::applyDataRelocs) {
+        writeU8(os, WASM_OPCODE_CALL, "CALL");
+        writeUleb128(os, WasmSym::applyDataRelocs->getFunctionIndex(),
+                     "function index");
+      }
       writeU8(os, WASM_OPCODE_END, "END");
     }
     createFunction(WasmSym::startFunction, bodyContent);
   } else if (WasmSym::initMemory) {
     WasmSym::startFunction = WasmSym::initMemory;
   } else if (WasmSym::applyGlobalRelocs) {
     WasmSym::startFunction = WasmSym::applyGlobalRelocs;
   }
 }
 
 // For -shared (PIC) output, we create create a synthetic function which will

 }
 
 // Create synthetic "__wasm_call_ctors" function based on ctor functions
 // in input object.
 void Writer::createCallCtorsFunction() {
   // If __wasm_call_ctors isn't referenced, there aren't any ctors, and we
   // aren't calling `__wasm_apply_data_relocs` for Emscripten-style PIC, don't
   // define the `__wasm_call_ctors` function.
-  if (!WasmSym::callCtors->isLive() && !WasmSym::applyDataRelocs &&
-      initFunctions.empty())
+  if (!WasmSym::callCtors->isLive() && initFunctions.empty())
     return;
 
   // First write the body's contents to a string.
   std::string bodyContent;
   {
     raw_string_ostream os(bodyContent);
     writeUleb128(os, 0, "num locals");