[WebAssembly] Fix for discarded init functions

[sbc100]

Summary

When a function is excluded via comdat we shouldn't add it to the
final list of init functions.

[sbc100]

• cleanup

[sbc100]

• cleanup

[sbc100]

• update comment

[ruiu]

LGTM

lld/test/wasm/Inputs/comdat1.ll

 target triple = "wasm32-unknown-unknown"
 
-$inlineFn = comdat any
-@constantData = weak_odr constant [3 x i8] c"abc", comdat($inlineFn)
-define linkonce_odr i32 @inlineFn() comdat {
-entry:
+$foo = comdat any
+
+@constantData = constant [3 x i8] c"abc", comdat($foo)
+
+define i32 @comdatFn() comdat($foo) {
   ret i32 ptrtoint ([3 x i8]* @constantData to i32)
 }
 
-define i32 @callInline1() {
-entry:
-    ret i32 ptrtoint (i32 ()* @inlineFn to i32)
+define internal void @do_init() comdat($foo) {
+  ret void
+}
+
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void
+()*, i8* } { i32 65535, void ()* @do_init, i8* null }]
+
+; Everything above this is part ofthe `foo` comdat group
+
+define i32 @callComdatFn1() {
+    ret i32 ptrtoint (i32 ()* @comdatFn to i32)
 }

lld/test/wasm/Inputs/comdat2.ll

 target triple = "wasm32-unknown-unknown"
 
-$inlineFn = comdat any
-@constantData = weak_odr constant [3 x i8] c"abc", comdat($inlineFn)
-define linkonce_odr i32 @inlineFn() comdat {
-entry:
+$foo = comdat any
+
+@constantData = constant [3 x i8] c"abc", comdat($foo)
+
+define i32 @comdatFn() comdat($foo) {
   ret i32 ptrtoint ([3 x i8]* @constantData to i32)
 }
 
-define i32 @callInline2() {
-entry:
-  ret i32 ptrtoint (i32 ()* @inlineFn to i32)
+define internal void @do_init() comdat($foo) {
+  ret void
+}
+
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void
+()*, i8* } { i32 65535, void ()* @do_init, i8* null }]
+
+
+; Everything above this is part ofthe `foo` comdat group
+
+define i32 @callComdatFn2() {
+  ret i32 ptrtoint (i32 ()* @comdatFn to i32)
 }

lld/test/wasm/comdats.ll

 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-uknown-wasm %p/Inputs/comdat1.ll -o %t1.o
 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-uknown-wasm %p/Inputs/comdat2.ll -o %t2.o
 ; RUN: llc -filetype=obj -mtriple=wasm32-unknown-uknown-wasm %s -o %t.o
 ; RUN: wasm-ld --export-dynamic -o %t.wasm %t.o %t1.o %t2.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
-declare i32 @inlineFn()
+declare i32 @comdatFn()
 
