diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index a9b6c4d8dd1fd3..ef391d68ecddda 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -414,7 +414,7 @@ bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const // CanRemoveJumpToTarget: determine if jump to target can be omitted // // Arguments: -// target - target of the BBJ_ALWAYS or true/false target of the BBJ_COND block +// target - target of the BBJ_ALWAYS/BBJ_CALLFINALLYRET or true/false target of the BBJ_COND block // compiler - current compiler instance // // Returns: @@ -422,7 +422,7 @@ bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const // bool BasicBlock::CanRemoveJumpToTarget(BasicBlock* target, Compiler* compiler) const { - assert((KindIs(BBJ_ALWAYS) && (GetTarget() == target)) || + assert((KindIs(BBJ_ALWAYS, BBJ_CALLFINALLYRET) && (GetTarget() == target)) || (KindIs(BBJ_COND) && (TrueTargetIs(target) || FalseTargetIs(target)))); if (!NextIs(target) || IsLastHotBlock(compiler)) { diff --git a/src/coreclr/jit/codegenwasm.cpp b/src/coreclr/jit/codegenwasm.cpp index 972141db58229f..d7236f10a87701 100644 --- a/src/coreclr/jit/codegenwasm.cpp +++ b/src/coreclr/jit/codegenwasm.cpp @@ -4140,13 +4140,13 @@ void CodeGen::genCallFinally(BasicBlock* block) return; } - // Branch to the continuation block if it's not the next block. + // Branch to the continuation block unless we can fall into it. assert(block->isBBCallFinallyPair()); BasicBlock* const callFinallyRet = block->Next(); assert(callFinallyRet->KindIs(BBJ_CALLFINALLYRET)); BasicBlock* const continuation = callFinallyRet->GetTarget(); - if (continuation != callFinallyRet->Next()) + if (!callFinallyRet->CanRemoveJumpToTarget(continuation, m_compiler)) { inst_JMP(EJ_jmp, continuation); } diff --git a/src/coreclr/jit/fgwasm.cpp b/src/coreclr/jit/fgwasm.cpp index 9cdc694c28421e..7311ffcf047c90 100644 --- a/src/coreclr/jit/fgwasm.cpp +++ b/src/coreclr/jit/fgwasm.cpp @@ -2022,6 +2022,61 @@ PhaseStatus Compiler::fgWasmControlFlow() } } } + + // Verify that an adjacent forward edge that cannot fall through, because a Try or + // ExnRefWrapper interval ends at its target (see BasicBlock::CanRemoveJumpToTarget), + // has a plain Block interval enclosing the source and ending at the target, so the + // explicit branch codegen emits has a label to bind to. For example, a callfinally + // whose continuation follows the end of a try_table: + // + // block ;; Block interval ending at the continuation + // try_table ... + // ... + // call_indirect ;; call the finally + // br 1 ;; branch to the continuation + // end ;; end of try_table + // unreachable ;; fall-through from the try_table would trap here + // end + // ... ;; continuation + // + for (unsigned cursor = 0; cursor < numBlocks; cursor++) + { + BasicBlock* const block = initialLayout[cursor]; + BasicBlock* const next = initialLayout[cursor + 1]; + + bool const fallsToNext = (block->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLYRET) && block->TargetIs(next)) || + (block->KindIs(BBJ_COND) && block->FalseTargetIs(next)); + if (!fallsToNext) + { + continue; + } + + bool endsTryOrWrapper = false; + bool hasBlockTarget = false; + for (WasmInterval* const interval : *fgWasmIntervals) + { + if (interval->End() != (cursor + 1)) + { + continue; + } + + if (interval->IsTry() || interval->IsExnRefWrapper()) + { + endsTryOrWrapper = true; + } + else if (!interval->IsLoop() && (interval->Start() <= cursor)) + { + hasBlockTarget = true; + } + } + + if (endsTryOrWrapper && !hasBlockTarget) + { + JITDUMP(FMT_BB "[%u] -> " FMT_BB "[%u] crosses a Try/ExnRefWrapper end without a Block target\n", + block->bbNum, cursor, next->bbNum, cursor + 1); + assert(!"Wasm fall-through across a Try/ExnRefWrapper end needs a Block target"); + } + } #endif // ----------------------------------------------- diff --git a/src/libraries/System.Net.WebSockets/tests/WebSocketReceiveErrorMessageTests.cs b/src/libraries/System.Net.WebSockets/tests/WebSocketReceiveErrorMessageTests.cs index e23f0e215bf55a..f8d452aa9f3d25 100644 --- a/src/libraries/System.Net.WebSockets/tests/WebSocketReceiveErrorMessageTests.cs +++ b/src/libraries/System.Net.WebSockets/tests/WebSocketReceiveErrorMessageTests.cs @@ -9,7 +9,6 @@ namespace System.Net.WebSockets.Tests { - [ActiveIssue("https://github.com/dotnet/runtime/issues/134264", typeof(PlatformDetection), nameof(PlatformDetection.IsWasmReadyToRun))] public class WebSocketReceiveErrorMessageTests { // The test WebSocket is created with isServer:false, so received frames must NOT be masked diff --git a/src/tests/JIT/Regression_2/Runtime_134264/Runtime_134264.cs b/src/tests/JIT/Regression_2/Runtime_134264/Runtime_134264.cs new file mode 100644 index 00000000000000..34777da01d5313 --- /dev/null +++ b/src/tests/JIT/Regression_2/Runtime_134264/Runtime_134264.cs @@ -0,0 +1,105 @@ +// 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 a WebAssembly R2R (crossgen) codegen bug in finally +// calls. When a callfinally's continuation was the lexically next block, +// codegen fell through to it without a branch. If a try_table ended between +// the two, the fall-through landed on the validation `unreachable` emitted +// after the try_table `end` and trapped (RuntimeError: unreachable). +// +// The shape needs a user try/finally that ends the async method (so the +// continuation is outside the state machine's try/catch) and a finally too +// large to be cloned onto the normal path. +// +// Reproduces only under crossgen wasm R2R (TargetOS=browser). Passes trivially +// on all other targets. + +using System; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Xunit; + +public class Runtime_134264 +{ + private static int s_value; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Touch(int value) => s_value += value; + + private static async Task TryCatchFinallyAsync(bool enter) + { + bool entered = false; + try + { + if (enter) + { + await Task.CompletedTask; + entered = true; + } + + Touch(1); + } + catch (Exception ex) + { + GC.KeepAlive(ex); + Touch(-100); + } + finally + { + if (entered) + { + Touch(1); Touch(2); Touch(3); Touch(4); Touch(5); Touch(6); Touch(7); Touch(8); + Touch(9); Touch(10); Touch(11); Touch(12); Touch(13); Touch(14); Touch(15); Touch(16); + } + + Touch(1000); + } + } + + private static async Task NestedTryFinallyAsync(bool enter) + { + bool entered = false; + try + { + try + { + if (enter) + { + await Task.CompletedTask; + entered = true; + } + + Touch(1); + } + finally + { + if (entered) + { + Touch(1); Touch(2); Touch(3); Touch(4); Touch(5); Touch(6); Touch(7); Touch(8); + Touch(9); Touch(10); Touch(11); Touch(12); Touch(13); Touch(14); Touch(15); Touch(16); + } + + Touch(1000); + } + } + catch (Exception ex) + { + GC.KeepAlive(ex); + Touch(-100); + } + } + + [Theory] + [InlineData(true, 1137)] + [InlineData(false, 1001)] + public static async Task TestEntryPoint(bool enter, int expected) + { + s_value = 0; + await TryCatchFinallyAsync(enter); + Assert.Equal(expected, s_value); + + s_value = 0; + await NestedTryFinallyAsync(enter); + Assert.Equal(expected, s_value); + } +} diff --git a/src/tests/JIT/Regression_2/Runtime_134264/Runtime_134264.csproj b/src/tests/JIT/Regression_2/Runtime_134264/Runtime_134264.csproj new file mode 100644 index 00000000000000..de6d5e08882e86 --- /dev/null +++ b/src/tests/JIT/Regression_2/Runtime_134264/Runtime_134264.csproj @@ -0,0 +1,8 @@ + + + True + + + + +