[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ý <MichalStrehovsky@users.noreply.github.com>

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 <62474226+SingleAccretion@users.noreply.github.com>

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.

Copilot

Pull request overview

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

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

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

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

…nalysis/FatFunctionPointerNode.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

...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

MichalStrehovsky · 2026-03-09T05:21:32Z

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

        }

-        private protected virtual void RecordMethodDeclaration(ISymbolDefinitionNode node, MethodDesc desc)
+        private protected virtual void RecordMethodDeclaration(IWasmCodeNode node, MethodDesc desc)


Hmm, I think we'd want to keep this as something general purpose. The general object writing API surface shouldn't have WASM things in it if we can help it.

We have INodeWithDebugInfo, INodeWithCodeInfo... maybe this interface is not Wasm specific and can be just INodeWithSignatureInfo (i.e. rename IWasmCodeNode to INodeWithSignatureInfo so that we don't add WASM specific virtuals)?

I can try renaming this to INodeWithSignatureInfo. If we want to keep Wasm-specific things out of the object writing API surface, we may also want to rename WasmTypeNode to PredeclaredSignatureNode or something similar? or we could have: WasmTypeNode : IPredeclaredSignatureNode and check for IPredeclaredSignatureNode in the object writer?

@MichalStrehovsky this should be addressed now!

we may also want to rename WasmTypeNode to PredeclaredSignatureNode or something similar? or we could have: WasmTypeNode : IPredeclaredSignatureNode and check for IPredeclaredSignatureNode in the object writer?

I think that part can eventually be done somewhere else. I think we'll be able to stop special casing this node.

…> 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

Copilot · 2026-03-09T18:54:35Z

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

 namespace ILCompiler.DependencyAnalysis.ReadyToRun
 {
-    public class MethodWithGCInfo : ObjectNode, IMethodBodyNode, ISymbolDefinitionNode
+    public class MethodWithGCInfo : ObjectNode, IMethodBodyNode, IMethodCodeNodeWithTypeSignature 


There’s trailing whitespace at the end of the class declaration line. This tends to cause noisy diffs and can violate repo formatting rules; please remove the extra space.

MichalStrehovsky · 2026-03-10T00:44:51Z

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

 using ILCompiler.DependencyAnalysis;
 using ILCompiler.DependencyAnalysis.ReadyToRun;
 using ILCompiler.DependencyAnalysis.Wasm;
+


I don't know if this diff makes sense. The System ones should go all the way up and the last ILCompiler one can stay in this group. I would not touch this. Not sure if it's worth respinning CI just for that though.

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ý <MichalStrehovsky@users.noreply.github.com>

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 <62474226+SingleAccretion@users.noreply.github.com>

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

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

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

3 tasks

Copilot AI reviewed Mar 4, 2026

View reviewed changes

This was referenced Mar 4, 2026

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

[android-arm64] The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#6408

Open

Update src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyA…

860d99f

…nalysis/FatFunctionPointerNode.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 5, 2026 01:21

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

b266382

…yAnalysis/ReadyToRun/MethodWithGCInfo.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

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

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!

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!

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!

MichalStrehovsky Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

adamperlin Mar 9, 2026 • 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!

adamperlin Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

MichalStrehovsky Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

MichalStrehovsky Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

adamperlin commented Mar 3, 2026 •

edited

Loading

adamperlin commented Mar 3, 2026 •

edited

Loading

adamperlin commented Mar 4, 2026 •

edited

Loading

adamperlin Mar 9, 2026 •

edited

Loading