[WebAssembly] Remove --emit-relocs

[sbc100]

Summary

This was added to mimick ELF, but maintaining it has cost
and we currently don't have any use for it outside of the
test code.

[sbc100]

• remove condition
• more tests

[ncw]

@ruiu some background. We've been discussing whether there should be a genuine difference between relocatable and non-relocatable Wasm objects. Or to put it another way, if you run LLD on a single object file, should anything really change? Maybe a few bits and pieces in the headers, but the actual body should be basically "stable", maybe stripping some dead functions,.

At the moment, --relocatable output really is quite different to normal output, in that it creates Wasm imports+exports for everything, whereas normal LLD output doesn't export local functions for example.

Hence --emit-relocs doesn't really work, you can't just "emit the relocs", since without having the imports/exports there's nothing to relocate against. You can't emit the relocs unless you're writing out a whole different object file.

[ruiu]

I'm not sure if I understand it correctly, so let me write down a few facts I believe true.

• --emit-relocs is not the same as -r. IIUC, they were created for pretty different purposes. --emit-relocs is to let the linker to emit the regular linked dso/executables along with relocation information (which is usually consumed by the linker and then removed from output), so that some post-processing tool can analyze the binary to do whatever they want to do. So, relocations in --emit-relocs is just "informative". OTOH, -r lets the linker to create a re-linkable, regular object files. With that feature, you can combine multiple .o files into a single .o.

• The output of -r is naturally quite different from the normal output. That's a natural consequence that the normal output is assumed to be consumed by the dynamic linker while the -r output is consumed by the static linker. So the difference is as large as the .o file and the .exe file, and that's not odd.

• I don't know why you wanted --emit-relocs at this point -- lld have that option because, IIRC, the Linux kernel needs it to do some tricky thing. Normally, you don't need that option.

Hope this helps.

[sbc100]

In D42324#982698, @ruiu wrote:

I'm not sure if I understand it correctly, so let me write down a few facts I believe true.

• --emit-relocs is not the same as -r. IIUC, they were created for pretty different purposes. --emit-relocs is to let the linker to emit the regular linked dso/executables along with relocation information (which is usually consumed by the linker and then removed from output), so that some post-processing tool can analyze the binary to do whatever they want to do. So, relocations in --emit-relocs is just "informative". OTOH, -r lets the linker to create a re-linkable, regular object files. With that feature, you can combine multiple .o files into a single .o.

• The output of -r is naturally quite different from the normal output. That's a natural consequence that the normal output is assumed to be consumed by the dynamic linker while the -r output is consumed by the static linker. So the difference is as large as the .o file and the .exe file, and that's not odd.

• I don't know why you wanted --emit-relocs at this point -- lld have that option because, IIRC, the Linux kernel needs it to do some tricky thing. Normally, you don't need that option.

Hope this helps.

That all sounds right to me.

In short, we probably never should have add --emit-relocs to the wasm linker in the first place. If we ever find a need for it we can add it back.

And yes, our .o files and .wasm files do have a fair few differences.

[sbc100]

ping @ruiu (not sure if I should be waiting on your lg here or not)

[ruiu]

LGTM

test/wasm/data-layout.ll

 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-unknown-wasm %p/Inputs/hello.ll -o %t.hello.o
 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-unknown-wasm %s -o %t.o
-; RUN: lld -flavor wasm --emit-relocs --allow-undefined --no-entry -o %t.wasm %t.o %t.hello.o
-; RUN: obj2yaml %t.wasm | FileCheck %s
 
 @foo = hidden global i32 1, align 4
 @aligned_bar = hidden global i32 3, align 16
 
 @hello_str = external global i8*
 @external_ref = global i8** @hello_str, align 8
 
+; RUN: lld -flavor wasm --allow-undefined -o %t.wasm %t.o %t.hello.o
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
 ; CHECK:        - Type:            GLOBAL
-; CHECK-NEXT:     Globals:
+; CHECK-NEXT:     Globals:         
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Type:            I32
 ; CHECK-NEXT:         Mutable:         true
-; CHECK-NEXT:         InitExpr:
+; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66608
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Type:            I32
 ; CHECK-NEXT:         Mutable:         false
-; CHECK-NEXT:         InitExpr:
+; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66608
-; CHECK-NEXT:       - Index:           2
-; CHECK-NEXT:         Type:            I32
-; CHECK-NEXT:         Mutable:         false
-; CHECK-NEXT:         InitExpr:
-; CHECK-NEXT:           Opcode:          I32_CONST
-; CHECK-NEXT:           Value:           1024
-; CHECK-NEXT:       - Index:           3
-; CHECK-NEXT:         Type:            I32
-; CHECK-NEXT:         Mutable:         false
-; CHECK-NEXT:         InitExpr:
-; CHECK-NEXT:           Opcode:          I32_CONST
-; CHECK-NEXT:           Value:           1040
-; CHECK-NEXT:       - Index:           4
-; CHECK-NEXT:         Type:            I32
-; CHECK-NEXT:         Mutable:         false
-; CHECK-NEXT:         InitExpr:
-; CHECK-NEXT:           Opcode:          I32_CONST
-; CHECK-NEXT:           Value:           1048
-; CHECK-NEXT:       - Index:           5
-; CHECK-NEXT:         Type:            I32
-; CHECK-NEXT:         Mutable:         false
-; CHECK-NEXT:         InitExpr:
-; CHECK-NEXT:           Opcode:          I32_CONST
-; CHECK-NEXT:           Value:           1052
 
-; CHECK:       - Type:            DATA
-; CHECK-NEXT:     Relocations:
-; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_MEMORY_ADDR_I32
-; CHECK-NEXT:         Index:           5
-; CHECK-NEXT:         Offset:          0x0000001F
-; CHECK-NEXT:     Segments:
+; CHECK:         - Type:            DATA
+; CHECK-NEXT:     Segments:        
 ; CHECK-NEXT:       - SectionOffset:   7
 ; CHECK-NEXT:         MemoryIndex:     0
-; CHECK-NEXT:         Offset:
+; CHECK-NEXT:         Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1024
 ; CHECK-NEXT:         Content:         0100000000000000000000000000000003000000000000001C040000
 ; CHECK-NEXT:       - SectionOffset:   41
 ; CHECK-NEXT:         MemoryIndex:     0
-; CHECK-NEXT:         Offset:
+; CHECK-NEXT:         Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1052
 ; CHECK-NEXT:         Content:         68656C6C6F0A00
 
-; CHECK:       - Type:            CUSTOM
-; CHECK-NEXT:     Name:            linking
-; CHECK-NEXT:     DataSize:        35
+
+; RUN: lld -flavor wasm --relocatable -o %t_reloc.wasm %t.o %t.hello.o
+; RUN: obj2yaml %t_reloc.wasm | FileCheck %s -check-prefix=RELOC
+
+; RELOC:        - Type:            GLOBAL
+; RELOC-NEXT:     Globals:
+; RELOC-NEXT:       - Index:           0
+; RELOC-NEXT:         Type:            I32
+; RELOC-NEXT:         Mutable:         false
+; RELOC-NEXT:         InitExpr:
+; RELOC-NEXT:           Opcode:          I32_CONST
+; RELOC-NEXT:           Value:           0
+; RELOC-NEXT:       - Index:           1
+; RELOC-NEXT:         Type:            I32
+; RELOC-NEXT:         Mutable:         false
+; RELOC-NEXT:         InitExpr:
+; RELOC-NEXT:           Opcode:          I32_CONST
+; RELOC-NEXT:           Value:           16
+
+; RELOC:       - Type:            DATA
+; RELOC-NEXT:     Relocations:
+; RELOC-NEXT:       - Type:            R_WEBASSEMBLY_MEMORY_ADDR_I32
+; RELOC-NEXT:         Index:           3
+; RELOC-NEXT:         Offset:          0x00000018
+; RELOC-NEXT:     Segments:
+; RELOC-NEXT:       - SectionOffset:   6
+; RELOC-NEXT:         MemoryIndex:     0
+; RELOC-NEXT:         Offset:
+; RELOC-NEXT:           Opcode:          I32_CONST
+; RELOC-NEXT:           Value:           0
+; RELOC-NEXT:         Content:         '01000000'
+; RELOC-NEXT:       - SectionOffset:   15
+; RELOC-NEXT:         MemoryIndex:     0
+; RELOC-NEXT:         Offset:
+; RELOC-NEXT:           Opcode:          I32_CONST
+; RELOC-NEXT:           Value:           16
+; RELOC-NEXT:         Content:         '03000000'
+; RELOC-NEXT:       - SectionOffset:   24
+; RELOC-NEXT:         MemoryIndex:     0
+; RELOC-NEXT:         Offset:
+; RELOC-NEXT:           Opcode:          I32_CONST
+; RELOC-NEXT:           Value:           24
+; RELOC-NEXT:         Content:         1C000000
+; RELOC-NEXT:       - SectionOffset:   33
+; RELOC-NEXT:         MemoryIndex:     0
+; RELOC-NEXT:         Offset:
+; RELOC-NEXT:           Opcode:          I32_CONST
+; RELOC-NEXT:           Value:           28
+; RELOC-NEXT:         Content:         68656C6C6F0A00
+
+; RELOC:       - Type:            CUSTOM
+; RELOC-NEXT:     Name:            linking
+; RELOC-NEXT:     DataSize:        35

