[WebAssembly] Remove special handling of entry point export. NFC

[sbc100]

Summary

Its much easier to export it via setHidden(false), now that we have that mechanism.

As a side effect the start function is not longer always exports first (becuase
its being exported just like all the other function).

[sbc100]

Split out unrelated change

[sbc100]

• clang-format

[ncw]

You've removed the check for CtorSymbol == EntrySym, I guess because you can't think of a reason why anyone would want to do --entry=__wasm_init_ctors? But if we do ever make the entry-point be treated as the "start" function, then that might be what we want.

This is good, because it gives --entry a clear reason to exist, but maybe it's not generic enough. Apart for the setHidden(false) call, is --entry identical to --export? So really --entry is redundant now, maybe it should actually go away.

Go ahead though and merge, I'm just musing here.

[sbc100]

In D42321#982653, @ncw wrote:

You've removed the check for CtorSymbol == EntrySym, I guess because you can't think of a reason why anyone would want to do --entry=__wasm_init_ctors? But if we do ever make the entry-point be treated as the "start" function, then that might be what we want.

This is good, because it gives --entry a clear reason to exist, but maybe it's not generic enough. Apart for the setHidden(false) call, is --entry identical to --export? So really --entry is redundant now, maybe it should actually go away.

Go ahead though and merge, I'm just musing here.

Oh.. maybe you are right.. let me right a test for that. I though that wasn't needed anymore.

[sbc100]

• and check for -entry=__wasm_call_ctors

[sbc100]

• rebase

test/wasm/alias.ll

 ; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:       - Name:            start_alias
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            start_alias
+; CHECK-NEXT:       - Name:            _start
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:       
 ; CHECK-NEXT:       - Index:           0

test/wasm/call-indirect.ll

 ; CHECK-NEXT:         InitExpr:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66576
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            bar
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            call_bar_indirect
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            call_ptr
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           4
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:   - Type:            ELEM
 ; CHECK-NEXT:     Segments:

test/wasm/entry.ll

 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-unknown-wasm %s -o %t.o
-; RUN: lld -flavor wasm -e entry -o %t.wasm %t.o
-; RUN: obj2yaml %t.wasm | FileCheck %s
-; RUN: lld -flavor wasm --entry=entry -o %t.wasm %t.o
-; RUN: obj2yaml %t.wasm | FileCheck %s
 
-define void @entry() local_unnamed_addr #0 {
+define hidden void @entry() local_unnamed_addr #0 {
 entry:
   ret void
 }
 
-; CHECK:   - Type:            EXPORT
-; CHECK:     Exports:
-; CHECK:       - Name:            memory
-; CHECK:         Kind:            MEMORY
-; CHECK:         Index:           0
-; CHECK:       - Name:            entry
-; CHECK:         Kind:            FUNCTION
-; CHECK:         Index:           0
+; RUN: lld -flavor wasm -e entry -o %t1.wasm %t.o
+; RUN: obj2yaml %t1.wasm | FileCheck %s
+; RUN: lld -flavor wasm --entry=entry -o %t2.wasm %t.o
+; RUN: obj2yaml %t2.wasm | FileCheck %s
+
+; CHECK:        - Type:            EXPORT
+; CHECK-NEXT:     Exports:
+; CHECK-NEXT:       - Name:            memory
+; CHECK-NEXT:         Kind:            MEMORY
+; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            entry
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           0
+
+; The __wasm_call_ctors is somewhat special.  Make sure we can use it
+; as the entry point if we choose
+; RUN: lld -flavor wasm --entry=__wasm_call_ctors -o %t3.wasm %t.o
+; RUN: obj2yaml %t3.wasm | FileCheck %s -check-prefix=CHECK-CTOR
+
+; CHECK-CTOR:        - Type:            EXPORT
+; CHECK-CTOR-NEXT:     Exports:
+; CHECK-CTOR-NEXT:       - Name:            memory
+; CHECK-CTOR-NEXT:         Kind:            MEMORY
+; CHECK-CTOR-NEXT:         Index:           0
+; CHECK-CTOR-NEXT:       - Name:            __wasm_call_ctors
+; CHECK-CTOR-NEXT:         Kind:            FUNCTION
+; CHECK-CTOR-NEXT:         Index:           1

test/wasm/export.ll

 
 ; CHECK-ERROR: error: symbol exported via --export not found: missing
 
 ; CHECK:        - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            hidden_function
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:   - Type:            CODE

test/wasm/stack-pointer.ll

 ; CHECK-NEXT:         InitExpr:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; 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:            _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:

test/wasm/visibility-hidden.ll

entry:
   ret i32 0
 }
 
 ; CHECK:        - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            objectDefault
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            archiveDefault
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           4
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:   - Type:

test/wasm/weak-alias-overide.ll

 ; CHECK-NEXT:         InitExpr:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            alias_fn
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            direct_fn
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            call_direct
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            call_alias
 ; CHECK-NEXT:         Kind:            FUNCTION

test/wasm/weak-undefined.ll

 ; CHECK-NEXT:         InitExpr:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            get_address_of_foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            get_address_of_global_var
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            __heap_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Locals:
 ; CHECK-NEXT:         Body:            4180808080000B

wasm/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (!Args.hasArg(OPT_INPUT))
     error("no input files");
 
   if (Config->Relocatable && !Config->Entry.empty())
     error("entry point specified for relocatable output file");
   if (Config->Relocatable && Args.hasArg(OPT_undefined))
     error("undefined symbols specified for relocatable output file");
 
