[WebAssembly] Symbol changes #4b: export stack-pointer, LLD only

[ncw]

Summary

Split out of D41955.

This solves the problem that --emit-relocs needs the stack-pointer to be exported, in order to write out any relocations that reference the __stack_pointer symbol by its symbol index.

Rationale: --emit-relocs is only ever going to be used by downstream tools (such as Emscripten's binaryen tools). In particular, we don't expect that the output from --emit-relocs will be sent to browsers without further post-processing. Although browsers don't allow mutable globals to be imported/exported (yet), tools like binaryen do accept it.

[sbc100]

Great! Thanks for the small review!

[ncw]

Ah! I thought this might be contentious. I guess the --emit-relocs output isn't critical for anything at the moment.

[sbc100]

In D42237#980509, @ncw wrote:

Ah! I thought this might be contentious. I guess the --emit-relocs output isn't critical for anything at the moment.

Indeed, I'm not even sure we want to continue support it as a separate options to '-r/--relocatable' (of which --emit-relocs) is a subset. I'm tempted to rip and out and replace Config->EmitRelocs with Config->Relocatable, at least until we can come up with a reasonable use case (I suppose we use it in tests... but that isn't really a good enough motivation).

In any case this change is need for the --relocatable case.

[ncw]

It's not currently needed for the --relocatable case, since we don't create a stack-pointer symbol at all in that case (it's an import until the "final" link is done).

test/wasm/stack-pointer.ll

 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            _start
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __wasm_call_ctors
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            __stack_pointer
+; CHECK-NEXT:         Kind:            GLOBAL
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Relocations:
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_GLOBAL_INDEX_LEB
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x00000004

wasm/Writer.cpp

for (Symbol *Sym : File->getSymbols()) {
         continue;
       if (ExportEntry && Sym == EntrySym)
         continue;
       ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
     }
   }
 
   for (const Symbol *Sym : DefinedGlobals) {
-    // Can't export the SP right now because it's mutable and mutable globals
-    // cannot be exported.
-    if (Sym == Config->StackPointerSymbol)
+    // Can't export the SP right now because its mutable, and mutuable globals
+    // are yet supported in the official binary format.  However, for
+    // intermediate output we need to export it in case it is the target of any
+    // relocations.
+    // TODO(sbc): Remove this if/when the "mutable global" proposal is accepted.
+    if (Sym == Config->StackPointerSymbol && !Config->EmitRelocs)
       continue;
     ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
   }
 }
 
 uint32_t Writer::lookupType(const WasmSignature &Sig) {
   auto It = TypeIndices.find(Sig);
   if (It == TypeIndices.end()) {