Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix asserts and GC holes with Unsafe.Subtract/ByteOffset #99019

Merged
merged 15 commits into from
Feb 29, 2024
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
Loading