-define void @_start() local_unnamed_addr {
+define void @_start() {
 entry:
-  %call = call i32 @inlineFn()
+  %call = call i32 @comdatFn()
   ret void
 }
 
 ; CHECK:       - Type:            GLOBAL
 ; CHECK-NEXT:    Globals:
 ; CHECK-NEXT:      - Index:           0
 ; CHECK-NEXT:        Type:            I32
 ; CHECK-NEXT:        Mutable:         true

 ; 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:            inlineFn
+; CHECK-NEXT:      - Name:            comdatFn
 ; CHECK-NEXT:        Kind:            FUNCTION
 ; CHECK-NEXT:        Index:           1
 ; CHECK-NEXT:      - Name:            constantData
 ; CHECK-NEXT:        Kind:            GLOBAL
 ; CHECK-NEXT:        Index:           1
-; CHECK-NEXT:      - Name:            callInline1
+; CHECK-NEXT:      - Name:            callComdatFn1
 ; CHECK-NEXT:        Kind:            FUNCTION
 ; CHECK-NEXT:        Index:           2
-; CHECK-NEXT:      - Name:            callInline2
+; CHECK-NEXT:      - Name:            callComdatFn2
 ; CHECK-NEXT:        Kind:            FUNCTION
 ; CHECK-NEXT:        Index:           3
 ; CHECK-NEXT:  - Type:            ELEM
 ; CHECK-NEXT:    Segments:
 ; CHECK-NEXT:      - Offset:
 ; CHECK-NEXT:          Opcode:          I32_CONST
 ; CHECK-NEXT:          Value:           1
 ; CHECK-NEXT:        Functions:       [ 1 ]

lld/test/wasm/lto/comdat.ll

 ; Verify that comdat symbols can be defined in LTO objects.  We had a
 ; regression where the comdat handling code was causing symbol in the lto object
 ; to be ignored.
 ; RUN: llvm-as %s -o %t.bc
 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: wasm-ld %t.bc %t.o -o %t.wasm
 ; RUN: wasm-ld %t.o %t.bc -o %t.wasm
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 $foo = comdat any
 
+define void @func_sym() comdat($foo) {
+  ret void
+}
+
 define void @_start() comdat($foo) {
 entry:
   ret void
 }

lld/wasm/InputChunks.h

public:
   ObjFile *File;
   int32_t OutputOffset = 0;
 
   // Signals that the section is part of the output.  The garbage collector,
   // and COMDAT handling can set a sections' Live bit.
   // If GC is disabled, all sections start out as live by default.
   unsigned Live : 1;
 
+  // Signals the check was discarded by COMDAT handling.
+  unsigned Discarded : 1;
+
 protected:
   InputChunk(ObjFile *F, Kind K)
-      : File(F), Live(!Config->GcSections), SectionKind(K) {}
+      : File(F), Live(!Config->GcSections), Discarded(false), SectionKind(K) {}
   virtual ~InputChunk() = default;
   virtual ArrayRef<uint8_t> data() const = 0;
 
   // Verifies the existing data at relocation targets matches our expectations.
   // This is performed only debug builds as an extra sanity check.
   void verifyRelocTargets() const;
 
   ArrayRef<WasmRelocation> Relocations;

lld/wasm/InputFiles.cpp

for (const WasmRelocation &Reloc : Section.Relocations)
       if (Reloc.Type == R_WASM_FUNCTION_INDEX_LEB)
         IsCalledDirectly[Reloc.Index] = true;
   }
 
   TypeMap.resize(getWasmObj()->types().size());
   TypeIsUsed.resize(getWasmObj()->types().size(), false);
 
   ArrayRef<StringRef> Comdats = WasmObj->linkingData().Comdats;
