From 54f23a51b5fc0833bfe6e276885382a9fe27e968 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 28 Jul 2022 13:49:36 +0200 Subject: [PATCH 1/5] Add test --- .../Directed/aliasing_retbuf/CMakeLists.txt | 7 + .../aliasing_retbuf/aliasing_retbuf.cs | 130 ++++++++++++++++++ .../aliasing_retbuf/aliasing_retbuf.csproj | 13 ++ .../aliasing_retbuf_native.cpp | 25 ++++ 4 files changed, 175 insertions(+) create mode 100644 src/tests/JIT/Directed/aliasing_retbuf/CMakeLists.txt create mode 100644 src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs create mode 100644 src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.csproj create mode 100644 src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp diff --git a/src/tests/JIT/Directed/aliasing_retbuf/CMakeLists.txt b/src/tests/JIT/Directed/aliasing_retbuf/CMakeLists.txt new file mode 100644 index 00000000000000..cc07026a86f45e --- /dev/null +++ b/src/tests/JIT/Directed/aliasing_retbuf/CMakeLists.txt @@ -0,0 +1,7 @@ +project(AliasingRetBufNative) + +# This test always needs to be optimized to hit the problem. +set(CMAKE_BUILD_TYPE Release) + +add_library(AliasingRetBufNative SHARED aliasing_retbuf_native.cpp) + diff --git a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs new file mode 100644 index 00000000000000..8947a44e1dec90 --- /dev/null +++ b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs @@ -0,0 +1,130 @@ +// 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 System.Runtime.InteropServices; + +public unsafe class AliasingRetBuf +{ + public static int Main() + { + int failures = 0; + + Foo f = new Foo { A = 1, B = 2, C = 3 }; + CallPtrPInvoke(&f); + if (f.A != 2 || f.B != 3 || f.C != 1) + { + Console.WriteLine("FAIL: After CallPtrPInvoke: {0}", f); + failures |= 1; + } + + f = new Foo { A = 1, B = 2, C = 3 }; + CallRefPInvoke(ref f); + if (f.A != 2 || f.B != 3 || f.C != 1) + { + Console.WriteLine("FAIL: After CallRefPInvoke: {0}", f); + failures |= 2; + } + + f = new Foo { A = 1, B = 2, C = 3 }; + CallStructFieldPInvoke(ref f); + if (f.A != 2 || f.B != 3 || f.C != 1) + { + Console.WriteLine("FAIL: After CallStructFieldPInvoke: {0}", f); + failures |= 4; + } + + IntPtr lib = NativeLibrary.Load(nameof(AliasingRetBufNative), typeof(AliasingRetBufNative).Assembly, null); + IntPtr export = NativeLibrary.GetExport(lib, "TransposeRetBuf"); + + f = new Foo { A = 3, B = 2, C = 1 }; + CallPtrFPtr(&f, (delegate* unmanaged[SuppressGCTransition])export); + if (f.A != 2 || f.B != 1 || f.C != 3) + { + Console.WriteLine("FAIL: After CallPtrFPtr: {0}", f); + failures |= 8; + } + + f = new Foo { A = 3, B = 2, C = 1 }; + CallStructFieldFPtr(ref f, (delegate* unmanaged[SuppressGCTransition])export); + if (f.A != 2 || f.B != 1 || f.C != 3) + { + Console.WriteLine("FAIL: After CallStructFieldFPtr: {0}", f); + failures |= 16; + } + + f = new Foo { A = 3, B = 2, C = 1 }; + CallRefFPtr(ref f, (delegate* unmanaged[SuppressGCTransition])export); + if (f.A != 2 || f.B != 1 || f.C != 3) + { + Console.WriteLine("FAIL: After CallRefFPtr: {0}", f); + failures |= 32; + } + + if (failures == 0) + { + Console.WriteLine("PASS"); + } + + return 100 + failures; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CallPtrPInvoke(Foo* fi) + { + *fi = AliasingRetBufNative.TransposeRetBufPtr(fi); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CallRefPInvoke(ref Foo fi) + { + fi = AliasingRetBufNative.TransposeRetBufRef(ref fi); + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static void CallStructFieldPInvoke(ref Foo fi) + { + Fooer fooer = new() { F = fi }; + fooer.F = AliasingRetBufNative.TransposeRetBufPtr(&fooer.F); + fi = fooer.F; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CallPtrFPtr(Foo* fi, delegate* unmanaged[SuppressGCTransition] fptr) + { + *fi = fptr(fi); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CallRefFPtr(ref Foo fi, delegate* unmanaged[SuppressGCTransition] fptr) + { + fi = fptr(ref fi); + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static void CallStructFieldFPtr(ref Foo fi, delegate* unmanaged[SuppressGCTransition] fptr) + { + Fooer fooer = new() { F = fi }; + fooer.F = fi; + fooer.F = fptr(&fooer.F); + fi = fooer.F; + } + + private record struct Foo(nint A, nint B, nint C); + private struct Fooer + { + public Foo F; + } + + static class AliasingRetBufNative + { + [DllImport(nameof(AliasingRetBufNative), EntryPoint = "TransposeRetBuf")] + [SuppressGCTransition] + public static unsafe extern Foo TransposeRetBufPtr(Foo* fi); + + [DllImport(nameof(AliasingRetBufNative), EntryPoint = "TransposeRetBuf")] + [SuppressGCTransition] + public static unsafe extern Foo TransposeRetBufRef(ref Foo fi); + } +} \ No newline at end of file diff --git a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.csproj b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.csproj new file mode 100644 index 00000000000000..3cf2777cc643db --- /dev/null +++ b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.csproj @@ -0,0 +1,13 @@ + + + Exe + True + True + + + + + + + + \ No newline at end of file diff --git a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp new file mode 100644 index 00000000000000..af93c7c444f90c --- /dev/null +++ b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if defined(_MSC_VER) +#define EXPORT_API extern "C" __declspec(dllexport) +#else +#define EXPORT_API extern "C" __attribute__((visibility("default"))) +#endif + +struct Foo +{ + void* A; + void* B; + void* C; +}; + +EXPORT_API +Foo TransposeRetBuf(Foo* fi) +{ + Foo f; + f.A = fi->B; + f.B = fi->C; + f.C = fi->A; + return f; +} \ No newline at end of file From a1af63b4fd8b1c9469eb11202afdaa370edebe96 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 29 Jul 2022 14:07:07 +0200 Subject: [PATCH 2/5] JIT: Disallow return buffer aliasing for unmanaged calls Native calling conventions disallow the return buffer from aliasing anything else. The managed ABI allows this so we need to handle unmanaged calls specially. Fix #72270 --- src/coreclr/jit/importer.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 2346219ffbd42b..a8b45546d800c7 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1219,7 +1219,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, var_types asgType = src->TypeGet(); - if (src->gtOper == GT_CALL) + if (src->IsCall()) { GenTreeCall* srcCall = src->AsCall(); if (srcCall->TreatAsShouldHaveRetBufArg(this)) @@ -10995,6 +10995,17 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN { assert(returnType == TYP_UNKNOWN); call->gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG; + + if (call->IsUnmanaged()) + { + // Native ABIs do not allow retbufs to alias anything. + // This is allowed by the managed ABI and impAssignStructPtr will + // never introduce copies due to this. + unsigned tmpNum = lvaGrabTemp(true DEBUGARG("Retbuf for unmanaged call")); + impAssignTempGen(tmpNum, call, retClsHnd, (unsigned)CHECK_SPILL_ALL); + return gtNewLclvNode(tmpNum, lvaGetDesc(tmpNum)->TypeGet()); + } + return call; } From 606a5903311d332059d179ede3c155e84f9ed16b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 29 Jul 2022 14:15:23 +0200 Subject: [PATCH 3/5] Test nit --- src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs index 8947a44e1dec90..04ee323bd791c5 100644 --- a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs +++ b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs @@ -70,13 +70,13 @@ public static int Main() return 100 + failures; } - [MethodImpl(MethodImplOptions.NoInlining)] + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] private static void CallPtrPInvoke(Foo* fi) { *fi = AliasingRetBufNative.TransposeRetBufPtr(fi); } - [MethodImpl(MethodImplOptions.NoInlining)] + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] private static void CallRefPInvoke(ref Foo fi) { fi = AliasingRetBufNative.TransposeRetBufRef(ref fi); @@ -90,13 +90,13 @@ private static void CallStructFieldPInvoke(ref Foo fi) fi = fooer.F; } - [MethodImpl(MethodImplOptions.NoInlining)] + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] private static void CallPtrFPtr(Foo* fi, delegate* unmanaged[SuppressGCTransition] fptr) { *fi = fptr(fi); } - [MethodImpl(MethodImplOptions.NoInlining)] + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] private static void CallRefFPtr(ref Foo fi, delegate* unmanaged[SuppressGCTransition] fptr) { fi = fptr(ref fi); From 5e207d57b1712e18bd52ecace6b503ecdac46213 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 29 Jul 2022 22:14:47 +0200 Subject: [PATCH 4/5] Fix test on x86 --- .../aliasing_retbuf/aliasing_retbuf.cs | 18 +++++++++--------- .../aliasing_retbuf/aliasing_retbuf_native.cpp | 9 +++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs index 04ee323bd791c5..7374aed3e463d3 100644 --- a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs +++ b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf.cs @@ -39,7 +39,7 @@ public static int Main() IntPtr export = NativeLibrary.GetExport(lib, "TransposeRetBuf"); f = new Foo { A = 3, B = 2, C = 1 }; - CallPtrFPtr(&f, (delegate* unmanaged[SuppressGCTransition])export); + CallPtrFPtr(&f, (delegate* unmanaged[Cdecl, SuppressGCTransition])export); if (f.A != 2 || f.B != 1 || f.C != 3) { Console.WriteLine("FAIL: After CallPtrFPtr: {0}", f); @@ -47,7 +47,7 @@ public static int Main() } f = new Foo { A = 3, B = 2, C = 1 }; - CallStructFieldFPtr(ref f, (delegate* unmanaged[SuppressGCTransition])export); + CallStructFieldFPtr(ref f, (delegate* unmanaged[Cdecl, SuppressGCTransition])export); if (f.A != 2 || f.B != 1 || f.C != 3) { Console.WriteLine("FAIL: After CallStructFieldFPtr: {0}", f); @@ -55,7 +55,7 @@ public static int Main() } f = new Foo { A = 3, B = 2, C = 1 }; - CallRefFPtr(ref f, (delegate* unmanaged[SuppressGCTransition])export); + CallRefFPtr(ref f, (delegate* unmanaged[Cdecl, SuppressGCTransition])export); if (f.A != 2 || f.B != 1 || f.C != 3) { Console.WriteLine("FAIL: After CallRefFPtr: {0}", f); @@ -91,19 +91,19 @@ private static void CallStructFieldPInvoke(ref Foo fi) } [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] - private static void CallPtrFPtr(Foo* fi, delegate* unmanaged[SuppressGCTransition] fptr) + private static void CallPtrFPtr(Foo* fi, delegate* unmanaged[Cdecl, SuppressGCTransition] fptr) { *fi = fptr(fi); } [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] - private static void CallRefFPtr(ref Foo fi, delegate* unmanaged[SuppressGCTransition] fptr) + private static void CallRefFPtr(ref Foo fi, delegate* unmanaged[Cdecl, SuppressGCTransition] fptr) { fi = fptr(ref fi); } [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] - private static void CallStructFieldFPtr(ref Foo fi, delegate* unmanaged[SuppressGCTransition] fptr) + private static void CallStructFieldFPtr(ref Foo fi, delegate* unmanaged[Cdecl, SuppressGCTransition] fptr) { Fooer fooer = new() { F = fi }; fooer.F = fi; @@ -119,12 +119,12 @@ private struct Fooer static class AliasingRetBufNative { - [DllImport(nameof(AliasingRetBufNative), EntryPoint = "TransposeRetBuf")] + [DllImport(nameof(AliasingRetBufNative), EntryPoint = "TransposeRetBuf", CallingConvention = CallingConvention.Cdecl)] [SuppressGCTransition] public static unsafe extern Foo TransposeRetBufPtr(Foo* fi); - [DllImport(nameof(AliasingRetBufNative), EntryPoint = "TransposeRetBuf")] + [DllImport(nameof(AliasingRetBufNative), EntryPoint = "TransposeRetBuf", CallingConvention = CallingConvention.Cdecl)] [SuppressGCTransition] public static unsafe extern Foo TransposeRetBufRef(ref Foo fi); } -} \ No newline at end of file +} diff --git a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp index af93c7c444f90c..d56c02b1a086bc 100644 --- a/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp +++ b/src/tests/JIT/Directed/aliasing_retbuf/aliasing_retbuf_native.cpp @@ -1,8 +1,14 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#define CALLCONV + #if defined(_MSC_VER) #define EXPORT_API extern "C" __declspec(dllexport) +#ifdef _WIN32 +#undef CALLCONV +#define CALLCONV __cdecl +#endif #else #define EXPORT_API extern "C" __attribute__((visibility("default"))) #endif @@ -14,8 +20,7 @@ struct Foo void* C; }; -EXPORT_API -Foo TransposeRetBuf(Foo* fi) +EXPORT_API Foo CALLCONV TransposeRetBuf(Foo* fi) { Foo f; f.A = fi->B; From 832eeb251a3e38529b474da1ae3c86af773ff470 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 1 Aug 2022 12:25:00 +0200 Subject: [PATCH 5/5] Disable test on mono wasm --- src/tests/issues.targets | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 0352b6bc49395c..a13e3ecf235b9e 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -3395,6 +3395,9 @@ https://github.com/dotnet/runtime/issues/41519 + + https://github.com/dotnet/runtime/issues/41519 + https://github.com/dotnet/runtime/issues/41472