Skip to content

Conversation

radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Sep 22, 2025

MethodDescCallSite::CallTargetWorker was setting wrong size in the call description data, resulting in only one argument being copied to the interpreter stack.

@radekdoulik radekdoulik added this to the Future milestone Sep 22, 2025
@radekdoulik radekdoulik added arch-wasm WebAssembly architecture area-VM-coreclr labels Sep 22, 2025
Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

@radekdoulik
Copy link
Member Author

I noticed that when I was fixing the other case with simple calls.

The changes here should fix it, they lead to a new problem though, so it will need more work.

Currently it crashes with

RuntimeError: memory access out of bounds
    at corerun.wasm.DispatchToken::DecodeTypeID(unsigned long) (wasm://wasm/corerun.wasm-1b64916e:wasm-function[14974]:0x5801c5)
    at corerun.wasm.DispatchToken::GetTypeID() const (wasm://wasm/corerun.wasm-1b64916e:wasm-function[14958]:0x57dea9)
    at corerun.wasm.DispatchToken::IsThisToken() const (wasm://wasm/corerun.wasm-1b64916e:wasm-function[14972]:0x57fe08)
    at corerun.wasm.VirtualCallStubManager::GetTarget(DispatchToken, MethodTable*, int) (wasm://wasm/corerun.wasm-1b64916e:wasm-function[14976]:0x5804aa)
    at corerun.wasm.MethodTable::GetMethodDescForInterfaceMethod(TypeHandle, MethodDesc*, int) (wasm://wasm/corerun.wasm-1b64916e:wasm-function[12092]:0x479f61)
    at corerun.wasm.MethodTable::GetMethodDescForInterfaceMethodAndServer(TypeHandle, MethodDesc*, OBJECTREF*) (wasm://wasm/corerun.wasm-1b64916e:wasm-function[12091]:0x479d55)
    at corerun.wasm.MethodDesc::GetMethodDescOfVirtualizedCode(OBJECTREF*, TypeHandle) (wasm://wasm/corerun.wasm-1b64916e:wasm-function[11754]:0x46015d)
    at corerun.wasm.InterpExecMethod(InterpreterFrame*, InterpMethodContextFrame*, InterpThreadContext*, ExceptionClauseArgs*) (wasm://wasm/corerun.wasm-1b64916e:wasm-function[17466]:0x6af64a)
    at corerun.wasm.ExecuteInterpretedMethod (wasm://wasm/corerun.wasm-1b64916e:wasm-function[13211]:0x4d8451)
    at corerun.wasm.ExecuteInterpretedMethodWithArgs (wasm://wasm/corerun.wasm-1b64916e:wasm-function[13219]:0x4d8973)

It wasn't crashing before probably because the call with 3 arguments to AppDomain:Setup resulted 3rd argument being zero and that is the properties count. It was the only call so far with more than 1 argument, so I think we were just lucky.

@radekdoulik
Copy link
Member Author

radekdoulik commented Sep 23, 2025

Now we are getting stack overflow, because of missing System.Runtime.Intrinsics.Wasm.WasmBase:get_IsSupported() intrinsic.

Compiled method: .System.Runtime.Intrinsics.Wasm.WasmBase:get_IsSupported()
Locals size 16
0004: IR_0000: initlocals     [nil <- nil], 0,0
0010: IR_0003: safepoint      [nil <- nil],
0014: IR_0004: call           [0 <- 16], .System.Runtime.Intrinsics.Wasm.WasmBase:get_IsSupported()
0024: IR_0008: ret            [nil <- 0],
End of method: 002c: IR_000a

Allocating gcinfo slots for 1 vars
EH info:
  None
Aborted(Stack overflow! Stack cookie has been overwritten at 0x00000004, expected hex dwords 0x89BACDFE and 0x2135467, but received 0x00000000 0x02135467)

Looks like it is not recognized as intrinsic method.

@jkotas
Copy link
Member

jkotas commented Sep 23, 2025

Looks like it is not recognized as intrinsic method.

This should be fixed here:

if (hr == S_OK && (strcmp(nameSpace, "System.Runtime.Intrinsics.X86") == 0))

@radekdoulik
Copy link
Member Author

Now we get to the missing allocator. I will look at it tomorrow.

ASSERT FAILED
	Expression: false && "RhpNewVariableSizeObject is not yet implemented"
	Location:   line 11 in /Users/rodo/git/runtime-main/src/coreclr/runtime/portable/AllocFast.cpp
	Function:   RhpNewVariableSizeObject
	Process:    42
Frame (InterpreterFrame): 0x4ff188
   0) System.String::FastAllocateString, IR_001d
   1) System.String::Ctor, IR_004b
   2) System.AppContext::Setup, IR_006b

@radekdoulik radekdoulik marked this pull request as ready for review September 24, 2025 09:54
@Copilot Copilot AI review requested due to automatic review settings September 24, 2025 09:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an argument passing bug in MethodDescCallSite::CallTargetWorker on WebAssembly where the wrong size was being set in the call description data, causing only one argument to be copied to the interpreter stack instead of all arguments.

Key changes:

  • Implements the StackElemSize function for WASM to properly calculate stack element sizes
  • Fixes argument size calculation in CallTargetWorker to use the correct nStackBytes value
  • Adds WASM support to hardware intrinsics handling and virtual call stub management

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/coreclr/vm/wasm/cgencpu.h Implements StackElemSize function for WASM architecture
src/coreclr/vm/callhelpers.cpp Fixes argument size calculation in CallTargetWorker
src/coreclr/vm/virtualcallstub.cpp Adds conditional compilation for portable entrypoints on WASM
src/coreclr/vm/methodtablebuilder.cpp Extends hardware intrinsics support to include WASM architecture
src/coreclr/vm/contractimpl.h Disables fat token stress testing on WASM
src/coreclr/runtime/portable/AllocFast.cpp Implements RhpNewVariableSizeObject for WASM

@radekdoulik
Copy link
Member Author

With the allocator in place, the initialization passes again.

@radekdoulik
Copy link
Member Author

/ba-g unrelated timeouts

@radekdoulik radekdoulik merged commit bd1d206 into dotnet:main Sep 25, 2025
109 of 113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-VM-coreclr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants