Skip to content

Commit

Permalink
Fix asserts and GC holes with Unsafe.Subtract/ByteOffset (#99019)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichalPetryka committed Feb 29, 2024
1 parent 8dcca1c commit b72ec6c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9220,11 +9220,11 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA

// TODO #4104: there are a lot of other places where
// this condition is not checked before transformations.
if (fgGlobalMorph)
noway_assert(op2);
if (fgGlobalMorph && !op2->TypeIs(TYP_BYREF))
{
/* Check for "op1 - cns2" , we change it to "op1 + (-cns2)" */

noway_assert(op2);
if (op2->IsCnsIntOrI() && !op2->IsIconHandle())
{
// Negate the constant and change the node to be "+",
Expand All @@ -9242,7 +9242,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
noway_assert(op1);
if (op1->IsCnsIntOrI())
{
noway_assert(varTypeIsIntOrI(tree));
noway_assert(varTypeIsIntegralOrI(tree));

// The type of the new GT_NEG node cannot just be op2->TypeGet().
// Otherwise we may sign-extend incorrectly in cases where the GT_NEG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,26 @@ public static void ByteOffsetStackByte4()
Assert.Equal(new IntPtr(-3), Unsafe.ByteOffset(ref byte4.B3, ref byte4.B0));
}

private static unsafe class StaticReadonlyHolder
{
public static readonly void* Pointer = (void*)RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(StaticReadonlyHolder), 1);
}

[Fact]
public static unsafe void ByteOffsetConstantRef()
{
// https://github.com/dotnet/runtime/pull/99019
[MethodImpl(MethodImplOptions.NoInlining)]
static nint NullTest(ref byte origin) => Unsafe.ByteOffset(ref origin, ref Unsafe.NullRef<byte>());
Assert.Equal(0, NullTest(ref Unsafe.NullRef<byte>()));

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static ref byte GetStatic(ref byte x) => ref x;
[MethodImpl(MethodImplOptions.NoInlining)]
static nint StaticReadonlyTest(ref byte x) => Unsafe.ByteOffset(ref GetStatic(ref Unsafe.AsRef<byte>(StaticReadonlyHolder.Pointer)), ref x);
Assert.Equal(0, StaticReadonlyTest(ref Unsafe.AsRef<byte>(StaticReadonlyHolder.Pointer)));
}

[Fact]
public static unsafe void AsRef()
{
Expand Down Expand Up @@ -597,7 +617,7 @@ public static void RefAddNuintByteOffset()
}

[Fact]
public static void RefSubtract()
public static unsafe void RefSubtract()
{
string[] a = new string[] { "abc", "def", "ghi", "jkl" };

Expand All @@ -609,6 +629,11 @@ public static void RefSubtract()

ref string r3 = ref Unsafe.Subtract(ref r2, 3);
Assert.Equal("abc", r3);

// https://github.com/dotnet/runtime/pull/99019
[MethodImpl(MethodImplOptions.NoInlining)]
static ref byte NullTest(nuint offset) => ref Unsafe.Subtract(ref Unsafe.NullRef<byte>(), offset);
Assert.True(Unsafe.IsNullRef(ref NullTest(0)));
}

[Fact]
Expand Down

0 comments on commit b72ec6c

Please sign in to comment.