+  Symbol *EntrySym = nullptr;
   if (!Config->Relocatable) {
     static WasmSignature Signature = {{}, WASM_TYPE_NORESULT};
     if (!Config->Entry.empty())
-      Symtab->addUndefinedFunction(Config->Entry, &Signature);
+      EntrySym = Symtab->addUndefinedFunction(Config->Entry, &Signature);
 
     // Handle the `--undefined <sym>` options.
     for (auto* Arg : Args.filtered(OPT_undefined))
       Symtab->addUndefinedFunction(Arg->getValue(), nullptr);
 
     // Create linker-synthetic symbols
     // __wasm_call_ctors:
     //    Function that directly calls all ctors in priority order.
for (auto *Arg : Args.filtered(OPT_export)) {
     Symbol *Sym = Symtab->find(Arg->getValue());
     if (!Sym || !Sym->isDefined())
       error("symbol exported via --export not found: " +
             Twine(Arg->getValue()));
     else
       Sym->setHidden(false);
   }
 
+  if (EntrySym)
+    EntrySym->setHidden(false);
+
   if (errorCount())
     return;
 
   // Write the result to the file.
   writeResult();
 }

wasm/SymbolTable.cpp

Symbol *SymbolTable::addDefinedFunction(StringRef Name,
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
   if (WasInserted) {
     S->update(Symbol::DefinedFunctionKind, nullptr, Flags);
     S->setFunctionType(Type);
   } else if (!S->isFunction()) {
     error("symbol type mismatch: " + Name);
+  } else if (!S->isDefined()) {
+    DEBUG(dbgs() << "resolving existing undefined function: " << Name << "\n");
+    S->update(Symbol::DefinedFunctionKind, nullptr, Flags);
   }
   return S;
 }
 
 Symbol *SymbolTable::addDefinedGlobal(StringRef Name) {
   DEBUG(dbgs() << "addDefinedGlobal: " << Name << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
-  if (WasInserted)
+  if (WasInserted) {
     S->update(Symbol::DefinedGlobalKind);
-  else if (!S->isGlobal())
+  } else if (!S->isGlobal()) {
     error("symbol type mismatch: " + Name);
+  } else {
+    DEBUG(dbgs() << "resolving existing undefined global: " << Name << "\n");
+    S->update(Symbol::DefinedGlobalKind);
+  }
   return S;
 }
 
 Symbol *SymbolTable::addDefined(StringRef Name, Symbol::Kind Kind,
                                 uint32_t Flags, InputFile *F,
                                 const InputSegment *Segment,
                                 InputFunction *Function, uint32_t Address) {
   DEBUG(dbgs() << "addDefined: " << Name << " addr:" << Address << "\n");

wasm/Symbols.cpp

bool Symbol::isLocal() const {
   return (Flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_LOCAL;
 }
 
 bool Symbol::isHidden() const {
   return (Flags & WASM_SYMBOL_VISIBILITY_MASK) == WASM_SYMBOL_VISIBILITY_HIDDEN;
 }
 
 void Symbol::setHidden(bool IsHidden) {
+  DEBUG(dbgs() << "setHidden: " << Name << " -> " << IsHidden << "\n");
   Flags &= ~WASM_SYMBOL_VISIBILITY_MASK;
   if (IsHidden)
     Flags |= WASM_SYMBOL_VISIBILITY_HIDDEN;
   else
     Flags |= WASM_SYMBOL_VISIBILITY_DEFAULT;
 }
 
 std::string lld::toString(const wasm::Symbol &Sym) {

wasm/Writer.cpp

for (Symbol *Sym : Symtab->getSymbols()) {
     } else {
       Sym->setOutputIndex(ImportedGlobals.size());
       ImportedGlobals.push_back(Sym);
     }
   }
 }
 
 void Writer::calculateExports() {
-  Symbol *EntrySym = Symtab->find(Config->Entry);
-  bool ExportEntry = !Config->Relocatable && EntrySym && EntrySym->isDefined();
   bool ExportHidden = Config->EmitRelocs;
   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.
     if (!Symtab->find(SymName) && UsedNames.insert(SymName).second)
       return SymName;
     for (unsigned I = 1; ; ++I) {
       std::string NameBuf = (SymName + "." + Twine(I)).str();
       if (!UsedNames.count(NameBuf)) {
         StringRef Name = Saver.save(NameBuf);
         UsedNames.insert(Name); // Insert must use safe StringRef from save()
         return Name;
       }
     }
   };
 
-  if (ExportEntry)
-    ExportedSymbols.emplace_back(WasmExportEntry{EntrySym, EntrySym->getName()});
-
-  if (Config->CtorSymbol && ExportHidden &&
-      !(ExportEntry && Config->CtorSymbol == EntrySym))
+  if (Config->CtorSymbol && (!Config->CtorSymbol->isHidden() || ExportHidden))
     ExportedSymbols.emplace_back(
         WasmExportEntry{Config->CtorSymbol, Config->CtorSymbol->getName()});
 
   for (ObjFile *File : Symtab->ObjectFiles) {
     for (Symbol *Sym : File->getSymbols()) {
       if (!Sym->isDefined() || File != Sym->getFile())
         continue;
       if (Sym->isGlobal())
         continue;
       if (Sym->getFunction()->Discarded)
         continue;
 
       if ((Sym->isHidden() || Sym->isLocal()) && !ExportHidden)
         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 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