Exception Handler support (#78773)

* Update EH tests to run with runtime async

* Handle non-null exception filter prologues in the spill sequencer

* Add more testing to show current incorrect behavior

* Unskip ConditionalFacts that do not need to be skipped.

* Handle ensuring that the method remains valid, even when there is an `await` in a finally section. Add signifcant testing of `await using`.

* Fix baselines

* Support `await foreach` and add runtime async verification to existing tests.

* Remove unnecessary generic

* Failing tests, add async void test suggestion

* CI failures

* Add additional test

* Test fixes

* Remove implemented PROTOTYPE, add assertion on behavior.

* Update to SpillSequenceSpiller after some more debugging and tightening the assertion

* Fix nullref

* Enable nullable for VisitCatchBlock

Author: 333fred

src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs

@@ -263,7 +263,7 @@ private BoundForEachStatement BindForEachPartsWorker(BindingDiagnosticBag diagno
                 var placeholder = new BoundAwaitableValuePlaceholder(expr, builder.MoveNextInfo?.Method.ReturnType ?? CreateErrorType());
                 awaitInfo = BindAwaitInfo(placeholder, expr, diagnostics, ref hasErrors);
 
-                if (!hasErrors && awaitInfo.GetResult?.ReturnType.SpecialType != SpecialType.System_Boolean)
+                if (!hasErrors && (awaitInfo.GetResult ?? awaitInfo.RuntimeAsyncAwaitMethod)?.ReturnType.SpecialType != SpecialType.System_Boolean)
                 {
                     diagnostics.Add(ErrorCode.ERR_BadGetAsyncEnumerator, expr.Location, getEnumeratorMethod.ReturnTypeWithAnnotations, getEnumeratorMethod);
                     hasErrors = true;

src/Compilers/CSharp/Portable/BoundTree/BoundAwaitableInfo.cs

@@ -34,5 +34,7 @@ private partial void Validate()
                     break;
             }
         }
+
+        Debug.Assert(GetAwaiter is not null || RuntimeAsyncAwaitMethod is not null || IsDynamic || HasErrors);
     }
 }

src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs

@@ -321,7 +321,10 @@ private void HandleReturn()
         {
             _builder.MarkLabel(s_returnLabel);
 
-            Debug.Assert(_method.ReturnsVoid == (_returnTemp == null));
+            Debug.Assert(_method.ReturnsVoid == (_returnTemp == null)
+                || (_method.IsAsync
+                    && _module.Compilation.IsRuntimeAsyncEnabledIn(_method)
+                    && ((InternalSpecialType)_method.ReturnType.ExtendedSpecialType) is InternalSpecialType.System_Threading_Tasks_Task or InternalSpecialType.System_Threading_Tasks_ValueTask));
 
             if (_emitPdbSequencePoints && !_method.IsIterator && !_method.IsAsync)
             {

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncExceptionHandlerRewriter.cs

@@ -28,6 +28,7 @@ internal sealed class AsyncExceptionHandlerRewriter : BoundTreeRewriterWithStack
         private AwaitCatchFrame _currentAwaitCatchFrame;
         private AwaitFinallyFrame _currentAwaitFinallyFrame = new AwaitFinallyFrame();
         private bool _inCatchWithoutAwaits;
+        private bool _needsFinalThrow;
 
         private AsyncExceptionHandlerRewriter(
             MethodSymbol containingMethod,
@@ -129,9 +130,45 @@ public static BoundStatement Rewrite(
             var rewriter = new AsyncExceptionHandlerRewriter(containingSymbol, containingType, factory, analysis);
             var loweredStatement = (BoundStatement)rewriter.Visit(statement);
 
+            loweredStatement = rewriter.FinalizeMethodBody(loweredStatement);
+
             return loweredStatement;
         }
 
+        private BoundStatement FinalizeMethodBody(BoundStatement loweredStatement)
+        {
+            if (loweredStatement == null)
+            {
+                return null;
+            }
+
+            // When we add a `switch (pendingBranch)` to the end of the try block,
+            // this can result in a method body that cannot be proven to terminate.
+            // While we can technically prove it by doing a full data flow analysis,
+            // this is effectively the halting problem, and the runtime will not do
+            // this analysis. The resulting IL will be technically invalid, and if it's
+            // not wrapped in another state machine (a la the compiler async rewriter),
+            // the runtime will refuse to load it. For runtime async, where we are effectively
+            // emitting the result of this rewriter directly, we need to ensure that
+            // we always emit a throw at the end of the try block when the switch is present.
+            // This ensures that the method can be proven to terminate, and the runtime will
+            // accept it. This throw will never be reached, and we could potentially do a
+            // more sophisticated analysis to determine if it is needed by pushing control
+            // flow analysis through the bound nodes, see https://github.com/dotnet/roslyn/pull/78970.
+            // This is risky, however, and for now we are taking the conservative approach
+            // of always emitting the throw.
+            BoundStatement result = loweredStatement;
+            if (_needsFinalThrow)
+            {
+                result = _F.Block(
+                    loweredStatement,
+                    _F.Throw(_F.Null(_F.SpecialType(SpecialType.System_Object)))
+                );
+            }
+
+            return result;
+        }
+
         public override BoundNode VisitTryStatement(BoundTryStatement node)
         {
             var tryStatementSyntax = node.Syntax;
@@ -354,6 +391,7 @@ private BoundStatement UnpendBranches(
                 cases.Add(caseStatement);
             }
 
+            _needsFinalThrow = true;
             return _F.Switch(_F.Local(pendingBranchVar), cases.ToImmutableAndFree());
         }
 
@@ -402,22 +440,37 @@ public override BoundNode VisitReturnStatement(BoundReturnStatement node)
 
         private BoundStatement UnpendException(LocalSymbol pendingExceptionLocal)
         {
+            // If this is runtime async, we don't need to create a second local for the exception,
+            // as the pendingExceptionLocal will not be hoisted to a state machine by a future rewrite.
+            if (_F.Compilation.IsRuntimeAsyncEnabledIn(_F.CurrentFunction))
+            {
+                // pendingExceptionLocal is already an object
+                // so we can just use it directly
+                return checkAndThrow(pendingExceptionLocal);
+            }
+
             // create a temp. 
             // pendingExceptionLocal will certainly be captured, no need to access it over and over.
             LocalSymbol obj = _F.SynthesizedLocal(_F.SpecialType(SpecialType.System_Object));
             var objInit = _F.Assignment(_F.Local(obj), _F.Local(pendingExceptionLocal));
 
             // throw pendingExceptionLocal;
-            BoundStatement rethrow = Rethrow(obj);
-
             return _F.Block(
                     ImmutableArray.Create<LocalSymbol>(obj),
                     objInit,
-                    _F.If(
-                        _F.ObjectNotEqual(
-                            _F.Local(obj),
-                            _F.Null(obj.Type)),
-                        rethrow));
+                    checkAndThrow(obj));
+
+            BoundStatement checkAndThrow(LocalSymbol obj)
+            {
+                BoundStatement rethrow = Rethrow(obj);
+
+                BoundStatement checkAndThrow = _F.If(
+                            _F.ObjectNotEqual(
+                                _F.Local(obj),
+                                _F.Null(obj.Type)),
+                            rethrow);
+                return checkAndThrow;
+            }
         }
 
         private BoundStatement Rethrow(LocalSymbol obj)
@@ -706,14 +759,24 @@ public override BoundNode VisitLambda(BoundLambda node)
         {
             var oldContainingSymbol = _F.CurrentFunction;
             var oldAwaitFinallyFrame = _currentAwaitFinallyFrame;
+            var oldNeedsFinalThrow = _needsFinalThrow;
 
             _F.CurrentFunction = node.Symbol;
             _currentAwaitFinallyFrame = new AwaitFinallyFrame();
+            _needsFinalThrow = false;
 
-            var result = base.VisitLambda(node);
+            var result = (BoundLambda)base.VisitLambda(node);
+            result = result.Update(
+                result.UnboundLambda,
+                result.Symbol,
+                (BoundBlock)FinalizeMethodBody(result.Body),
+                node.Diagnostics,
+                node.Binder,
+                node.Type);
 
             _F.CurrentFunction = oldContainingSymbol;
             _currentAwaitFinallyFrame = oldAwaitFinallyFrame;
+            _needsFinalThrow = oldNeedsFinalThrow;
 
             return result;
         }
@@ -722,14 +785,18 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen
         {
             var oldContainingSymbol = _F.CurrentFunction;
             var oldAwaitFinallyFrame = _currentAwaitFinallyFrame;
+            var oldNeedsFinalThrow = _needsFinalThrow;
 
             _F.CurrentFunction = node.Symbol;
             _currentAwaitFinallyFrame = new AwaitFinallyFrame();
+            _needsFinalThrow = false;
 
-            var result = base.VisitLocalFunctionStatement(node);
+            var result = (BoundLocalFunctionStatement)base.VisitLocalFunctionStatement(node);
+            result = result.Update(node.Symbol, (BoundBlock)FinalizeMethodBody(result.Body), (BoundBlock)FinalizeMethodBody(result.ExpressionBody));
 
             _F.CurrentFunction = oldContainingSymbol;
             _currentAwaitFinallyFrame = oldAwaitFinallyFrame;
+            _needsFinalThrow = oldNeedsFinalThrow;
 
             return result;
         }

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/RuntimeAsyncRewriter.cs

@@ -22,20 +22,17 @@ public static BoundStatement Rewrite(
             return node;
         }
 
-        // PROTOTYPE: try/finally rewriting
         // PROTOTYPE: struct lifting
-        var rewriter = new RuntimeAsyncRewriter(compilationState.Compilation, new SyntheticBoundNodeFactory(method, node.Syntax, compilationState, diagnostics));
+        var rewriter = new RuntimeAsyncRewriter(new SyntheticBoundNodeFactory(method, node.Syntax, compilationState, diagnostics));
         var result = (BoundStatement)rewriter.Visit(node);
         return SpillSequenceSpiller.Rewrite(result, method, compilationState, diagnostics);
     }
 
-    private readonly CSharpCompilation _compilation;
     private readonly SyntheticBoundNodeFactory _factory;
     private readonly Dictionary<BoundAwaitableValuePlaceholder, BoundExpression> _placeholderMap;
 
-    private RuntimeAsyncRewriter(CSharpCompilation compilation, SyntheticBoundNodeFactory factory)
+    private RuntimeAsyncRewriter(SyntheticBoundNodeFactory factory)
     {
-        _compilation = compilation;
         _factory = factory;
         _placeholderMap = [];
     }

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ForEachStatement.cs

@@ -223,14 +223,14 @@ private BoundStatement RewriteForEachEnumerator(
             var disposalFinallyBlock = GetDisposalFinallyBlock(forEachSyntax, enumeratorInfo, enumeratorType, boundEnumeratorVar, out var hasAsyncDisposal);
             if (isAsync)
             {
-                Debug.Assert(awaitableInfo is { GetResult: { } });
+                Debug.Assert(awaitableInfo is { GetResult: not null } or { RuntimeAsyncAwaitMethod: not null });
 
                 // We need to be sure that when the disposal isn't async we reserve an unused state machine state number for it,
                 // so that await foreach always produces 2 state machine states: one for MoveNextAsync and the other for DisposeAsync.
                 // Otherwise, EnC wouldn't be able to map states when the disposal changes from having async dispose to not, or vice versa.
                 var debugInfo = new BoundAwaitExpressionDebugInfo(s_moveNextAsyncAwaitId, ReservedStateMachineCount: (byte)(hasAsyncDisposal ? 0 : 1));
 
-                rewrittenCondition = RewriteAwaitExpression(forEachSyntax, rewrittenCondition, awaitableInfo, awaitableInfo.GetResult.ReturnType, debugInfo, used: true);
+                rewrittenCondition = RewriteAwaitExpression(forEachSyntax, rewrittenCondition, awaitableInfo, (awaitableInfo.GetResult ?? awaitableInfo.RuntimeAsyncAwaitMethod)!.ReturnType, debugInfo, used: true);
             }
 
             BoundStatement whileLoop = RewriteWhileStatement(

src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs

@@ -704,15 +704,24 @@ public override BoundNode VisitYieldReturnStatement(BoundYieldReturnStatement no
             return UpdateStatement(builder, node.Update(expression));
         }
 
+#nullable enable
         public override BoundNode VisitCatchBlock(BoundCatchBlock node)
         {
-            BoundExpression exceptionSourceOpt = (BoundExpression)this.Visit(node.ExceptionSourceOpt);
+            BoundExpression? exceptionSourceOpt = (BoundExpression?)this.Visit(node.ExceptionSourceOpt);
             var locals = node.Locals;
 
             var exceptionFilterPrologueOpt = node.ExceptionFilterPrologueOpt;
-            Debug.Assert(exceptionFilterPrologueOpt is null); // it is introduced by this pass
-            BoundSpillSequenceBuilder builder = null;
+            if (exceptionFilterPrologueOpt is not null)
+            {
+                exceptionFilterPrologueOpt = (BoundStatementList?)VisitStatementList(exceptionFilterPrologueOpt);
+            }
+            BoundSpillSequenceBuilder? builder = null;
+
             var exceptionFilterOpt = VisitExpression(ref builder, node.ExceptionFilterOpt);
+            Debug.Assert(exceptionFilterPrologueOpt is null || builder is null,
+                "You are exercising SpillSequenceSpiller in a new fashion, causing a spill in an exception filter after LocalRewriting is complete. This is not someting " +
+                "that this builder supports today, so please update this rewrite to include the statements from exceptionFilterPrologueOpt with the appropriate " +
+                "syntax node and tracking.");
             if (builder is { })
             {
                 Debug.Assert(builder.Value is null);
@@ -721,9 +730,10 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock node)
             }
 
             BoundBlock body = (BoundBlock)this.Visit(node.Body);
-            TypeSymbol exceptionTypeOpt = this.VisitType(node.ExceptionTypeOpt);
+            TypeSymbol? exceptionTypeOpt = this.VisitType(node.ExceptionTypeOpt);
             return node.Update(locals, exceptionSourceOpt, exceptionTypeOpt, exceptionFilterPrologueOpt, exceptionFilterOpt, body, node.IsSynthesizedAsyncCatchAll);
         }
+#nullable disable
 
 #if DEBUG
         public override BoundNode DefaultVisit(BoundNode node)