[RyuJit Wasm] Fix Signature Generation Bug with Un-called methods by adamperlin · Pull Request #125090 · dotnet/runtime

adamperlin · 2026-03-03T00:36:26Z

#124685 changed our signature recording logic in favor of using WasmTypeNodes in the dependency graph. However, we do not create these nodes currently unless they are the target of a relocation, but every compiled method will need one. This PR adds WasmTypeNode as a static dependency of MethodWithGCInfo to make sure we always create a type signature node for every method we compile.

…ithGCInfo on Wasm, clean up signature index assignment logic

Copilot

Pull request overview

Ensures WebAssembly ReadyToRun compilation always materializes a WasmTypeNode (type signature) for every compiled managed method, fixing missing signature/type-section entries for methods that were compiled but never referenced by relocations.

Changes:

Add a NodeFactory.WasmTypeNode(MethodDesc) overload that derives the signature via WasmLowering.GetSignature.
Add a static dependency from MethodWithGCInfo to the method’s WasmTypeNode when targeting Wasm32.
Simplify signature index assignment in WasmObjectWriter.RecordMethodSignature by using _uniqueSignatures.Count.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs	Adds a factory helper to create/cache `WasmTypeNode` from a `MethodDesc` signature.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs	Ensures each compiled method depends on (and thus emits/records) its Wasm signature node.
src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs	Uses dictionary count as the next signature index (removes redundant counter).

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: Michal Strehovský <[email protected]>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs

SingleAccretion · 2026-03-03T12:55:51Z

WasmTypeNode as a static dependency of MethodWithGCInfo to make sure we always create a type signature node for every method we compile.

I don't think this is an unreasonable approach, but MethodWithGCInfo is not going to be the only node that needs a signature in the long run. All nodes that may inspire imports need a signature. Hand-generated WASM helpers will need a signature. It is going to be quite pervasive, especially for NAOT. Still better than handling it implicitly in the object writer?

adamperlin · 2026-03-03T18:14:13Z

WasmTypeNode as a static dependency of MethodWithGCInfo to make sure we always create a type signature node for every method we compile.

I don't think this is an unreasonable approach, but MethodWithGCInfo is not going to be the only node that needs a signature in the long run. All nodes that may inspire imports need a signature. Hand-generated WASM helpers will need a signature. It is going to be quite pervasive, especially for NAOT. Still better than handling it implicitly in the object writer?

This is a good point, and I wasn't considering all the possible NAOT nodes nodes that may need to have this new dependency. The reason I decided on this approach was for clarity -- it seemed clearer to have all type signatures originate in the dependency graph and to simply process them in the object writer, as opposed to having some originate in the graph (for reloc targets) and others in the object writer.

kg · 2026-03-03T22:06:13Z

My work is blocked on this, can we land it as-is or is there something wrong with it?

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: SingleAccretion <[email protected]>

adamperlin · 2026-03-04T00:02:49Z

My work is blocked on this, can we land it as-is or is there something wrong with it?

I think we can merge as-is so you're unblocked. I do have a follow up I'm working on that I'll want to merge shortly (basically, an interface that code nodes can implement so that we enforce that they do indeed produce a type signature in the graph).

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

kg · 2026-03-04T00:09:27Z

I was able to just apply your changes locally so we can take our time merging this if you want.

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: Copilot <[email protected]>

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Change IWasmCodeNode to look more like IMethodNode for ease of integration. Implement IWasmMethodCodeNode for code carrying nodes, and a dispatch check in ObjectNode.GetStaticDependencies to ensure we add a new edge and node to represent the type signature of a method code node in the dependency graph.

…b.com:adamperlin/runtime into adamperlin/wasm-crossgen-method-typenode-deps

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

adamperlin · 2026-03-06T19:27:19Z

@MichalStrehovsky I've implemented your suggestions for the interface / ObjectNode implementation. Please feel free to take another look when you have a chance. I'm leaving out AssemblyStubNode as an impl of the interface for now, as I think what we need to do there will become clearer shortly as we implement import thunks for Wasm!

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

.../ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DelayLoadHelperMethodImport.cs

src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs

…> IMethodCodeNodeWithTypeSignature; remove some redundant interface impl declarations

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs

src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs

kg · 2026-03-13T17:24:39Z

EDIT: Ignore this

…25090) #124685 changed our signature recording logic in favor of using `WasmTypeNode`s in the dependency graph. However, we do not create these nodes currently unless they are the target of a relocation, but every compiled method will need one. This PR adds `WasmTypeNode` as a static dependency of `MethodWithGCInfo` to make sure we always create a type signature node for every method we compile. --------- Co-authored-by: Michal Strehovský <[email protected]> Co-authored-by: SingleAccretion <[email protected]> Co-authored-by: Copilot <[email protected]>

Crossgen Wasm fixes: add WasmTypeNode as static dependency of MethodW…

03dee5f

…ithGCInfo on Wasm, clean up signature index assignment logic

adamperlin requested a review from MichalStrehovsky as a code owner March 3, 2026 00:36

Copilot AI review requested due to automatic review settings March 3, 2026 00:36

github-actions bot added the area-crossgen2-coreclr label Mar 3, 2026

dotnet-policy-service bot assigned adamperlin Mar 3, 2026

Copilot started reviewing on behalf of adamperlin March 3, 2026 00:37 View session

adamperlin requested a review from AndyAyersMS March 3, 2026 00:37

Copilot AI reviewed Mar 3, 2026

View reviewed changes

MichalStrehovsky reviewed Mar 3, 2026

View reviewed changes

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs Outdated Show resolved Hide resolved

MichalStrehovsky reviewed Mar 3, 2026

View reviewed changes

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs Outdated Show resolved Hide resolved

Update src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/Dependenc…

04ff689

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: Michal Strehovský <[email protected]>

Copilot AI review requested due to automatic review settings March 3, 2026 01:43

Copilot started reviewing on behalf of adamperlin March 3, 2026 01:45 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

adamperlin requested a review from MichalStrehovsky March 3, 2026 02:00

build-analysis bot mentioned this pull request Mar 3, 2026

[android][clr] No peer certificates when executing System.Net.Http.Functional.Tests on Android emulator #124526

Open

SingleAccretion reviewed Mar 3, 2026

View reviewed changes

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs Outdated Show resolved Hide resolved

kg approved these changes Mar 3, 2026

View reviewed changes

Update src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/Dependenc…

b0bf243

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: SingleAccretion <[email protected]>

Copilot AI review requested due to automatic review settings March 3, 2026 23:58

Copilot started reviewing on behalf of adamperlin March 4, 2026 00:01 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

This was referenced Mar 4, 2026

[android] Android.Device_Emulator.JIT.Test failing on emulators with CoreCLR #112633

Open

[FAIL] System.Net.Http.Functional.Tests.SocketsHttpHandler_HttpClientHandlerTest.LargeUriAndHeaders_Works #124487

Open

am11 added the arch-wasm WebAssembly architecture label Mar 4, 2026

Update src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/Dependenc…

b266382

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: Copilot <[email protected]>

Copilot started reviewing on behalf of adamperlin March 5, 2026 01:22 View session

MichalStrehovsky reviewed Mar 5, 2026

View reviewed changes

...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs Outdated Show resolved Hide resolved

Copilot AI reviewed Mar 5, 2026

View reviewed changes

This was referenced Mar 5, 2026

System.IO.Tests.Directory_Create_Tests.FileSystemWatcher_Directory_Create_InNestedDirectory fails with "Created event did not occur as expected" #103630

Open

Android WebSocket failure #121518

Open

adamperlin added 3 commits March 6, 2026 09:46

Merge branch 'adamperlin/wasm-crossgen-method-typenode-deps' of githu…

8c84283

…b.com:adamperlin/runtime into adamperlin/wasm-crossgen-method-typenode-deps

Add additional TODOs and remove stray GetWasmTypeSignature definitions

6c38afe

Copilot AI review requested due to automatic review settings March 6, 2026 18:06

Copilot started reviewing on behalf of adamperlin March 6, 2026 18:07 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs Show resolved Hide resolved

adamperlin requested a review from davidwrighton March 6, 2026 21:54

This was referenced Mar 6, 2026

XHarness package install failure on iOS due to devicectl NSPOSIXErrorDomain error 49 #123796

Open

[8.0] System.Exception: Failed to list devices - tvOS, iOS #125135

Open

MichalStrehovsky reviewed Mar 9, 2026

View reviewed changes

adamperlin added 2 commits March 9, 2026 11:34

Rename IWasmCodeNode -> INodeWithTypeSignature, IWasmMethodCodeNode -…

905ed72

…> IMethodCodeNodeWithTypeSignature; remove some redundant interface impl declarations

Group more usings of ILCompiler.DependencyAnalysis.Wasm

4c84b2a

Copilot AI review requested due to automatic review settings March 9, 2026 18:41

Copilot started reviewing on behalf of adamperlin March 9, 2026 18:45 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

Remove trailing whitespace

b135e60

MichalStrehovsky approved these changes Mar 10, 2026

View reviewed changes

src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs Show resolved Hide resolved

davidwrighton approved these changes Mar 11, 2026

View reviewed changes

adamperlin merged commit 32a7ecb into dotnet:main Mar 11, 2026
105 checks passed

adamperlin deleted the adamperlin/wasm-crossgen-method-typenode-deps branch March 11, 2026 16:47

dotnet-maestro bot mentioned this pull request Mar 12, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#5408

Merged

Conversation

adamperlin commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

SingleAccretion commented Mar 3, 2026

Uh oh!

adamperlin commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kg commented Mar 3, 2026

Uh oh!

adamperlin commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

kg commented Mar 4, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

adamperlin commented Mar 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kg commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

adamperlin commented Mar 3, 2026 •

edited

Loading

adamperlin commented Mar 3, 2026 •

edited

Loading

adamperlin commented Mar 4, 2026 •

edited

Loading

kg commented Mar 13, 2026 •

edited

Loading