[do not review] test regression 2 by radekdoulik · Pull Request #124522 · dotnet/runtime

radekdoulik · 2026-02-17T18:19:42Z

No description provided.

Set `isTailcall` to `false`, when preparing the call. This is similar to how we set other state vars for the call, so I did it this way. Alternatively we can reset the `isTailcall` after updating the stack and frame for tailcalls. That would invalidate the meaning of the variable for the rest of the tailcall though, so we would need to rename it at least to signal, that it is valid only in the beginning of the tailcall.

Based on changes from the previous branch (https://github.com/radekdoulik/runtime/tree/clr-wasm-runtime-tests-with-node)

…hecked job

…on (dotnet#123215) Fixes dotnet#122923

…s' into clr-wasm-runtime-tests-on-node-2

dotnet-policy-service · 2026-02-17T18:20:58Z

Tagging subscribers to this area: @agocke, @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR adds support for running CoreCLR WASM tests with NodeJS and implements native symbol map functionality for better stack trace symbolication in WASM environments. The changes enable running tests on platforms that use NodeJS instead of V8 shell, improving test coverage for CoreCLR on browser/WASM.

Changes:

Added --node command-line option to test runners (run.sh, run.py, run.cmd) with automatic default for WASM architecture
Implemented WASM symbol map loading and stack trace symbolication for debugging
Refactored CoreCLR prestub logic to move DoPrestub calls out of CallDescrWorkerInternal
Improved exit code propagation through BrowserHost_ExecuteAssembly and BrowserHost_ShutdownCoreCLR
Updated threading tests to use ConditionalFact instead of ActiveIssue annotations
Added IsWasm property to PlatformDetection for test filtering
Restructured build pipelines for browser_wasm with separate Release and CoreCLR_ReleaseLibraries builds

Reviewed changes

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

Show a summary per file

File	Description
src/tests/run.sh, run.py, run.cmd	Added --node option for NodeJS test execution on WASM
src/tests/Common/CoreCLRTestLibrary/PlatformDetection.cs	Added IsWasm property combining IsBrowser and IsWasi
src/tests/Common/CLRTest.Execute.{Bash,Batch}.targets	Implemented NodeJS execution path with timeout support
src/tests/baseservices/threading/coverage/OSThreadId/OSThreadId.cs	Changed from ActiveIssue to ConditionalFact for threading support
src/tests/Loader/classloader/StaticVirtualMethods/*.il	Added ActiveIssue attributes for WASM platform
src/native/libs/System.Native.Browser/diagnostics/symbolicate.ts	Implemented symbol map parsing and stack trace symbolication
src/native/corehost/browserhost/loader/logging.ts	Refactored error() signature and added normalizeException()
src/native/corehost/browserhost/browserhost.cpp	Added BrowserHost_ShutdownCoreCLR with proper exit code handling
src/coreclr/vm/method.hpp	Added GetInterpreterCodeWithPrestub() helper method
src/coreclr/vm/wasm/calldescrworkerwasm.cpp	Removed prestub call (now called earlier via helper)
src/coreclr/vm/reflectioninvocation.cpp	Added prestub call before CallDescrWorkerInternal
src/coreclr/vm/callhelpers.cpp	Added prestub calls for PORTABLE_ENTRYPOINTS
src/coreclr/vm/dllimport.cpp	Added debug printf statements (issue)
src/tasks/WasmAppBuilder/coreclr/ManagedToNativeGenerator.cs	Added missing cookie signatures for interop
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/*.cs	Added EmitSymbolMap property support
eng/pipelines/runtime.yml	Restructured browser_wasm build configurations
eng/pipelines/coreclr/templates/*.yml	Added browser_wasm test job configurations

Copilot · 2026-02-17T18:26:32Z

src/coreclr/vm/dllimport.cpp

+            printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);
            return;
        }
    }

+    printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);


Debug printf statements should not be committed to production code. These should be replaced with proper logging or removed entirely.

Suggested change

printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);

return;

}

}

printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);

return;

}

}

Copilot · 2026-02-17T18:26:33Z

src/coreclr/vm/wasm/calldescrworkerwasm.cpp

    MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(pCallDescrData->pTarget);
    InterpByteCodeStart* targetIp = pMethod->GetInterpreterCode();
-    if (targetIp == NULL)
-    {
-        GCX_PREEMP();
-        (void)pMethod->DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);
-        targetIp = pMethod->GetInterpreterCode();
-    }
+    _ASSERTE(targetIp != nullptr);


The removal of the DoPrestub call and related GC comment changes the behavior significantly. The code now asserts that targetIp is not null instead of lazily calling DoPrestub when needed. This could cause crashes if GetInterpreterCode() returns nullptr. Ensure that all code paths guarantee the interpreter code is set before CallDescrWorkerInternal is called, or restore the prestub logic.

Copilot · 2026-02-17T18:26:33Z

src/coreclr/vm/reflectioninvocation.cpp

+    GCX_COOP();
+


There's a redundant GCX_COOP() call at line 375. The code is already in GCX_COOP() mode from line 336. This second call is unnecessary and should be removed.

Suggested change

GCX_COOP();

This reverts commit 771f7db.

Copilot

Pull request overview

Copilot reviewed 79 out of 80 changed files in this pull request and generated 5 comments.

Copilot · 2026-02-18T10:27:58Z

src/coreclr/vm/dllimport.cpp

            *ppMD = pMD;
+            printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);
            return;
        }
    }

+    printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);


Debug printf statement should not be committed to production code. This will output to stdout in all builds, not just debug builds. If logging is needed, use appropriate logging mechanisms or wrap in #ifdef _DEBUG.

Suggested change

*ppMD = pMD;

printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);

return;

}

}

printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);

*ppMD = pMD;

#ifdef _DEBUG

printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);

#endif // _DEBUG

return;

}

}

#ifdef _DEBUG

printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);

#endif // _DEBUG

Copilot · 2026-02-18T10:27:58Z

src/coreclr/vm/wasm/calldescrworkerwasm.cpp

-    // revisited to ensure correctness.
-
    MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(pCallDescrData->pTarget);
    InterpByteCodeStart* targetIp = pMethod->GetInterpreterCode();


Removing the DoPrestub call and related GC protection logic appears to assume that GetInterpreterCode() will always return non-null. The assertion at line 19 will crash if the method hasn't been prestubbed. The comment about GC protection being needed suggests this was a known issue. Verify that all callers now ensure prestubbing happens before calling CallDescrWorkerInternal, otherwise this introduces a regression where methods that haven't been jitted will crash instead of being lazily compiled.

Suggested change

InterpByteCodeStart* targetIp = pMethod->GetInterpreterCode();

InterpByteCodeStart* targetIp = pMethod->GetInterpreterCode();

if (targetIp == nullptr)

{

pMethod->DoPrestub(nullptr);

targetIp = pMethod->GetInterpreterCode();

}

Copilot · 2026-02-18T10:27:59Z

src/tests/Common/CLRTest.Execute.Bash.targets

+  (
+    sleep "$timeout_sec"
+    if [ $? -eq 0 ]; then
+      echo "Process $pid exceeded timeout of $timeout_sec seconds. Killing..."
+      if kill -0 "$pid" 2>/dev/null; then
+        kill -9 "$pid" 2>/dev/null
+      fi
+    fi


The timeout watchdog implementation has a logic issue. The condition if [ $? -eq 0 ] at line 346 always evaluates to true after sleep completes successfully, meaning the watchdog will always try to kill the process even if it completed normally. The watchdog should check if the process is still running before attempting to kill it. Consider using a different synchronization mechanism, such as checking if the PID is still alive inside the watchdog before killing, or using a signal-based approach where the main process signals the watchdog when it completes.

Copilot · 2026-02-18T10:27:59Z

src/native/libs/System.Native.Browser/diagnostics/symbolicate.ts

+    } catch (exc) {
+    }


Empty catch block silently swallows all exceptions during symbol map initialization. At minimum, log the exception using console.debug or console.error to aid debugging when symbol map parsing fails.

Copilot · 2026-02-18T10:27:59Z

src/coreclr/vm/dllimport.cpp

            *ppMD = pMD;
+            printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);
            return;
        }
    }

+    printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);


Debug printf statement should not be committed to production code. This will output to stdout in all builds, not just debug builds. If logging is needed, use appropriate logging mechanisms or wrap in #ifdef _DEBUG.

Suggested change

*ppMD = pMD;

printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);

return;

}

}

printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);

*ppMD = pMD;

#ifdef _DEBUG

printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);

#endif // _DEBUG

return;

}

}

#ifdef _DEBUG

printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);

#endif // _DEBUG

This reverts commit fa70e3f.

This reverts commit bb17c16.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/tests/JIT/Directed/Misc/function_pointer/MutualThdRecur-fptr.il:14

This IL file now declares Microsoft.DotNet.XUnitExtensions, System.Runtime, and TestLibrary twice in the extern list. Duplicate .assembly extern entries are easy to overlook and can break ilasm/build or cause confusing metadata. Remove the duplicate declarations and keep a single extern per assembly.

Copilot · 2026-02-18T12:58:43Z

src/tests/run.sh

+# Set default for RunWithNodeJS when using wasm architecture
+if [ "$buildArch" = "wasm" ] && [ -z "$RunWithNodeJS" ]; then
+    export RunWithNodeJS=1
+fi


RunWithNodeJS is defaulted on any buildArch=wasm, even when buildOS=wasi is explicitly selected. That will pass --node into run.py and set RunWithNodeJS=1 for WASI runs, but NodeJS is only applicable to browser/wasm execution. Consider gating the default to the browser scenario only (e.g., only when buildOS is unset or equals browser).

Copilot · 2026-02-18T12:58:43Z

src/coreclr/vm/dllimport.cpp

+            printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);
            return;
        }
    }

+    printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);


The new printf diagnostics in LookupUnmanagedCallersOnlyMethodByName will emit to stdout on every lookup, which can severely impact performance and pollute test output/logs. Please remove these prints or gate them behind an existing logging facility/conditional compilation (e.g., a debug-only or feature-specific trace macro).

Suggested change

printf("Found UCO method %s on type %s\n", methodName, fullQualifiedTypeName);

return;

}

}

printf("Did not find UCO method %s on type %s, looking for a method with the given name\n", methodName, fullQualifiedTypeName);

return;

}

}

radekdoulik added 30 commits January 15, 2026 19:04

Alternative implementation

1016c3c

[wasm][coreclr] Build and run runtime tests on node

237de71

Based on changes from the previous branch (https://github.com/radekdoulik/runtime/tree/clr-wasm-runtime-tests-with-node)

Conditinally disable tests with threads on wasm in ST scenario

c7217c1

Add missing signuters for runtime tests

7e44832

Build artifacts and try to run tests

769df50

Reduce CI to browser_wasm temporarily

32e6a77

Fix build_browser_wasm_linux_Release_CoreCLR_WasmBuildTests dependency

b5543f2

Fix build job suffix

7942bd0

Add hostedOs to the build dependency

37c67c9

Fix typo

7697993

Fix hostedOs location

3847f92

add jobParameters

d345451

Use hostedOs directly

d18ade8

Try to create the missing coreclr_common_test_build_p0_AnyOS_AnyCPU_c…

87d1662

…hecked job

Fix suffix

c4ad36d

Use checked runtime configuration

840fea1

Fix build name for cases wiht hostedOs set

4febea9

Put back linux_x64 for CoreClrTestBuildHost

85c5607

Put TestAssemblies=false back

683842e

[wasm][coreclr] Implement portable RhExceptionHandling_FailedAllocati…

b4c8999

…on (dotnet#123215) Fixes dotnet#122923

Merge remote-tracking branch 'remotes/rodo/clr-interp-fix-helper-call…

edf3697

…s' into clr-wasm-runtime-tests-on-node-2

Make IsCoreClrInterpreter return true on wasm

02a4b47

Feedback

01bd8c6

Prepare the runtime tests assets

ed88628

Run with node as default behavior

faeda88

Merge branch 'main' into clr-wasm-runtime-tests-on-node-2

b7553c2

Fix build after merge

d1da106

Add -os parameter

27041fc

Fix build

68c4b04

radekdoulik requested review from ilonatommy, lewing and pavelsavara as code owners February 17, 2026 18:19

github-actions bot added the area-Infrastructure-coreclr label Feb 17, 2026

github-project-automation bot added this to Runtime Infra Feb 17, 2026

radekdoulik marked this pull request as draft February 17, 2026 18:20

dotnet-policy-service bot assigned radekdoulik Feb 17, 2026

radekdoulik added the NO-REVIEW Experimental/testing PR, do NOT review it label Feb 17, 2026

Copilot started reviewing on behalf of radekdoulik February 17, 2026 18:20 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

radekdoulik added 3 commits February 18, 2026 10:35

Revert "Move prestub out of CallDescrWorkerInternal"

a069ddf

This reverts commit 771f7db.

Move prestub out of CallDescrWorkerInternal

bb17c16

Merge branch 'main' into clr-wasm-test-regression-2

a1e9f70

Copilot AI review requested due to automatic review settings February 18, 2026 10:22

Copilot started reviewing on behalf of radekdoulik February 18, 2026 10:23 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

radekdoulik added 3 commits February 18, 2026 12:53

Fix ifdefs

fa70e3f

Revert "Fix ifdefs"

9f1649a

This reverts commit fa70e3f.

Revert "Move prestub out of CallDescrWorkerInternal"

6633d8d

This reverts commit bb17c16.

Copilot AI review requested due to automatic review settings February 18, 2026 12:52

Copilot started reviewing on behalf of radekdoulik February 18, 2026 12:52 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

radekdoulik closed this Feb 20, 2026

github-project-automation bot moved this to Done in Runtime Infra Feb 20, 2026

Conversation

radekdoulik commented Feb 17, 2026

Uh oh!

dotnet-policy-service bot commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 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!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 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!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments