Merge pull request #42807 from YairHalberstadt/fix-ltr-semantics-async

dotnet · Apr 17, 2020 · b18cb03 · b18cb03
2 parents 0c847a1 + f5bc26d
commit b18cb03
Show file tree

Hide file tree

Showing 6 changed files with 4,861 additions and 187 deletions.
diff --git a/src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs b/src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs
@@ -677,17 +677,15 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
                         if (field.FieldSymbol.IsStatic) break;
 
                         // instance fields are directly assignable, but receiver is pushed, so need to spill that.
-                        var receiver = VisitExpression(ref leftBuilder, field.ReceiverOpt);
-                        receiver = Spill(builder, receiver, field.FieldSymbol.ContainingType.IsValueType ? RefKind.Ref : RefKind.None);
-                        left = field.Update(receiver, field.FieldSymbol, field.ConstantValueOpt, field.ResultKind, field.Type);
+                        left = fieldWithSpilledReceiver(field, ref leftBuilder, isAssignmentTarget: true);
                         break;
 
                     case BoundKind.ArrayAccess:
                         var arrayAccess = (BoundArrayAccess)left;
                         // array and indices are pushed on stack so need to spill that
                         var expression = VisitExpression(ref leftBuilder, arrayAccess.Expression);
-                        expression = Spill(builder, expression, RefKind.None);
-                        var indices = this.VisitExpressionList(ref builder, arrayAccess.Indices, forceSpill: true);
+                        expression = Spill(leftBuilder, expression, RefKind.None);
+                        var indices = this.VisitExpressionList(ref leftBuilder, arrayAccess.Indices, forceSpill: true);
                         left = arrayAccess.Update(expression, indices, arrayAccess.Type);
                         break;
 
@@ -710,6 +708,53 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
             }
 
             return UpdateExpression(builder, node.Update(left, right, node.IsRef, node.Type));
+
+            BoundExpression fieldWithSpilledReceiver(BoundFieldAccess field, ref BoundSpillSequenceBuilder leftBuilder, bool isAssignmentTarget)
+            {
+                BoundExpression receiver;
+                var generateDummyFieldAccess = false;
+                if (field.FieldSymbol.ContainingType.IsReferenceType)
+                {
+                    // a reference type can always live across await so Spill using leftBuilder
+                    receiver = Spill(leftBuilder, VisitExpression(ref leftBuilder, field.ReceiverOpt));
+
+                    // dummy field access to trigger NRE
+                    // a.b = c will trigger a NRE if a is null on assignment,
+                    // but a.b.c = d will trigger a NRE if a is null before evaluating d
+                    // so check whether we assign to the field directly
+                    generateDummyFieldAccess = !isAssignmentTarget;
+                }
+                else if (field.ReceiverOpt is BoundArrayAccess arrayAccess)
+                {
+                    // an arrayAccess returns a ref so can only be called after the await, but spill expression and indices
+                    var expression = VisitExpression(ref leftBuilder, arrayAccess.Expression);
+                    expression = Spill(leftBuilder, expression, RefKind.None);
+                    var indices = this.VisitExpressionList(ref leftBuilder, arrayAccess.Indices, forceSpill: true);
+                    receiver = arrayAccess.Update(expression, indices, arrayAccess.Type);
+                    // dummy array access to trigger IndexOutRangeException or NRE
+                    // we only need this if the array access is a receiver since
+                    // a[0] = b triggers a NRE/IORE on assignment
+                    // but a[0].b = c triggers an NRE/IORE before evaluating c
+                    Spill(leftBuilder, receiver, sideEffectsOnly: true);
+                }
+                else if (field.ReceiverOpt is BoundFieldAccess receiverField)
+                {
+                    receiver = fieldWithSpilledReceiver(receiverField, ref leftBuilder, isAssignmentTarget: false);
+                }
+                else
+                {
+                    receiver = Spill(leftBuilder, VisitExpression(ref leftBuilder, field.ReceiverOpt), RefKind.Ref);
+                }
+
+                field = field.Update(receiver, field.FieldSymbol, field.ConstantValueOpt, field.ResultKind, field.Type);
+
+                if (generateDummyFieldAccess)
+                {
+                    Spill(leftBuilder, field, sideEffectsOnly: true);
+                }
+
+                return field;
+            }
         }
 
         public override BoundNode VisitBadExpression(BoundBadExpression node)