test/wasm/stack-pointer.ll

 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-unknown-wasm %s -o %t.o
-; RUN: lld -flavor wasm --emit-relocs -o %t.wasm %t.o
+; RUN: lld -flavor wasm --relocatable -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 ; Function Attrs: nounwind
-define hidden i32 @_start() local_unnamed_addr {
+define i32 @_start() local_unnamed_addr {
 entry:
   %retval = alloca i32, align 4
   ret i32 0
 }
 
 ; CHECK:      --- !WASM
 ; CHECK-NEXT: FileHeader:
 ; CHECK-NEXT:   Version:         0x00000001
 ; CHECK-NEXT: Sections:
 ; CHECK-NEXT:   - Type:            TYPE
 ; CHECK-NEXT:     Signatures:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         ReturnType:      I32
 ; CHECK-NEXT:         ParamTypes:
-; CHECK-NEXT:       - Index:           1
-; CHECK-NEXT:         ReturnType:      NORESULT
-; CHECK-NEXT:         ParamTypes:   
+; CHECK-NEXT:   - Type:            IMPORT
+; CHECK-NEXT:     Imports:         
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           __stack_pointer
+; CHECK-NEXT:         Kind:            GLOBAL
+; CHECK-NEXT:         GlobalType:      I32
+; CHECK-NEXT:         GlobalMutable:   false
 ; CHECK-NEXT:   - Type:            FUNCTION
-; CHECK-NEXT:     FunctionTypes:   [ 0, 1 ]
+; CHECK-NEXT:     FunctionTypes:   [ 0 ]
 ; CHECK-NEXT:   - Type:            TABLE
 ; CHECK-NEXT:     Tables:
 ; CHECK-NEXT:       - ElemType:        ANYFUNC
 ; CHECK-NEXT:         Limits:
 ; CHECK-NEXT:           Flags:           [ HAS_MAX ]
-; CHECK-NEXT:           Initial:         0x00000001
-; CHECK-NEXT:           Maximum:         0x00000001
+; CHECK-NEXT:           Initial:         0x00000000
+; CHECK-NEXT:           Maximum:         0x00000000
 ; CHECK-NEXT:   - Type:            MEMORY
 ; CHECK-NEXT:     Memories:
-; CHECK-NEXT:       - Initial:         0x00000002
-; CHECK-NEXT:   - Type:            GLOBAL
-; 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:           Value:           66560
-; CHECK-NEXT:       - Index:           1
-; CHECK-NEXT:         Type:            I32
-; CHECK-NEXT:         Mutable:         false
-; CHECK-NEXT:         InitExpr:
-; CHECK-NEXT:           Opcode:          I32_CONST
-; CHECK-NEXT:           Value:           66560
+; CHECK-NEXT:       - Initial:         0x00000000
 ; CHECK-NEXT:   - Type:            EXPORT
-; CHECK-NEXT:     Exports:
-; CHECK-NEXT:       - Name:            memory
-; CHECK-NEXT:         Kind:            MEMORY
-; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            __wasm_call_ctors
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            _start
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
-; 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
 ; CHECK-NEXT:     Functions:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            23808080800041106B1A41000B
-; CHECK-NEXT:       - Index:           1
-; CHECK-NEXT:         Locals:
-; CHECK-NEXT:         Body:            0B
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            _start
-; CHECK-NEXT:       - Index:           1
-; CHECK-NEXT:         Name:            __wasm_call_ctors
 ; CHECK-NEXT: ...

wasm/Config.h

 
 namespace lld {
 namespace wasm {
 
 struct Configuration {
   bool AllowUndefined;
   bool CheckSignatures;
   bool Demangle;
-  bool EmitRelocs;
   bool ImportMemory;
   bool Relocatable;
   bool StripAll;
   bool StripDebug;
   uint32_t GlobalBase;
   uint32_t InitialMemory;
   uint32_t MaxMemory;
   uint32_t ZStackSize;

wasm/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (Args.hasArg(OPT_version) || Args.hasArg(OPT_v)) {
     outs() << getLLDVersion() << "\n";
     return;
   }
 
   Config->AllowUndefined = Args.hasArg(OPT_allow_undefined);
   Config->CheckSignatures =
       Args.hasFlag(OPT_check_signatures, OPT_no_check_signatures, false);
-  Config->EmitRelocs = Args.hasArg(OPT_emit_relocs);
   Config->Entry = getEntry(Args, Args.hasArg(OPT_relocatable) ? "" : "_start");
   Config->ImportMemory = Args.hasArg(OPT_import_memory);
   Config->OutputFile = Args.getLastArgValue(OPT_o);
   Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SearchPaths = args::getStrings(Args, OPT_L);
   Config->StripAll = Args.hasArg(OPT_strip_all);
   Config->StripDebug = Args.hasArg(OPT_strip_debug);
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
   ThreadsEnabled = Args.hasFlag(OPT_threads, OPT_no_threads, true);
-  if (Config->Relocatable)
-    Config->EmitRelocs = true;
 
   Config->InitialMemory = args::getInteger(Args, OPT_initial_memory, 0);
   Config->GlobalBase = args::getInteger(Args, OPT_global_base, 1024);
   Config->MaxMemory = args::getInteger(Args, OPT_max_memory, 0);
   Config->ZStackSize =
       args::getZOptionValue(Args, OPT_z, "stack-size", WasmPageSize);
 
   if (auto *Arg = Args.getLastArg(OPT_allow_undefined_file))

wasm/InputChunks.cpp

for (const WasmRelocation &Reloc : Relocations) {
     OutputRelocation NewReloc;
     NewReloc.Reloc = Reloc;
     assert(Reloc.Offset + Off > 0);
     NewReloc.Reloc.Offset += Off;
     DEBUG(dbgs() << "reloc: type=" << Reloc.Type << " index=" << Reloc.Index
                  << " offset=" << Reloc.Offset
                  << " newOffset=" << NewReloc.Reloc.Offset << "\n");
 
-    if (Config->EmitRelocs)
+    if (Config->Relocatable)
       NewReloc.NewIndex = File->calcNewIndex(Reloc);
 
     switch (Reloc.Type) {
     case R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
     case R_WEBASSEMBLY_MEMORY_ADDR_I32:
     case R_WEBASSEMBLY_MEMORY_ADDR_LEB:
       NewReloc.Value = File->getRelocatedAddress(Reloc.Index) + Reloc.Addend;
       break;

wasm/Options.td

 def v: Flag<["-"], "v">, HelpText<"Display the version number">;
 
 def version: F<"version">, HelpText<"Display the version number and exit">;
 
 def verbose: F<"verbose">, HelpText<"Verbose mode">;
 
 def relocatable: F<"relocatable">, HelpText<"Create relocatable object file">;
 
-def emit_relocs: F<"emit-relocs">, HelpText<"Generate relocations in output">;
-
 def strip_all: F<"strip-all">, HelpText<"Strip all symbols">;
 
 def strip_debug: F<"strip-debug">, HelpText<"Strip debugging information">;
 
 defm undefined: Eq<"undefined">,
   HelpText<"Force undefined symbol during linking">;
 
 def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,

wasm/Writer.cpp

void Writer::createSections() {
   createGlobalSection();
   createExportSection();
   createStartSection();
   createElemSection();
   createCodeSection();
   createDataSection();
 
   // Custom sections
-  if (Config->EmitRelocs)
+  if (Config->Relocatable)
     createRelocSections();
   createLinkingSection();
   if (!Config->StripDebug && !Config->StripAll)
     createNameSection();
 
   for (OutputSection *S : OutputSections) {
     S->setOffset(FileSize);
     S->finalizeContents();
     FileSize += S->getSize();
   }
 }
 
 void Writer::calculateImports() {
   for (Symbol *Sym : Symtab->getSymbols()) {
-    if (!Sym->isUndefined() || (Sym->isWeak() && !Config->EmitRelocs))
+    if (!Sym->isUndefined() || (Sym->isWeak() && !Config->Relocatable))
       continue;
 
     if (Sym->isFunction()) {
       Sym->setOutputIndex(ImportedFunctions.size());
       ImportedFunctions.push_back(Sym);
     } else {
       Sym->setOutputIndex(ImportedGlobals.size());
       ImportedGlobals.push_back(Sym);
     }
   }
 }
 
 void Writer::calculateExports() {
-  bool ExportHidden = Config->EmitRelocs;
+  bool ExportHidden = Config->Relocatable;
   StringSet<> UsedNames;
   auto BudgeLocalName = [&](const Symbol *Sym) {
     StringRef SymName = Sym->getName();
     // We can't budge non-local names.
     if (!Sym->isLocal())
       return SymName;
     // We must budge local names that have a collision with a symbol that we
     // haven't yet processed.
for (Symbol *Sym : File->getSymbols()) {
       if ((Sym->isHidden() || Sym->isLocal()) && !ExportHidden)
         continue;
       ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
     }
   }
 
   for (const Symbol *Sym : DefinedGlobals) {
     // 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.
+    // are yet supported in the official binary format.
     // TODO(sbc): Remove this if/when the "mutable global" proposal is accepted.
-    if (Sym == Config->StackPointerSymbol && !Config->EmitRelocs)
+    if (Sym == Config->StackPointerSymbol)
       continue;
     ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
   }
 }
 
 uint32_t Writer::lookupType(const WasmSignature &Sig) {
   auto It = TypeIndices.find(Sig);
   if (It == TypeIndices.end()) {
if (Config->StackPointerSymbol) {
     Config->StackPointerSymbol->setOutputIndex(GlobalIndex++);
   }
 
   if (Config->HeapBaseSymbol) {
     DefinedGlobals.emplace_back(Config->HeapBaseSymbol);
     Config->HeapBaseSymbol->setOutputIndex(GlobalIndex++);
   }
 
-  if (Config->EmitRelocs)
+  if (Config->Relocatable)
     DefinedGlobals.reserve(Symtab->getSymbols().size());
 
   uint32_t TableIndex = InitialTableOffset;
 
   for (ObjFile *File : Symtab->ObjectFiles) {
-    if (Config->EmitRelocs) {
+    if (Config->Relocatable) {
       DEBUG(dbgs() << "Globals: " << File->getName() << "\n");
       for (Symbol *Sym : File->getSymbols()) {
         // Create wasm globals for data symbols defined in this file
         if (!Sym->isDefined() || File != Sym->getFile())
           continue;
         if (Sym->isFunction())
           continue;