[lld][WebAssembly] Don't export deps for unused stub symbols by aheejin · Pull Request #173422 · llvm/llvm-project

aheejin · 2025-12-23T22:46:32Z

When a stub .so file contains

A: B

And A is defined in bitcode that's pulled in for LTO, but both A and B are removed in LTO::linkRegularLTO due to not being dead:

llvm-project/llvm/lib/LTO/LTO.cpp

Lines 1042 to 1054 in 24297be

    
           if (LivenessFromIndex && !ThinLTO.CombinedIndex.isGUIDLive(GV->getGUID())) { 
        
             if (Function *F = dyn_cast<Function>(GV)) { 
        
               if (DiagnosticOutputFile) { 
        
                 if (Error Err = F->materialize()) 
        
                   return Err; 
        
                 OptimizationRemarkEmitter ORE(F, nullptr); 
        
                 ORE.emit(OptimizationRemark(DEBUG_TYPE, "deadfunction", F) 
        
                          << ore::NV("Function", F) 
        
                          << " not added to the combined module "); 
        
               } 
        
             } 
        
             continue; 
        
           }

Then the symbol A becomes undefined after LTO, processStubLibraries tries to import A from JS, and tries to export its dependency B:

llvm-project/lld/wasm/Driver.cpp

Lines 1108 to 1109 in 24297be

    
           if (sym && sym->isUndefined()) { 
        
             depsAdded |= addStubSymbolDeps(stub_file, sym, deps);

But B is gone, causing this error:

wasm-ld: error: ....: undefined symbol: B. Required by A

This PR checks if the symbol is used in regular objects before trying to exporrt its dependences, ensuring the case above doesn't crash the linker.

When a stub .so file contains ``` A: B ``` And `A` is defined in bitcode that's pulled in for LTO, but both `A` and `B` are removed in `LTO::linkRegularLTO` due to not being dead: https://github.com/llvm/llvm-project/blob/24297bea9672722d8fbaaff137b301b0becaae9c/llvm/lib/LTO/LTO.cpp#L1042-L1054 Then the symbol `A` becomes undefined after LTO, `processStubLibraries` tries to import `A` from JS, and tries to export its dependency `B`: https://github.com/llvm/llvm-project/blob/24297bea9672722d8fbaaff137b301b0becaae9c/lld/wasm/Driver.cpp#L1108-L1109 But `B` is gone, causing this error: ```console wasm-ld: error: ....: undefined symbol: B. Required by A ``` This PR checks if the symbol is used in regular objects before trying to exporrt its dependences, ensuring the case above doesn't crash the linker.

llvmbot · 2025-12-23T22:46:58Z

@llvm/pr-subscribers-lld-wasm

@llvm/pr-subscribers-lld

Author: Heejin Ahn (aheejin)

Changes

When a stub .so file contains

A: B

And A is defined in bitcode that's pulled in for LTO, but both A and B are removed in LTO::linkRegularLTO due to not being dead:

llvm-project/llvm/lib/LTO/LTO.cpp

Lines 1042 to 1054 in 24297be

    
           if (LivenessFromIndex && !ThinLTO.CombinedIndex.isGUIDLive(GV->getGUID())) { 
        
             if (Function *F = dyn_cast<Function>(GV)) { 
        
               if (DiagnosticOutputFile) { 
        
                 if (Error Err = F->materialize()) 
        
                   return Err; 
        
                 OptimizationRemarkEmitter ORE(F, nullptr); 
        
                 ORE.emit(OptimizationRemark(DEBUG_TYPE, "deadfunction", F) 
        
                          << ore::NV("Function", F) 
        
                          << " not added to the combined module "); 
        
               } 
        
             } 
        
             continue; 
        
           }

Then the symbol A becomes undefined after LTO, processStubLibraries tries to import A from JS, and tries to export its dependency B:

llvm-project/lld/wasm/Driver.cpp

Lines 1108 to 1109 in 24297be

    
           if (sym && sym->isUndefined()) { 
        
             depsAdded |= addStubSymbolDeps(stub_file, sym, deps);

But B is gone, causing this error:

wasm-ld: error: ....: undefined symbol: B. Required by A

This PR checks if the symbol is used in regular objects before trying to exporrt its dependences, ensuring the case above doesn't crash the linker.

Full diff: https://github.com/llvm/llvm-project/pull/173422.diff

4 Files Affected:

(modified) lld/test/wasm/lto/Inputs/foo.ll (+11-1)
(modified) lld/test/wasm/lto/Inputs/stub.so (+1)
(modified) lld/test/wasm/lto/stub-library.s (+6)
(modified) lld/wasm/Driver.cpp (+1-1)

diff --git a/lld/test/wasm/lto/Inputs/foo.ll b/lld/test/wasm/lto/Inputs/foo.ll
index f3b54cc7d0094..695b552fc6249 100644
--- a/lld/test/wasm/lto/Inputs/foo.ll
+++ b/lld/test/wasm/lto/Inputs/foo.ll
@@ -1,7 +1,17 @@
 target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
 target triple = "wasm32-unknown-unknown"
 
-define void @foo() local_unnamed_addr {
+define void @foo() {
+entry:
+  ret void
+}
+
+define void @baz() {
+entry:
+  ret void
+}
+
+define void @quux() {
 entry:
   ret void
 }
diff --git a/lld/test/wasm/lto/Inputs/stub.so b/lld/test/wasm/lto/Inputs/stub.so
index e76c890dbb435..c471b81da5064 100644
--- a/lld/test/wasm/lto/Inputs/stub.so
+++ b/lld/test/wasm/lto/Inputs/stub.so
@@ -1,3 +1,4 @@
 #STUB
 bar: foo
 memcpy: foo
+baz: quux
diff --git a/lld/test/wasm/lto/stub-library.s b/lld/test/wasm/lto/stub-library.s
index 36d6b7b3f38aa..d77296b6cff3d 100644
--- a/lld/test/wasm/lto/stub-library.s
+++ b/lld/test/wasm/lto/stub-library.s
@@ -1,6 +1,10 @@
 ## The function `bar` is declared in stub.so and depends on `foo` which is
 ## defined in an LTO object.  We also test the case where the LTO object is
 ## with an archive file.
+## The function `baz` is declared in stub.so and depends on `quux`, and both
+## `baz` and `quux` are defined in an LTO object. When `baz` and `quux` are
+## DCE'd and become undefined in the LTO process, wasm-ld should not try to
+## export the (nonexistent) `quux`.
 ## This verifies that stub library dependencies (which are required exports) can
 ## be defined in LTO objects, even when they are within archive files.
 
@@ -40,3 +44,5 @@ _start:
 # CHECK-NEXT:       - Name:            foo
 # CHECK-NEXT:         Kind:            FUNCTION
 # CHECK-NEXT:         Index:           2
+
+# CHECK-NOT:        - Name:            quux
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 97e50783985a8..d18126bef766f 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -1105,7 +1105,7 @@ static void processStubLibraries() {
       // the names of the stub imports
       for (auto [name, deps]: stub_file->symbolDependencies) {
         auto* sym = symtab->find(name);
-        if (sym && sym->isUndefined()) {
+        if (sym && sym->isUndefined() && sym->isUsedInRegularObj) {
           depsAdded |= addStubSymbolDeps(stub_file, sym, deps);
         } else {
           if (sym && sym->traced)

sbc100

lgtm % comment

sbc100 · 2025-12-23T22:51:20Z

+  ret void
+}
+
+define void @quux() {


Can you make a new input file rather than adding to this (since this one is just called foo I think its good it just defines foo)

What I will create is basically another file with two methods. I can actually reuse this file and foo and baz here (without adding anything to it) but what I need is a different stub file having one line foo: baz (or vice versa). How about creating a new stub file? (You preferred a new file rather than echo'ing it, right?)

The reason I can't reuse the existing stub file is foo is a dependency for both bar and memcpy here:

#STUB bar: foo memcpy: foo

Will create a new stub file. Tell me what you think

Oh sorry, baz is what I added... I thought it was already there. Will create another input file. Ignore the comment above

I don't think you need to make a new stub file. The stub file and the .ll file are not related explicitly.

Perhsonally I would just create a single new .ll file in under inputs to use for this test and not use the existings Inputs/foo.ll

Created another input and stub file: 423fcfc

aheejin · 2025-12-23T23:51:05Z

@@ -0,0 +1,3 @@
+#STUB
+bar:


This entry is necessary not to crash because stub-library.s has a bar declaration. Without this we crash with something like undefined symbol: bar

Can you explain why we need two different stub .so files?

Why can't you just add foo: baz to the existing stub? Or at least it seems like we should just be able to add a single new entry to repro this issue.

sbc100 · 2025-12-23T23:51:25Z

+
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
+# RUN: mkdir -p %t
+# RUN: llvm-as %S/Inputs/foo.ll -o %t/foo.o


Should this bee foobaz.ll here and above?

Also, I guess the object file and the archive file should get renamed accordingly.

Fixed: 9545345

sbc100 · 2025-12-23T23:58:57Z

+# RUN: rm -f %t/libfoobaz.a
+# RUN: llvm-ar rcs %t/libfoobaz.a %t/foobaz.o
+# RUN: wasm-ld %t.o %t/libfoobaz.a %p/Inputs/stub2.so -o %t2.wasm
+# RUN: obj2yaml %t2.wasm | FileCheck %s --check-prefix=UNUSED


I would hope that we could avoid duplicating all these lines and simply modify the existing lines above to use foobaz.ll instead of foo.ll?

Then it can't be foo because foo is already being used in stub.so and I need a symbol that's not used anywhere. Anyway, I will remove foo.ll and create another file that contains all functions we need then.

Then it becomes basically the same as the status before creating foobaz.ll, with only the name of foo.ll changed to something else though. (Also I don't think we need two different stub files then either...)

Reverted the last two commits and changed foo.ll to funcs.ll: a44a6c0

This reverts commit 9545345.

This reverts commit 423fcfc.

sbc100 · 2025-12-24T00:43:22Z

Thanks!

…3422) When a stub .so file contains ``` A: B ``` And `A` is defined in bitcode that's pulled in for LTO, but both `A` and `B` are removed in `LTO::linkRegularLTO` due to not being dead: https://github.com/llvm/llvm-project/blob/24297bea9672722d8fbaaff137b301b0becaae9c/llvm/lib/LTO/LTO.cpp#L1042-L1054 Then the symbol `A` becomes undefined after LTO, `processStubLibraries` tries to import `A` from JS, and tries to export its dependency `B`: https://github.com/llvm/llvm-project/blob/24297bea9672722d8fbaaff137b301b0becaae9c/lld/wasm/Driver.cpp#L1108-L1109 But `B` is gone, causing this error: ```console wasm-ld: error: ....: undefined symbol: B. Required by A ``` This PR checks if the symbol is used in regular objects before trying to exporrt its dependences, ensuring the case above doesn't crash the linker.

aheejin requested a review from sbc100 December 23, 2025 22:46

llvmbot added lld lld:wasm labels Dec 23, 2025

aheejin mentioned this pull request Dec 23, 2025

[lld][WebAssembly] Preserve LTO stub deps for bitcode symbols #173235

Closed

sbc100 approved these changes Dec 23, 2025

View reviewed changes

Create another input / stub file

423fcfc

aheejin commented Dec 23, 2025

View reviewed changes

sbc100 reviewed Dec 23, 2025

View reviewed changes

foo -> foobaz

9545345

sbc100 reviewed Dec 23, 2025

View reviewed changes

aheejin added 4 commits December 24, 2025 00:25

Revert "foo -> foobaz"

4a00bcd

This reverts commit 9545345.

Revert "Create another input / stub file"

a733607

This reverts commit 423fcfc.

Rename foo.ll to funcs.ll

a44a6c0

foo.ll -> funcs.ll in stub-library-libcall.s

6eae996

sbc100 approved these changes Dec 24, 2025

View reviewed changes

aheejin merged commit 9b020dd into llvm:main Dec 24, 2025
10 checks passed

aheejin deleted the fix_stub2 branch December 24, 2025 00:58

	if (LivenessFromIndex && !ThinLTO.CombinedIndex.isGUIDLive(GV->getGUID())) {
	if (Function *F = dyn_cast<Function>(GV)) {
	if (DiagnosticOutputFile) {
	if (Error Err = F->materialize())
	return Err;
	OptimizationRemarkEmitter ORE(F, nullptr);
	ORE.emit(OptimizationRemark(DEBUG_TYPE, "deadfunction", F)
	<< ore::NV("Function", F)
	<< " not added to the combined module ");
	}
	}
	continue;
	}

	if (sym && sym->isUndefined()) {
	depsAdded \|= addStubSymbolDeps(stub_file, sym, deps);

Conversation

aheejin commented Dec 23, 2025

Uh oh!

llvmbot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aheejin Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 commented Dec 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

llvmbot commented Dec 23, 2025 •

edited

Loading

aheejin Dec 23, 2025 •

edited

Loading