Jit: Fix SetIndirExceptionFlags.

SetIndirExceptionFlags should not set `GTF_IND_NONFAULTING` flag if the
address has `GTF_EXCEPT` flag.

The failing scenario was:

We were setting `GTF_IND_NONFAULTING` on this indirection (since `ADDR` Node
can't be null)

```
               [000003] *--XG-------              *  IND       int
               [000002] ---XG-------              \--*  ADDR      byref  Zero Fseq[i]
               [000001] ---XG-------                 \--*  FIELD     struct s
               [000000] ------------                    \--*  LCL_VAR   ref    V00 arg0
```
this was then transformed to

```
               [000003] *---G-------              *  IND       int
               [000013] -----+------              \--*  ADD       byref
               [000000] -----+------                 +--*  LCL_VAR   ref    V00 arg0
               [000012] -----+------                 \--*  CNS_INT   long   8 field offset Fseq[s, i]
```
The `GTF_EXCEPT` flag was cleared on `IND` because it had `GTF_IND_NONFAULTING`set
and the address no longer had  `GTF_EXCEPT` flag.

Fixes dotnet/coreclr#27027.

Commit migrated from https://github.com/dotnet/coreclr/commit/f58ce06e619e607d8e297d57f10afdc15cfce232

diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp
index ef95417288ce6d3c8cd6245f177eed6db8ae3b1b..231ab70e3edfaba92c775305bfca17344d64be61 100644 (file)
--- a/src/coreclr/src/jit/gentree.cpp
+++ b/src/coreclr/src/jit/gentree.cpp
@@ -7953,7 +7953,7 @@ void Compiler::gtUpdateNodeOperSideEffects(GenTree* tree)
         tree->gtFlags &= ~GTF_EXCEPT;
         if (tree->OperIsIndirOrArrLength())
         {
-            tree->gtFlags |= GTF_IND_NONFAULTING;
+            tree->SetIndirExceptionFlags(this);
         }
     }
 
@@ -9271,6 +9271,38 @@ bool GenTree::Precedes(GenTree* other)
     return false;
 }
 
+//------------------------------------------------------------------------------
+// SetIndirExceptionFlags : Set GTF_EXCEPT and GTF_IND_NONFAULTING flags as appropriate
+//                          on an indirection or an array length node.
+//
+// Arguments:
+//    comp  - compiler instance
+//
+
+void GenTree::SetIndirExceptionFlags(Compiler* comp)
+{
+    GenTree* addr = nullptr;
+    if (OperIsIndir())
+    {
+        addr = AsIndir()->Addr();
+    }
+    else
+    {
+        assert(gtOper == GT_ARR_LENGTH);
+        addr = AsArrLen()->ArrRef();
+    }
+
+    if (OperMayThrow(comp) || ((addr->gtFlags & GTF_EXCEPT) != 0))
+    {
+        gtFlags |= GTF_EXCEPT;
+    }
+    else
+    {
+        gtFlags &= ~GTF_EXCEPT;
+        gtFlags |= GTF_IND_NONFAULTING;
+    }
+}
+
 #ifdef DEBUG
 
 /* static */ int GenTree::gtDispFlags(unsigned flags, unsigned debugFlags)

diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h
index 6bc6a5193249f161a0718b8989c73680ade0e991..33bc1f0cbfcfed8e628e3e97d46b6e4941067394 100644 (file)
--- a/src/coreclr/src/jit/gentree.h
+++ b/src/coreclr/src/jit/gentree.h
@@ -2101,11 +2101,7 @@ public:
         gtFlags &= ~GTF_REUSE_REG_VAL;
     }
 
-    void SetIndirExceptionFlags(Compiler* comp)
-    {
-        assert(OperIsIndirOrArrLength());
-        gtFlags |= OperMayThrow(comp) ? GTF_EXCEPT : GTF_IND_NONFAULTING;
-    }
+    void SetIndirExceptionFlags(Compiler* comp);
 
 #if MEASURE_NODE_SIZE
     static void DumpNodeSizes(FILE* fp);

diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp
index 89721522c10cdb10d0fae9e5a906e7cb6701d45b..33fa62db63182a19b35aa2fa75985a0026359de1 100644 (file)
--- a/src/coreclr/src/jit/morph.cpp
+++ b/src/coreclr/src/jit/morph.cpp
@@ -6188,7 +6188,6 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac)
         tree->SetOper(GT_IND);
         tree->gtOp.gtOp1 = addr;
 
-        tree->gtFlags &= (~GTF_EXCEPT | addr->gtFlags);
         tree->SetIndirExceptionFlags(this);
 
         if (addExplicitNullCheck)
@@ -8859,7 +8858,6 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree)
                 tree->gtFlags |= GTF_GLOB_REF;
             }
 
-            dest->gtFlags &= (~GTF_EXCEPT | dest->AsIndir()->Addr()->gtFlags);
             dest->SetIndirExceptionFlags(this);
             tree->gtFlags |= (dest->gtFlags & GTF_EXCEPT);
         }
@@ -8906,7 +8904,6 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree)
                     src->gtFlags |= (GTF_GLOB_REF | GTF_IND_TGTANYWHERE);
                 }
 
-                src->gtFlags &= (~GTF_EXCEPT | src->AsIndir()->Addr()->gtFlags);
                 src->SetIndirExceptionFlags(this);
             }
         }
@@ -11756,21 +11753,24 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
 
 DONE_MORPHING_CHILDREN:
 
-    if (tree->OperMayThrow(this))
+    if (tree->OperIsIndirOrArrLength())
     {
-        // Mark the tree node as potentially throwing an exception
-        tree->gtFlags |= GTF_EXCEPT;
+        tree->SetIndirExceptionFlags(this);
     }
     else
     {
-        if (tree->OperIsIndirOrArrLength())
+        if (tree->OperMayThrow(this))
         {
-            tree->gtFlags |= GTF_IND_NONFAULTING;
+            // Mark the tree node as potentially throwing an exception
+            tree->gtFlags |= GTF_EXCEPT;
         }
-        if (((op1 == nullptr) || ((op1->gtFlags & GTF_EXCEPT) == 0)) &&
-            ((op2 == nullptr) || ((op2->gtFlags & GTF_EXCEPT) == 0)))
+        else
         {
-            tree->gtFlags &= ~GTF_EXCEPT;
+            if (((op1 == nullptr) || ((op1->gtFlags & GTF_EXCEPT) == 0)) &&
+                ((op2 == nullptr) || ((op2->gtFlags & GTF_EXCEPT) == 0)))
+            {
+                tree->gtFlags &= ~GTF_EXCEPT;
+            }
         }
     }
 
@@ -14649,7 +14649,7 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac)
             tree->gtDynBlk.Addr()        = fgMorphTree(tree->gtDynBlk.Addr());
             tree->gtDynBlk.gtDynamicSize = fgMorphTree(tree->gtDynBlk.gtDynamicSize);
 
-            tree->gtFlags &= (~GTF_EXCEPT & ~GTF_CALL);
+            tree->gtFlags &= ~GTF_CALL;
             tree->SetIndirExceptionFlags(this);
 
             if (tree->OperGet() == GT_STORE_DYN_BLK)

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_27027/GitHub_27027.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_27027/GitHub_27027.cs
new file mode 100644 (file)
index 0000000..c18dea5
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_27027/GitHub_27027.cs
@@ -0,0 +1,40 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.CompilerServices;
+
+public struct S
+{
+    public int i;
+    public int j;
+}
+
+public class Test
+{
+    public S s;
+
+    public static int Main()
+    {
+        // Test that the correct exception is thrown from Run.
+        // The bug was that the exceptions were reordered and DivideByZeroException
+        // was thrown instead of NullReferenceException.
+        try {
+            Run(null, 0);
+        }
+        catch (System.NullReferenceException)
+        {
+            return 100;
+        }
+
+        return -1;
+    }
+ 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static int Run(Test test, int j)
+    {
+        int k = test.s.i + 1/j;
+        return k;
+    }
+}

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_27027/GitHub_27027.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_27027/GitHub_27027.csproj
new file mode 100644 (file)
index 0000000..656bedd
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_27027/GitHub_27027.csproj
@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType />
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>