Skip to content

Commit 8c90a74

Browse files
authored
Fix optImpliedByTypeOfAssertions to correctly find a non-null assertion (#120917)
Fixes #120792 This looks like some very old bug that was exposed by an unrelated change in .NET 10.0 Basically, we have an assertion `obj->pMT == <cns_type>` we invoke `optImpliedByTypeOfAssertions` to see if we can find some existing assertion (anywhere) for `obj != null` so we can duplicate it into current `ASSERT_TP& activeAssertions`, because `obj->pMT` assertion implies obj is not null. We do find such an assertion but for some reason we don't check that it's null on the right side (op2) -> obj != null. Instead, we pick up some random `obj != <frozen_object>` assertion which then is incorrectly used. [No diffs.](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1180340&view=ms.vss-build-web.run-extensions-tab)
1 parent 8cb57ab commit 8c90a74

File tree

3 files changed

+51
-14
lines changed

3 files changed

+51
-14
lines changed

src/coreclr/jit/assertionprop.cpp

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5776,6 +5776,8 @@ bool Compiler::optCreateJumpTableImpliedAssertions(BasicBlock* switchBb)
57765776

57775777
void Compiler::optImpliedByTypeOfAssertions(ASSERT_TP& activeAssertions)
57785778
{
5779+
assert(!optLocalAssertionProp);
5780+
57795781
if (BitVecOps::IsEmpty(apTraits, activeAssertions))
57805782
{
57815783
return;
@@ -5804,31 +5806,25 @@ void Compiler::optImpliedByTypeOfAssertions(ASSERT_TP& activeAssertions)
58045806
{
58055807
AssertionDsc* impAssertion = optGetAssertion(impIndex);
58065808

5807-
// The impAssertion must be different from the chkAssertion
5809+
// The impAssertion must be different from the chkAssertion
58085810
if (impIndex == chkAssertionIndex)
58095811
{
58105812
continue;
58115813
}
58125814

5813-
// impAssertion must be a Non Null assertion on lclNum
5814-
if ((impAssertion->assertionKind != OAK_NOT_EQUAL) || (impAssertion->op1.kind != O1K_LCLVAR) ||
5815-
(impAssertion->op2.kind != O2K_CONST_INT) || (impAssertion->op1.vn != chkAssertion->op1.vn))
5815+
// impAssertion must be a Non Null assertion on op1.vn
5816+
if ((impAssertion->assertionKind != OAK_NOT_EQUAL) || !impAssertion->CanPropNonNull() ||
5817+
(impAssertion->op1.vn != chkAssertion->op1.vn))
58165818
{
58175819
continue;
58185820
}
58195821

58205822
// The bit may already be in the result set
5821-
if (!BitVecOps::IsMember(apTraits, activeAssertions, impIndex - 1))
5823+
if (BitVecOps::TryAddElemD(apTraits, activeAssertions, impIndex - 1))
58225824
{
5823-
BitVecOps::AddElemD(apTraits, activeAssertions, impIndex - 1);
5824-
#ifdef DEBUG
5825-
if (verbose)
5826-
{
5827-
printf("\nCompiler::optImpliedByTypeOfAssertions: %s Assertion #%02d, implies assertion #%02d",
5828-
(chkAssertion->op1.kind == O1K_SUBTYPE) ? "Subtype" : "Exact-type", chkAssertionIndex,
5829-
impIndex);
5830-
}
5831-
#endif
5825+
JITDUMP("\nCompiler::optImpliedByTypeOfAssertions: %s Assertion #%02d, implies assertion #%02d",
5826+
(chkAssertion->op1.kind == O1K_SUBTYPE) ? "Subtype" : "Exact-type", chkAssertionIndex,
5827+
impIndex);
58325828
}
58335829

58345830
// There is at most one non-null assertion that is implied by the current chkIndex assertion
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Runtime.CompilerServices;
6+
using System.Threading;
7+
using Xunit;
8+
9+
public class Runtime_120792
10+
{
11+
[Fact]
12+
public static void TestEntryPoint()
13+
{
14+
for (int i = 0; i < 300; i++)
15+
{
16+
Problem(42);
17+
Thread.Sleep(16);
18+
}
19+
}
20+
21+
[MethodImpl(MethodImplOptions.NoInlining)]
22+
private static void Problem(object value)
23+
{
24+
Type tt = value == null ? typeof(object) : value.GetType();
25+
if (!tt.Equals(typeof(int)) || value == null)
26+
throw new InvalidOperationException();
27+
}
28+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<DebugType>None</DebugType>
4+
<Optimize>True</Optimize>
5+
<!-- Needed for CLRTestEnvironmentVariable -->
6+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
7+
</PropertyGroup>
8+
<ItemGroup>
9+
<Compile Include="$(MSBuildProjectName).cs" />
10+
<CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="1" />
11+
<CLRTestEnvironmentVariable Include="DOTNET_TieredPGO" Value="1" />
12+
</ItemGroup>
13+
</Project>

0 commit comments

Comments
 (0)