Skip to content

Commit

Permalink
Fix the ExposedLocalsNumbering test (#105037)
Browse files Browse the repository at this point in the history
* Fix the ExposedLocalsNumbering test

The test had a stack corrupting race condition:

1) Main thread returns from "Main".
2) Main thread calls some runtime code on the stack of former "Main".
3) Mutator thread writes to the "safeIndex" variable, which is now
   part of that runtime's code stack.
4) Things crash.

Fix by 'parking' the mutated index in some dynamic memory instead.

* issues.targets de-exclusion
  • Loading branch information
SingleAccretion authored Jul 19, 2024
1 parent 8cc102f commit 31bc167
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 14 deletions.
21 changes: 10 additions & 11 deletions src/tests/JIT/opt/ValueNumbering/ExposedLocalsNumbering.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,23 @@

public unsafe class ExposedLocalsNumbering
{
private static volatile bool s_mutateIndex;
private const int UnsafeIndex = 1;

private static volatile bool s_finished;
private static int* s_pIndex = (int*)NativeMemory.Alloc(4);

[Fact]
public static int TestEntryPoint()
{
const int RetryCount = 100;
const int UnsafeIndex = 1;

try
{
new Thread(_ =>
{
while (!s_finished)
{
if (s_mutateIndex)
{
*s_pIndex = UnsafeIndex;
}
*s_pIndex = UnsafeIndex;
}
}).Start();
}
Expand All @@ -40,12 +37,11 @@ public static int TestEntryPoint()
int[] array = new int[UnsafeIndex + 1];
array[UnsafeIndex] = 1;

int safeIndex = 0;
for (int i = 0; i < RetryCount; i++)
{
try
{
if (RunBoundsChecks(array.AsSpan(0, UnsafeIndex), &safeIndex) != 0)
if (RunBoundsChecks(array.AsSpan(0, UnsafeIndex)) != 0)
{
s_finished = true;
return 101;
Expand All @@ -59,7 +55,7 @@ public static int TestEntryPoint()
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static int RunBoundsChecks(Span<int> span, int* pSafeIndex)
private static int RunBoundsChecks(Span<int> span)
{
int result = 0;
int index = 0;
Expand All @@ -80,8 +76,12 @@ private static int RunBoundsChecks(Span<int> span, int* pSafeIndex)
result += span[index];
result += span[index];

int* pSafeIndex = (int*)NativeMemory.AllocZeroed(4);
s_pIndex = pSafeIndex;
s_mutateIndex = false;

// Wait until the mutator thread sees the switch, so that it
// doesn't write to the stack of a method that has returned.
while (Volatile.Read(ref *pSafeIndex) != UnsafeIndex) { }

return result;
}
Expand All @@ -90,6 +90,5 @@ private static int RunBoundsChecks(Span<int> span, int* pSafeIndex)
private static void CaptureIndex(int* pIndex)
{
s_pIndex = pIndex;
s_mutateIndex = true;
}
}
3 changes: 0 additions & 3 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@
<ExcludeList Include="$(XunitTestBinBase)/baseservices/finalization/CriticalFinalizer/*">
<Issue>https://github.com/dotnet/runtime/issues/76041</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/JIT/opt/ValueNumbering/ExposedLocalsNumbering/**">
<Issue>https://github.com/dotnet/runtime/issues/80184</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/JIT/opt/SSA/MemorySsa/**">
<Issue>https://github.com/dotnet/runtime/issues/86112</Issue>
</ExcludeList>
Expand Down

0 comments on commit 31bc167

Please sign in to comment.