[WebAssembly] Only mark non-hidden symbols as live if they are also defined

[sbc100]

Summary

Previously we were also marking undefined symbols (i.e. imports)
as live.

[sbc100]

comment

[sbc100]

Gentle ping..

[sbc100]

• rebase

[ncw]

Looks good. I'm deducing that when you removed the "hidden" from "used_undef_fn" and "unused_undef_fn", the test then failed - and then the code change makes it pass again. Makes sense.

We could also explicitly test everything - by adding used_undef_hidden_fn/used_undef_public_fn/unused_undef_hidden_fn/unused_undef_public_fn, and the same for the globals as well. Might be worth it, just to cover all the cases.

test/wasm/gc-imports.ll

3 ↗

(On Diff #152186)

Could also remove -print-gc-sections here (or check/assert that its output matches what we expect). I think the checks were lost in all the debate about where and whether different section/function/global types would be reported...

[ncw]

I've been quiet for a while as well ... I keep meaning to get back to my patch queue and try and get them reviewed and submitted. I have a couple of features completed or mostly-completed, I just need a few days away from my main job to get them back on track! Maybe it's time to get LLD ready for native-exception "sections" as well - that's a feature I'm really hoping will arrive soon in WebAssembly.

lld/trunk/test/wasm/gc-imports.ll

 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: yaml2obj %S/Inputs/undefined-globals.yaml -o %t_globals.o
-; RUN: wasm-ld -print-gc-sections --allow-undefined -o %t1.wasm %t.o %t_globals.o
+; RUN: wasm-ld --allow-undefined -o %t1.wasm %t.o %t_globals.o
 
 target triple = "wasm32-unknown-unknown"
 
-declare hidden i64 @unused_undef_function(i64 %arg)
+declare i64 @unused_undef_function(i64 %arg)
 
-declare hidden i32 @used_undef_function()
+declare i32 @used_undef_function()
 
 declare i64 @use_undef_global()
 
+define hidden void @foo() {
+entry:
+  call i64 @unused_undef_function(i64 0)
+  ret void
+}
+
 define hidden void @_start() {
 entry:
   call i32 @used_undef_function()
   call i64 @use_undef_global()
   ret void
 }
 
 ; RUN: obj2yaml %t1.wasm | FileCheck %s
 
-; CHECK:        - 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:       - Index:           2
-; CHECK-NEXT:         ReturnType:      I64
-; CHECK-NEXT:         ParamTypes:
-; CHECK-NEXT:   - Type:            IMPORT
+; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           used_undef_function
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        0
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           used_undef_global
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I64
 ; CHECK-NEXT:         GlobalMutable:   true
 ; CHECK-NEXT:   - Type:
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            used_undef_function
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Name:            __wasm_call_ctors
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            _start
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Name:            use_undef_global
 ; CHECK-NEXT: ...
 
-; RUN: wasm-ld -print-gc-sections --no-gc-sections --allow-undefined \
+; RUN: wasm-ld --no-gc-sections --allow-undefined \
 ; RUN:     -o %t1.no-gc.wasm %t.o %t_globals.o
 ; RUN: obj2yaml %t1.no-gc.wasm | FileCheck %s -check-prefix=NO-GC
 
-; NO-GC:        - Type:            TYPE
-; NO-GC-NEXT:     Signatures:
-; NO-GC-NEXT:       - Index:           0
-; NO-GC-NEXT:         ReturnType:      I32
-; NO-GC-NEXT:         ParamTypes:
-; NO-GC-NEXT:       - Index:           1
-; NO-GC-NEXT:         ReturnType:      I64
-; NO-GC-NEXT:         ParamTypes:
-; NO-GC-NEXT:           - I64
-; NO-GC-NEXT:       - Index:           2
-; NO-GC-NEXT:         ReturnType:      NORESULT
-; NO-GC-NEXT:         ParamTypes:
-; NO-GC-NEXT:       - Index:           3
-; NO-GC-NEXT:         ReturnType:      I64
-; NO-GC-NEXT:         ParamTypes:
-; NO-GC-NEXT:   - Type:            IMPORT
+; NO-GC:        - Type:            IMPORT
 ; NO-GC-NEXT:     Imports:
 ; NO-GC-NEXT:       - Module:          env
-; NO-GC-NEXT:         Field:           used_undef_function
+; NO-GC-NEXT:         Field:           unused_undef_function
 ; NO-GC-NEXT:         Kind:            FUNCTION
 ; NO-GC-NEXT:         SigIndex:        0
 ; NO-GC-NEXT:       - Module:          env
-; NO-GC-NEXT:         Field:           unused_undef_function
+; NO-GC-NEXT:         Field:           used_undef_function
 ; NO-GC-NEXT:         Kind:            FUNCTION
 ; NO-GC-NEXT:         SigIndex:        1
 ; NO-GC-NEXT:       - Module:          env
 ; NO-GC-NEXT:         Field:           unused_undef_global
 ; NO-GC-NEXT:         Kind:            GLOBAL
 ; NO-GC-NEXT:         GlobalType:      I64
 ; NO-GC-NEXT:         GlobalMutable:   true
 ; NO-GC-NEXT:       - Module:          env
 ; NO-GC-NEXT:         Field:           used_undef_global
 ; NO-GC-NEXT:         Kind:            GLOBAL
 ; NO-GC-NEXT:         GlobalType:      I64
 ; NO-GC-NEXT:         GlobalMutable:   true
 ; NO-GC-NEXT:   - Type:
 ; NO-GC:        - Type:            CUSTOM
 ; NO-GC-NEXT:     Name:            name
 ; NO-GC-NEXT:     FunctionNames:
 ; NO-GC-NEXT:       - Index:           0
-; NO-GC-NEXT:         Name:            used_undef_function
-; NO-GC-NEXT:       - Index:           1
 ; NO-GC-NEXT:         Name:            unused_undef_function
+; NO-GC-NEXT:       - Index:           1
+; NO-GC-NEXT:         Name:            used_undef_function
 ; NO-GC-NEXT:       - Index:           2
 ; NO-GC-NEXT:         Name:            __wasm_call_ctors
 ; NO-GC-NEXT:       - Index:           3
-; NO-GC-NEXT:         Name:            _start
+; NO-GC-NEXT:         Name:            foo
 ; NO-GC-NEXT:       - Index:           4
+; NO-GC-NEXT:         Name:            _start
+; NO-GC-NEXT:       - Index:           5
 ; NO-GC-NEXT:         Name:            use_undef_global
 ; NO-GC-NEXT: ...

lld/trunk/wasm/MarkLive.cpp

 
 void lld::wasm::markLive() {
   LLVM_DEBUG(dbgs() << "markLive\n");
   SmallVector<InputChunk *, 256> Q;
 
   auto Enqueue = [&](Symbol *Sym) {
     if (!Sym || Sym->isLive())
       return;
+    LLVM_DEBUG(dbgs() << "markLive: " << Sym->getName() << "\n");
     Sym->markLive();
     if (Sym->SignatureMismatch)
       error("function signature mismatch: " + Sym->getName());
     if (InputChunk *Chunk = Sym->getChunk())
       Q.push_back(Chunk);
   };
 
   // Add GC root symbols.
   if (!Config->Entry.empty())
     Enqueue(Symtab->find(Config->Entry));
   Enqueue(WasmSym::CallCtors);
 
-  // By default we export all non-hidden, so they are gc roots too
+  // We export all defined, non-hidden symbols so they are all gc roots too
   for (Symbol *Sym : Symtab->getSymbols())
-    if (!Sym->isHidden())
+    if (Sym->isDefined() && !Sym->isHidden())
       Enqueue(Sym);
 
   // The ctor functions are all used in the synthetic __wasm_call_ctors
   // function, but since this function is created in-place it doesn't contain
   // relocations which mean we have to manually mark the ctors.
   for (const ObjFile *Obj : Symtab->ObjectFiles) {
     const WasmLinkingData &L = Obj->getWasmObj()->linkingData();
     for (const WasmInitFunc &F : L.InitFunctions)

lld/trunk/wasm/Symbols.h

Symbol(StringRef Name, Kind K, uint32_t Flags, InputFile *F)
       : IsUsedInRegularObj(false), SignatureMismatch(false), Name(Name),
         SymbolKind(K), Flags(Flags), File(F), Referenced(!Config->GcSections) {}
 
   StringRef Name;
   Kind SymbolKind;
   uint32_t Flags;
   InputFile *File;
   uint32_t OutputSymbolIndex = INVALID_INDEX;
-  bool Referenced = false;
+  bool Referenced;
 };
 
 class FunctionSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) {
     return S->kind() == DefinedFunctionKind ||
            S->kind() == UndefinedFunctionKind;
   }