Fix check for memory containment safety. (dotnet/coreclr#22563)

This change ensures that if an operand can produce an exception
and any instructions executed after the operand evaluation but before
the operand's parent can also produce an exception, the operand
shouldn't be contained. The reason is that in this case operand
containment may reorder exceptions.

With `strict` set to true the containment is blocked here:
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@d27fff3f65193dd71c6197e9876101f496bbd28b/src/jit/sideeffects.cpp#L485-L488

Also, make the check for ordering side-effect interference less
conservative.

Fixes dotnet/coreclr#22556.

Commit migrated from dotnet/coreclr@6c9ad25

src/coreclr/src/jit/lower.cpp

@@ -87,7 +87,8 @@ bool Lowering::IsSafeToContainMem(GenTree* parentNode, GenTree* childNode)
 
     for (GenTree* node = childNode->gtNext; node != parentNode; node = node->gtNext)
     {
-        if (m_scratchSideEffects.InterferesWith(comp, node, false))
+        const bool strict = true;
+        if (m_scratchSideEffects.InterferesWith(comp, node, strict))
         {
             return false;
         }

src/coreclr/src/jit/sideeffects.cpp

@@ -450,7 +450,7 @@ void SideEffectSet::AddNode(Compiler* compiler, GenTree* node)
 //    Two side effect sets interfere under any of the following
 //    conditions:
 //    - If the analysis is strict, and:
-//        - Either set contains a compiler barrier, or
+//        - One set contains a compiler barrier and the other set contains a global reference, or
 //        - Both sets produce an exception
 //    - Whether or not the analysis is strict:
 //        - One set produces an exception and the other set contains a
@@ -475,8 +475,14 @@ bool SideEffectSet::InterferesWith(unsigned               otherSideEffectFlags,
 
     if (strict)
     {
-        // If either set contains a compiler barrier, the sets interfere.
-        if (((m_sideEffectFlags | otherSideEffectFlags) & GTF_ORDER_SIDEEFF) != 0)
+        // If either set contains a compiler barrier, and the other set contains a global reference,
+        // the sets interfere.
+        if (((m_sideEffectFlags & GTF_ORDER_SIDEEFF) != 0) && ((otherSideEffectFlags & GTF_GLOB_REF) != 0))
+        {
+            return true;
+        }
+
+        if (((otherSideEffectFlags & GTF_ORDER_SIDEEFF) != 0) && ((m_sideEffectFlags & GTF_GLOB_REF) != 0))
         {
             return true;
         }

src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_22556/GitHub_22556.cs

@@ -0,0 +1,46 @@
+// 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 class Test
+{
+    int f;
+
+    Test(int f)
+    {
+        this.f = f;
+    }
+
+    public static int Main()
+    {
+        try
+        {
+            Add(null, 0);
+        }
+        catch (Exception e)
+        {
+            if (e is NullReferenceException)
+            {
+                Console.WriteLine("PASS");
+                return 100;
+            }
+        }
+        Console.WriteLine("FAIL");
+        return -1;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Add(Test t, int i)
+    {
+        // When t is null and i is 0, this should throw
+        // NullReferenceException since the operands of
+        // addition have to be evaluated left to right.
+        int x = t.f + 1 / i;
+        return x;
+    }
+}
+

src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_22556/GitHub_22556.csproj

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{2649FAFE-07BF-4F93-8120-BA9A69285ABB}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <PropertyGroup>
+    <LangVersion>latest</LangVersion>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>