diff --git a/eng/pipelines/common/templates/wasm-aot-runtime-test-131537.yml b/eng/pipelines/common/templates/wasm-aot-runtime-test-131537.yml new file mode 100644 index 00000000000000..25c35d098b8f42 --- /dev/null +++ b/eng/pipelines/common/templates/wasm-aot-runtime-test-131537.yml @@ -0,0 +1,55 @@ +# TEMP (wasm-aot-131537-validation): validate that the WebAssembly functional test +# FunctionalTests/WebAssembly/Browser/AOT_131537 reproduces #131537 under the Mono +# wasm-AOT configuration (AOT'd corelib + force-interpreted test assembly). Scoped to +# just that one test via `-tree`, AOT'd locally, run on Chrome. Delete this file (and +# its reference in runtime-extra-platforms-wasm.yml) before merging anything real. + +parameters: + alwaysRun: false + isExtraPlatformsBuild: false + isWasmOnlyBuild: false + platforms: [] + extraBuildArgs: '' + useHelixMonitor: false + +jobs: + +- template: /eng/pipelines/common/platform-matrix.yml + parameters: + jobTemplate: /eng/pipelines/common/global-build-job.yml + helixQueuesTemplate: /eng/pipelines/coreclr/templates/helix-queues-setup.yml + buildConfig: Release + runtimeFlavor: mono + platforms: ${{ parameters.platforms }} + variables: + - name: alwaysRunVar + value: ${{ parameters.alwaysRun }} + - name: timeoutPerTestInMinutes + value: 20 + - name: timeoutPerTestCollectionInMinutes + value: 200 + jobParameters: + testGroup: innerloop + isExtraPlatforms: ${{ parameters.isExtraPlatformsBuild }} + nameSuffix: AllSubsets_Mono_RuntimeTests_AOT_131537 + # No runtimeVariant (i.e. not monointerpreter): we want corelib AOT'd, and + # the functional test csproj force-interprets only its own assembly via + # _AOT_InternalForceInterpretAssemblies — reproducing the failing config. + buildArgs: -s mono+libs -c $(_BuildConfig) /p:MonoEnableAssertMessages=true /p:InstallV8ForTests=false ${{ parameters.extraBuildArgs }} + timeoutInMinutes: 180 + condition: eq(variables['alwaysRunVar'], true) + postBuildSteps: + - template: /eng/pipelines/common/templates/runtimes/build-runtime-tests-and-send-to-helix.yml + parameters: + creator: dotnet-bot + testRunNamePrefixSuffix: Mono_$(_BuildConfig)_AOT_131537 + # Scope the runtime-test build to just this one functional test, and AOT it + # locally (the csproj sets RunAOTCompilation=true) so corelib is AOT'd in the + # produced wasm app. _BuildAllTestGroupsForBrowser: browser CI otherwise builds + # only test group 1 (#114123); safe here since only one test is scoped. + testBuildArgs: tree FunctionalTests/WebAssembly/Browser/AOT_131537 /p:RunAOTCompilation=true /p:InstallV8ForTests=false /p:_BuildAllTestGroupsForBrowser=true + scenarios: + - WasmTestOnChrome + useHelixMonitor: ${{ parameters.useHelixMonitor }} + extraVariablesTemplates: + - template: /eng/pipelines/common/templates/runtimes/test-variables.yml diff --git a/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml b/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml index 4ba21df5c78571..99a25081bd5a2e 100644 --- a/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml +++ b/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml @@ -249,6 +249,18 @@ jobs: isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }} useHelixMonitor: ${{ variables.enableHelixJobMonitor }} + # TEMP (wasm-aot-131537-validation): Mono AOT + Chrome functional-test lane scoped to + # FunctionalTests/WebAssembly/Browser/AOT_131537 only. Remove before merging. + - template: /eng/pipelines/common/templates/wasm-aot-runtime-test-131537.yml + parameters: + platforms: + - browser_wasm + extraBuildArgs: /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS) + isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }} + isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }} + alwaysRun: ${{ parameters.isWasmOnlyBuild }} + useHelixMonitor: ${{ variables.enableHelixJobMonitor }} + - ${{ if and(ne(parameters.isRollingBuild, true), ne(parameters.excludeOptional, true)) }}: - template: /eng/pipelines/common/templates/wasm-library-tests.yml parameters: diff --git a/src/mono/mono/metadata/jit-icall-reg.h b/src/mono/mono/metadata/jit-icall-reg.h index 1d159485bc63af..c58a320f89ab5b 100644 --- a/src/mono/mono/metadata/jit-icall-reg.h +++ b/src/mono/mono/metadata/jit-icall-reg.h @@ -203,7 +203,6 @@ MONO_JIT_ICALL (mono_get_native_calli_wrapper) \ MONO_JIT_ICALL (mono_get_special_static_data) \ MONO_JIT_ICALL (mono_gsharedvt_constrained_call) \ MONO_JIT_ICALL (mono_gsharedvt_value_copy) \ -MONO_JIT_ICALL (mono_helper_box_nullable) \ MONO_JIT_ICALL (mono_helper_compile_generic_method) \ MONO_JIT_ICALL (mono_helper_ldstr) \ MONO_JIT_ICALL (mono_helper_ldstr_mscorlib) \ diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index 4d9a95b8181227..fba533310bd2c7 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -1169,15 +1169,6 @@ mono_helper_newobj_mscorlib (guint32 idx) return obj; } -MonoObject* -mono_helper_box_nullable (gpointer vbuf, MonoClass *klass) -{ - ERROR_DECL (error); - MonoObject *result = mono_nullable_box (vbuf, klass, error); - mono_error_set_pending_exception (error); - return result; -} - /* * On some architectures, gdb doesn't like encountering the cpu breakpoint instructions * in generated code. So instead we emit a call to this function and place a gdb diff --git a/src/mono/mono/mini/jit-icalls.h b/src/mono/mono/mini/jit-icalls.h index 42b394b06b28a7..6e9ea58124e1e3 100644 --- a/src/mono/mono/mini/jit-icalls.h +++ b/src/mono/mono/mini/jit-icalls.h @@ -116,8 +116,6 @@ ICALL_EXPORT MonoString *mono_helper_ldstr_mscorlib (guint32 idx); ICALL_EXPORT MonoObject *mono_helper_newobj_mscorlib (guint32 idx); -ICALL_EXPORT MonoObject *mono_helper_box_nullable (gpointer vbuf, MonoClass *klass); - ICALL_EXPORT double mono_fsub (double a, double b); ICALL_EXPORT double mono_fadd (double a, double b); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index b7faaa15cf79cc..4545e6e0a21168 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -3403,26 +3403,6 @@ handle_alloc (MonoCompile *cfg, MonoClass *klass, gboolean for_box, int context_ return mono_emit_jit_icall_id (cfg, alloc_ftn, iargs); } -/* - * Box a gsharedvt Nullable via a non-generic runtime helper, passing the value by - * address and the concrete class from the rgctx. This avoids a per-T box wrapper that - * cannot be emitted at AOT time when the consuming assembly runs interpreted. - */ -static MonoInst* -mini_emit_nullable_box_helper (MonoCompile *cfg, MonoInst *val, MonoClass *klass, int context_used) -{ - MonoInst *iargs [2], *addr, *var; - - var = get_vreg_to_inst (cfg, val->dreg); - if (!var) - var = mono_compile_create_var_for_vreg (cfg, m_class_get_byval_arg (klass), OP_LOCAL, val->dreg); - EMIT_NEW_VARLOADA (cfg, addr, var, var->inst_vtype); - - iargs [0] = addr; - iargs [1] = mini_emit_get_rgctx_klass (cfg, context_used, klass, MONO_RGCTX_INFO_KLASS); - return mono_emit_jit_icall (cfg, mono_helper_box_nullable, iargs); -} - /* * Returns NULL and set the cfg exception on error. */ @@ -3445,9 +3425,11 @@ mini_emit_box (MonoCompile *cfg, MonoInst *val, MonoClass *klass, int context_us MonoInst *addr; MonoMethodSignature *sig = mono_method_signature_internal (method); if (mini_is_gsharedvt_klass (klass)) - return mini_emit_nullable_box_helper (cfg, val, klass, context_used); - addr = emit_get_rgctx_method (cfg, context_used, method, - MONO_RGCTX_INFO_METHOD_FTNDESC); + addr = mini_emit_get_gsharedvt_info_klass (cfg, klass, + MONO_RGCTX_INFO_NULLABLE_CLASS_BOX); + else + addr = emit_get_rgctx_method (cfg, context_used, method, + MONO_RGCTX_INFO_METHOD_FTNDESC); cfg->interp_in_signatures = g_slist_prepend_mempool (cfg->mempool, cfg->interp_in_signatures, sig); return mini_emit_llvmonly_calli (cfg, sig, &val, addr); } else { @@ -3513,14 +3495,9 @@ mini_emit_box (MonoCompile *cfg, MonoInst *val, MonoClass *klass, int context_us /* Nullable case */ MONO_START_BB (cfg, is_nullable_bb); - if (cfg->llvm_only) { - MonoInst *box_call = mini_emit_nullable_box_helper (cfg, val, klass, context_used); - EMIT_NEW_UNALU (cfg, res, OP_MOVE, dreg, box_call->dreg); - res->type = STACK_OBJ; - res->klass = klass; - } else { + { MonoInst *box_addr = mini_emit_get_gsharedvt_info_klass (cfg, klass, - MONO_RGCTX_INFO_NULLABLE_CLASS_BOX); + MONO_RGCTX_INFO_NULLABLE_CLASS_BOX); MonoInst *box_call; MonoMethodSignature *box_sig; @@ -3533,7 +3510,10 @@ mini_emit_box (MonoCompile *cfg, MonoInst *val, MonoClass *klass, int context_us box_sig->param_count = 1; box_sig->params [0] = m_class_get_byval_arg (klass); - box_call = mini_emit_calli (cfg, box_sig, &val, box_addr, NULL, NULL); + if (cfg->llvm_only) + box_call = mini_emit_llvmonly_calli (cfg, box_sig, &val, box_addr); + else + box_call = mini_emit_calli (cfg, box_sig, &val, box_addr, NULL, NULL); EMIT_NEW_UNALU (cfg, res, OP_MOVE, dreg, box_call->dreg); res->type = STACK_OBJ; res->klass = klass; diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 6a2099535cbddd..46e28a680b3cc7 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -5114,7 +5114,6 @@ register_icalls (void) register_icall (mono_helper_ldstr, mono_icall_sig_object_ptr_int, FALSE); register_icall (mono_helper_ldstr_mscorlib, mono_icall_sig_object_int, FALSE); register_icall (mono_helper_newobj_mscorlib, mono_icall_sig_object_int, FALSE); - register_icall (mono_helper_box_nullable, mono_icall_sig_object_ptr_ptr, FALSE); register_icall (mono_value_copy_internal, mono_icall_sig_void_ptr_ptr_ptr, FALSE); register_icall (mono_object_castclass_unbox, mono_icall_sig_object_object_ptr, FALSE); register_icall (mono_break, mono_icall_sig_void, TRUE); diff --git a/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/Program.cs b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/Program.cs new file mode 100644 index 00000000000000..43c2c91a2f8802 --- /dev/null +++ b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/Program.cs @@ -0,0 +1,69 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Regression test for https://github.com/dotnet/runtime/issues/131537 +// (WASM manifestation of https://github.com/dotnet/runtime/issues/66220). +// +// Constructing a non-generic Queue from a List> enumerates the list +// through the non-generic IEnumerator, whose Current getter boxes each element +// Nullable -> object. With corelib AOT'd and this assembly running in the +// interpreter (see _AOT_InternalForceInterpretAssemblies in the csproj), the +// concrete Nullable is instantiated only in the interpreter, so the AOT +// compiler never emits the required gsharedvt out-sig wrapper object(Nullable) +// and the boxing call traps with "function signature mismatch" (assertion in +// mini-generic-sharing.c). The 16-byte payload types (Int128?/Guid?/decimal?) +// are the ones that regress. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Runtime.InteropServices.JavaScript; + +namespace Sample +{ + public partial class Test + { + public static void Main() + { + Console.WriteLine("Test_131537 loaded"); + } + + [JSExport] + public static int TestMeaning() + { + RoundTrip(new List { 1, 2, 3 }); + RoundTrip(new List { 1, 2, 3 }); + RoundTrip(new List { (Half)1, (Half)2, (Half)3 }); + RoundTrip(new List { 1m, 2m, 3m }); + RoundTrip(new List { Guid.Empty, Guid.NewGuid() }); + RoundTrip(new List { DateTime.UnixEpoch, DateTime.MaxValue }); + RoundTrip(new List { DateTimeOffset.UnixEpoch, DateTimeOffset.MaxValue }); + RoundTrip(new List { TimeSpan.Zero, TimeSpan.FromSeconds(5) }); + + // 42 == pass, per the WebAssembly functional test convention. + return 42; + } + + private static void RoundTrip(List source) + { + // Queue(ICollection) enumerates via the non-generic IEnumerator, boxing + // each element T -> object. This is the call that crashes when the + // object(Nullable) gsharedvt out-sig wrapper is missing. + var queue = new Queue(source); + if (queue.Count != source.Count) + { + throw new Exception($"count mismatch: {queue.Count} != {source.Count}"); + } + + int i = 0; + foreach (object boxed in queue) + { + if (!Equals(source[i], (T)boxed)) + { + throw new Exception($"element {i} mismatch"); + } + i++; + } + } + } +} diff --git a/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/WebAssembly.Browser.Aot_131537.Test.csproj b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/WebAssembly.Browser.Aot_131537.Test.csproj new file mode 100644 index 00000000000000..123ae874f442f4 --- /dev/null +++ b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/WebAssembly.Browser.Aot_131537.Test.csproj @@ -0,0 +1,20 @@ + + + + true + false + 42 + + + + + + + + <_AOT_InternalForceInterpretAssemblies Include="WebAssembly.Browser.Aot_131537.Test.dll" /> + + diff --git a/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/index.html b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/index.html new file mode 100644 index 00000000000000..d6311e51b3a785 --- /dev/null +++ b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/index.html @@ -0,0 +1,19 @@ + + + + + + + 131537 AOT nullable box test + + + + + + + + + Result: + + + diff --git a/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/main.js b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/main.js new file mode 100644 index 00000000000000..6f7b42c68d34ff --- /dev/null +++ b/src/tests/FunctionalTests/WebAssembly/Browser/AOT_131537/main.js @@ -0,0 +1,22 @@ +import { dotnet } from './_framework/dotnet.js' + +function wasm_exit(exit_code) { + var tests_done_elem = document.createElement("label"); + tests_done_elem.id = "tests_done"; + tests_done_elem.innerHTML = exit_code.toString(); + document.body.appendChild(tests_done_elem); + + console.log(`WASM EXIT ${exit_code}`); +} + +try { + const { getAssemblyExports } = await dotnet.create(); + const exports = await getAssemblyExports("WebAssembly.Browser.Aot_131537.Test.dll"); + const ret = exports.Sample.Test.TestMeaning(); + document.getElementById("out").innerHTML = `${ret}`; + console.debug(`ret: ${ret}`); + wasm_exit(ret); +} catch (err) { + console.log(`WASM ERROR ${err}`); + wasm_exit(1); +} diff --git a/src/tests/build.proj b/src/tests/build.proj index 884ccfcddb0f79..21960e2433cb3a 100644 --- a/src/tests/build.proj +++ b/src/tests/build.proj @@ -205,9 +205,10 @@