Don't remove dead indirections that may throw. (#40226)

Liveness incorrectly removed indirection rhs's of dead assignments.
This change fixes that.

Fixes #39823.

src/coreclr/src/jit/liveness.cpp

@@ -1980,11 +1980,10 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
                             GenTree* data =
                                 store->OperIs(GT_STOREIND) ? store->AsStoreInd()->Data() : store->AsBlk()->Data();
                             data->SetUnusedValue();
+
                             if (data->isIndir())
                             {
-                                // This is a block assignment. An indirection of the rhs is not considered
-                                // to happen until the assignment so mark it as non-faulting.
-                                data->gtFlags |= GTF_IND_NONFAULTING;
+                                Lowering::TransformUnusedIndirection(data->AsIndir(), this, block);
                             }
 
                             blockRange.Remove(store);
@@ -2322,17 +2321,6 @@ bool Compiler::fgRemoveDeadStore(GenTree**        pTree,
             }
 #endif // DEBUG
             // Extract the side effects
-            if (rhsNode->TypeGet() == TYP_STRUCT)
-            {
-                // This is a block assignment. An indirection of the rhs is not considered to
-                // happen until the assignment, so we will extract the side effects from only
-                // the address.
-                if (rhsNode->OperIsIndir())
-                {
-                    assert(rhsNode->OperGet() != GT_NULLCHECK);
-                    rhsNode = rhsNode->AsIndir()->Addr();
-                }
-            }
             gtExtractSideEffList(rhsNode, &sideEffList);
         }
 

src/coreclr/src/jit/lower.cpp

@@ -6450,41 +6450,7 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
 
         if (ind->OperIs(GT_NULLCHECK) || ind->IsUnusedValue())
         {
-            // A nullcheck is essentially the same as an indirection with no use.
-            // The difference lies in whether a target register must be allocated.
-            // On XARCH we can generate a compare with no target register as long as the addresss
-            // is not contained.
-            // On ARM64 we can generate a load to REG_ZR in all cases.
-            // However, on ARM we must always generate a load to a register.
-            // In the case where we require a target register, it is better to use GT_IND, since
-            // GT_NULLCHECK is a non-value node and would therefore require an internal register
-            // to use as the target. That is non-optimal because it will be modeled as conflicting
-            // with the source register(s).
-            // So, to summarize:
-            // - On ARM64, always use GT_NULLCHECK for a dead indirection.
-            // - On ARM, always use GT_IND.
-            // - On XARCH, use GT_IND if we have a contained address, and GT_NULLCHECK otherwise.
-            // In all cases, change the type to TYP_INT.
-            //
-            ind->gtType = TYP_INT;
-#ifdef TARGET_ARM64
-            bool useNullCheck = true;
-#elif TARGET_ARM
-            bool useNullCheck = false;
-#else  // TARGET_XARCH
-            bool useNullCheck = !ind->Addr()->isContained();
-#endif // !TARGET_XARCH
-
-            if (useNullCheck && ind->OperIs(GT_IND))
-            {
-                ind->ChangeOper(GT_NULLCHECK);
-                ind->ClearUnusedValue();
-            }
-            else if (!useNullCheck && ind->OperIs(GT_NULLCHECK))
-            {
-                ind->ChangeOper(GT_IND);
-                ind->SetUnusedValue();
-            }
+            TransformUnusedIndirection(ind, comp, m_block);
         }
     }
     else
@@ -6496,6 +6462,55 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
     }
 }
 
+//------------------------------------------------------------------------
+// TransformUnusedIndirection: change the opcode and the type of the unused indirection.
+//
+// Arguments:
+//    ind   - Indirection to transform.
+//    comp  - Compiler instance.
+//    block - Basic block of the indirection.
+//
+void Lowering::TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, BasicBlock* block)
+{
+    // A nullcheck is essentially the same as an indirection with no use.
+    // The difference lies in whether a target register must be allocated.
+    // On XARCH we can generate a compare with no target register as long as the addresss
+    // is not contained.
+    // On ARM64 we can generate a load to REG_ZR in all cases.
+    // However, on ARM we must always generate a load to a register.
+    // In the case where we require a target register, it is better to use GT_IND, since
+    // GT_NULLCHECK is a non-value node and would therefore require an internal register
+    // to use as the target. That is non-optimal because it will be modeled as conflicting
+    // with the source register(s).
+    // So, to summarize:
+    // - On ARM64, always use GT_NULLCHECK for a dead indirection.
+    // - On ARM, always use GT_IND.
+    // - On XARCH, use GT_IND if we have a contained address, and GT_NULLCHECK otherwise.
+    // In all cases, change the type to TYP_INT.
+    //
+    assert(ind->OperIs(GT_NULLCHECK, GT_IND));
+
+    ind->gtType = TYP_INT;
+#ifdef TARGET_ARM64
+    bool useNullCheck = true;
+#elif TARGET_ARM
+    bool useNullCheck = false;
+#else  // TARGET_XARCH
+    bool useNullCheck = !ind->Addr()->isContained();
+#endif // !TARGET_XARCH
+
+    if (useNullCheck && ind->OperIs(GT_IND))
+    {
+        comp->gtChangeOperToNullCheck(ind, block);
+        ind->ClearUnusedValue();
+    }
+    else if (!useNullCheck && ind->OperIs(GT_NULLCHECK))
+    {
+        ind->ChangeOper(GT_IND);
+        ind->SetUnusedValue();
+    }
+}
+
 //------------------------------------------------------------------------
 // LowerBlockStoreCommon: a common logic to lower STORE_OBJ/BLK/DYN_BLK.
 //

src/coreclr/src/jit/lower.h

@@ -8,6 +8,7 @@ XX                               Lower                                       XX
 XX                                                                           XX
 XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
+XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 */
 
 #ifndef _LOWER_H_
@@ -532,6 +533,8 @@ public:
     bool IsContainableHWIntrinsicOp(GenTreeHWIntrinsic* containingNode, GenTree* node, bool* supportsRegOptional);
 #endif // FEATURE_HW_INTRINSICS
 
+    static void TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, BasicBlock* block);
+
 private:
     static bool NodesAreEquivalentLeaves(GenTree* candidate, GenTree* storeInd);
 

src/tests/JIT/Regression/JitBlue/GitHub_39823/GitHub_39823.cs

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Diagnostics;
+
+class Runtime_39823
+{
+    struct IntsWrapped
+    {
+        public int i1;
+        public int i2;
+        public int i3;
+        public int i4;
+    };
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static unsafe int TestUnusedObjCopy(IntsWrapped* ps)
+    {
+        IntsWrapped s = *ps;
+        return 100;
+    }
+
+
+    public static unsafe int Main()
+    {
+        try
+        {
+            TestUnusedObjCopy((IntsWrapped*)0);
+            Debug.Assert(false, "unreachable");
+        }
+        catch
+        {
+            return 100;
+        }
+        return -1;
+    }
+}

src/tests/JIT/Regression/JitBlue/GitHub_39823/GitHub_39823.csproj

+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>Full</DebugType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>