dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.55k stars 4.54k forks source link

MemsetMemcpyNullref test fails on mono #98628

Open EgorBo opened 4 months ago

EgorBo commented 4 months ago

The test I am adding in https://github.com/dotnet/runtime/pull/98623 fails on mono (at least on mono windows x64 Release @ Windows.10.Amd64.Open - logs. probably will fail on other pipelines as well).

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
using Xunit;

public unsafe class MemsetMemcpyNullref
{
    [Fact]
    public static void MemsetMemcpyThrowNullRefonNull()
    {
        Assert.Throws<NullReferenceException>(() => MemoryInit(null));
        Assert.Throws<NullReferenceException>(() => MemoryCopy(null, null));
        Assert.Throws<NullReferenceException>(() =>
            {
                // Check when only src is null
                HugeStruct hs = default;
                MemoryCopy(&hs, null);
            });
        Assert.Throws<NullReferenceException>(() =>
            {
                // Check when only dst is null
                HugeStruct hs = default;
                MemoryCopy(null, &hs);
            });

        // Check various lengths
        uint[] lengths = [1, 10, 100, 1000, 10000, 100000, 1000000];
        foreach (uint length in lengths)
        {
            Assert.Throws<NullReferenceException>(() => MemoryInitByref(ref Unsafe.NullRef<byte>(), length));
            Assert.Throws<NullReferenceException>(() => MemoryCopyByref(ref Unsafe.NullRef<byte>(), ref Unsafe.NullRef<byte>(), length));
        }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void MemoryCopy(HugeStruct* dst, HugeStruct* src) => 
        *dst = *src;

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void MemoryCopyByref(ref byte dst, ref byte src, uint len) => 
        Unsafe.CopyBlockUnaligned(ref dst, ref src, len);

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void MemoryInit(HugeStruct* dst) => 
        *dst = default;

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void MemoryInitByref(ref byte dst, uint len) => 
        Unsafe.InitBlockUnaligned(ref dst, 42, len);

    private struct HugeStruct
    {
        public fixed byte Data[20_000];
    }
}
lambdageek commented 4 months ago

/cc @steveisok

vargaz commented 4 months ago

This happens because it eventually calls Buffer:Memmove, which doesn't throw for null/null because of these lines:

            if (((nuint)(nint)Unsafe.ByteOffset(ref src, ref dest) < len) || ((nuint)(nint)Unsafe.ByteOffset(ref dest, ref src) < len))
            {
                goto BuffersOverlap;
            }
        BuffersOverlap:
            // If the buffers overlap perfectly, there's no point to copying the data.
            if (Unsafe.AreSame(ref dest, ref src))
            {
                return;
            }
steveisok commented 2 weeks ago

Actually, given it's windows x64 mono, we probably won't fix this.

lambdageek commented 2 weeks ago

This is a disabled test issue, and it's marked for all platforms. So we should update issues.targets before closing; also we should validate what happens on non-Windows platforms

https://github.com/dotnet/runtime/blob/18bc1158575cc4c653a660d4a52c0217ea20c647/src/tests/issues.targets#L1911-L1913