-  for (unsigned I = 0; I < Comdats.size(); ++I) {
-    bool IsNew = IgnoreComdats || Symtab->addComdat(Comdats[I]);
+  for (StringRef Comdat : Comdats) {
+    dbgs() << "comdat: " << Comdat << "\n";
+    bool IsNew = IgnoreComdats || Symtab->addComdat(Comdat);
     KeptComdats.push_back(IsNew);
   }
 
   // Populate `Segments`.
-  for (const WasmSegment &S : WasmObj->dataSegments())
-    Segments.emplace_back(make<InputSegment>(S, this));
+  for (const WasmSegment &S : WasmObj->dataSegments()) {
+    auto* Seg = make<InputSegment>(S, this);
+    Seg->Discarded = isExcludedByComdat(Seg);
+    Segments.emplace_back(Seg);
+  }
   setRelocs(Segments, DataSection);
 
   // Populate `Functions`.
   ArrayRef<WasmFunction> Funcs = WasmObj->functions();
   ArrayRef<uint32_t> FuncTypes = WasmObj->functionTypes();
   ArrayRef<WasmSignature> Types = WasmObj->types();
   Functions.reserve(Funcs.size());
 
-  for (size_t I = 0, E = Funcs.size(); I != E; ++I)
-    Functions.emplace_back(
-        make<InputFunction>(Types[FuncTypes[I]], &Funcs[I], this));
+  for (size_t I = 0, E = Funcs.size(); I != E; ++I) {
+    auto* Func = make<InputFunction>(Types[FuncTypes[I]], &Funcs[I], this);
+    Func->Discarded = isExcludedByComdat(Func);
+    Functions.emplace_back(Func);
+  }
   setRelocs(Functions, CodeSection);
 
   // Populate `Globals`.
   for (const WasmGlobal &G : WasmObj->globals())
     Globals.emplace_back(make<InputGlobal>(G, this));
 
   // Populate `Events`.
   for (const WasmEvent &E : WasmObj->events())
     Events.emplace_back(make<InputEvent>(Types[E.Type.SigIndex], E, this));
 
   // Populate `Symbols` based on the WasmSymbols in the object.
   Symbols.reserve(WasmObj->getNumberOfSymbols());
   for (const SymbolRef &Sym : WasmObj->symbols()) {
     const WasmSymbol &WasmSym = WasmObj->getWasmSymbol(Sym.getRawDataRefImpl());
+
     if (WasmSym.isDefined()) {
       // createDefined may fail if the symbol is comdat excluded in which case
       // we fall back to creating an undefined symbol
       if (Symbol *D = createDefined(WasmSym)) {
         Symbols.push_back(D);
         continue;
       }
     }

 Symbol *ObjFile::createDefined(const WasmSymbol &Sym) {
   StringRef Name = Sym.Info.Name;
   uint32_t Flags = Sym.Info.Flags;
 
   switch (Sym.Info.Kind) {
   case WASM_SYMBOL_TYPE_FUNCTION: {
     InputFunction *Func =
         Functions[Sym.Info.ElementIndex - WasmObj->getNumImportedFunctions()];
-    if (isExcludedByComdat(Func)) {
-      Func->Live = false;
+    if (Func->Discarded)
       return nullptr;
-    }
-
     if (Sym.isBindingLocal())
       return make<DefinedFunction>(Name, Flags, this, Func);
     return Symtab->addDefinedFunction(Name, Flags, this, Func);
   }
   case WASM_SYMBOL_TYPE_DATA: {
     InputSegment *Seg = Segments[Sym.Info.DataRef.Segment];
-    if (isExcludedByComdat(Seg)) {
-      Seg->Live = false;
+    if (Seg->Discarded)
       return nullptr;
-    }
 
     uint32_t Offset = Sym.Info.DataRef.Offset;
     uint32_t Size = Sym.Info.DataRef.Size;
 
     if (Sym.isBindingLocal())
       return make<DefinedData>(Name, Flags, this, Seg, Offset, Size);
     return Symtab->addDefinedData(Name, Flags, this, Seg, Offset, Size);
   }

 }
 
 Symbol *ObjFile::createUndefined(const WasmSymbol &Sym, bool IsCalledDirectly) {
   StringRef Name = Sym.Info.Name;
   uint32_t Flags = Sym.Info.Flags;
 
   switch (Sym.Info.Kind) {
   case WASM_SYMBOL_TYPE_FUNCTION:
+    if (Sym.isBindingLocal())
+      return make<UndefinedFunction>(Name, Sym.Info.ImportName,
+                                     Sym.Info.ImportModule, Flags, this,
+                                     Sym.Signature, IsCalledDirectly);
     return Symtab->addUndefinedFunction(Name, Sym.Info.ImportName,
                                         Sym.Info.ImportModule, Flags, this,
                                         Sym.Signature, IsCalledDirectly);
   case WASM_SYMBOL_TYPE_DATA:
+    if (Sym.isBindingLocal())
+      return make<UndefinedData>(Name, Flags, this);
     return Symtab->addUndefinedData(Name, Flags, this);
   case WASM_SYMBOL_TYPE_GLOBAL:
+    if (Sym.isBindingLocal())
+      return make<UndefinedGlobal>(Name, Sym.Info.ImportName,
+                                   Sym.Info.ImportModule, Flags, this,
+                                   Sym.GlobalType);
     return Symtab->addUndefinedGlobal(Name, Sym.Info.ImportName,
                                       Sym.Info.ImportModule, Flags, this,
                                       Sym.GlobalType);
   case WASM_SYMBOL_TYPE_SECTION:
     llvm_unreachable("section symbols cannot be undefined");
   }
   llvm_unreachable("unknown symbol kind");
 }

lld/wasm/MarkLive.cpp

if (InputChunk *Chunk = Sym->getChunk())
       Q.push_back(Chunk);
 
     // The ctor functions are all referenced by the synthetic CallCtors
     // function.  However, this function does not contain relocations so we
     // have to manually mark the ctors as live if CallCtors itself is live.
     if (Sym == WasmSym::CallCtors) {
       for (const ObjFile *Obj : Symtab->ObjectFiles) {
         const WasmLinkingData &L = Obj->getWasmObj()->linkingData();
-        for (const WasmInitFunc &F : L.InitFunctions)
-          Enqueue(Obj->getFunctionSymbol(F.Symbol));
+        for (const WasmInitFunc &F : L.InitFunctions) {
+          auto* InitSym = Obj->getFunctionSymbol(F.Symbol);
+          if (InitSym->isDiscarded())
+            continue;
+          Enqueue(InitSym);
+        }
       }
     }
   };
 
   // Add GC root symbols.
   if (!Config->Entry.empty())
     Enqueue(Symtab->find(Config->Entry));
 

lld/wasm/Symbols.h

public:
   }
 
   bool isLazy() const { return SymbolKind == LazyKind; }
 
   bool isLocal() const;
   bool isWeak() const;
   bool isHidden() const;
 
+  // Returns true if this symbol exists in a discard (due to COMDAT) section
+  bool isDiscarded() const;
+
   // True if this is an undefined weak symbol. This only works once
   // all input files have been added.
   bool isUndefWeak() const {
     // See comment on lazy symbols for details.
     return isWeak() && (isUndefined() || isLazy());
   }
 
   // Returns the symbol name.

lld/wasm/Symbols.cpp

 InputChunk *Symbol::getChunk() const {
   if (auto *F = dyn_cast<DefinedFunction>(this))
     return F->Function;
   if (auto *D = dyn_cast<DefinedData>(this))
     return D->Segment;
   return nullptr;
 }
 
+bool Symbol::isDiscarded() const {
+  if (InputChunk *C = getChunk())
+    return C->Discarded;
+  return false;
+}
+
 bool Symbol::isLive() const {
   if (auto *G = dyn_cast<DefinedGlobal>(this))
     return G->Global->Live;
   if (auto *E = dyn_cast<DefinedEvent>(this))
     return E->Event->Live;
   if (InputChunk *C = getChunk())
     return C->Live;
   return Referenced;
 }
 
 void Symbol::markLive() {
+  assert(!isDiscarded());
   if (auto *G = dyn_cast<DefinedGlobal>(this))
     G->Global->Live = true;
   if (auto *E = dyn_cast<DefinedEvent>(this))
     E->Event->Live = true;
   if (InputChunk *C = getChunk())
     C->Live = true;
   Referenced = true;
 }

lld/wasm/Writer.cpp

 void Writer::calculateInitFunctions() {
   if (!Config->Relocatable && !WasmSym::CallCtors->isLive())
     return;
 
   for (ObjFile *File : Symtab->ObjectFiles) {
     const WasmLinkingData &L = File->getWasmObj()->linkingData();
     for (const WasmInitFunc &F : L.InitFunctions) {
       FunctionSymbol *Sym = File->getFunctionSymbol(F.Symbol);
+      // comdat exclusions can sometimes cause init functions be discarded.
+      if (Sym->isDiscarded())
+        continue;
       assert(Sym->isLive());
       if (*Sym->Signature != WasmSignature{{}, {}})
         error("invalid signature for init func: " + toString(*Sym));
       InitFunctions.emplace_back(WasmInitEntry{Sym, F.Priority});
     }
   }
 
   // Sort in order of priority (lowest first) so that they are called