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

UnmanagedCallersOnly does not work when called from a P/Invoke in Release mode #38192

Closed
tannergooding opened this issue Jun 20, 2020 · 8 comments · Fixed by #38303
Closed

UnmanagedCallersOnly does not work when called from a P/Invoke in Release mode #38192

tannergooding opened this issue Jun 20, 2020 · 8 comments · Fixed by #38303
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@tannergooding
Copy link
Member

As per the title, the following program succeeds if run under Debug but fails under Release

using System;
using System.Runtime.InteropServices;

unsafe class Program
{
    public const int CS_VREDRAW = 0x0001;
    public const int CS_HREDRAW = 0x0002;

    public const int CW_USEDEFAULT = unchecked((int)0x80000000);

    public const int SW_SHOWDEFAULT = 10;

    public const uint WS_OVERLAPPED = 0x00000000;
    public const uint WS_CAPTION = 0x00C00000;
    public const uint WS_SYSMENU = 0x00080000;
    public const uint WS_THICKFRAME = 0x00040000;
    public const uint WS_MINIMIZEBOX = 0x00020000;
    public const uint WS_MAXIMIZEBOX = 0x00010000;

    public const uint WS_OVERLAPPEDWINDOW = WS_OVERLAPPED | WS_CAPTION | WS_SYSMENU | WS_THICKFRAME | WS_MINIMIZEBOX | WS_MAXIMIZEBOX;

    public unsafe partial struct WNDCLASSEXW
    {
        public uint cbSize;
        public uint style;
        public delegate* stdcall<IntPtr, uint, nuint, nint, nint> lpfnWndProc;
        public int cbClsExtra;
        public int cbWndExtra;
        public IntPtr hInstance;
        public IntPtr hIcon;
        public IntPtr hCursor;
        public IntPtr hbrBackground;
        public ushort* lpszMenuName;
        public ushort* lpszClassName;
        public IntPtr hIconSm;
    }

    [DllImport("user32", EntryPoint = "CreateWindowExW", ExactSpelling = true)]
    public static extern IntPtr CreateWindowExW(uint dwExStyle, ushort* lpClassName, ushort* lpWindowName, uint dwStyle, int X, int Y, int nWidth, int nHeight, IntPtr hWndParent, IntPtr hMenu, IntPtr hInstance, void* lpParam);

    [DllImport("user32", EntryPoint = "DefWindowProcW", ExactSpelling = true)]
    public static extern nint DefWindowProcW(IntPtr hWnd, uint Msg, nuint wParam, nint lParam);

    [DllImport("user32", EntryPoint = "RegisterClassExW", ExactSpelling = true)]
    public static extern ushort RegisterClassExW(WNDCLASSEXW* param0);

    [UnmanagedCallersOnly]
    private static nint WindowProc(IntPtr hWnd, uint Msg, nuint wParam, nint lParam)
    {
        return DefWindowProcW(hWnd, Msg, wParam, lParam);
    }

    static void Main(string[] args)
    {
        fixed (char* lpszClassName = "SampleClass")
        fixed (char* lpWindowName = "SampleTitle")
        {
            var hInstance = Marshal.GetHINSTANCE(typeof(Program).Module);

            // Requires an explicit cast until C# handles UnmanagedCallersOnly
            var wndProc = (delegate* stdcall<IntPtr, uint, nuint, nint, nint>)(delegate* managed<IntPtr, uint, nuint, nint, nint>)&WindowProc;

            // Initialize the window class.
            var windowClass = new WNDCLASSEXW
            {
                cbSize = (uint)sizeof(WNDCLASSEXW),
                style = CS_HREDRAW | CS_VREDRAW,
                lpfnWndProc = wndProc,
                hInstance = hInstance,
                lpszClassName = (ushort*)lpszClassName
            };
            _ = RegisterClassExW(&windowClass);

            var hwnd = CreateWindowExW(
                0,
                windowClass.lpszClassName,
                (ushort*)lpWindowName,
                WS_OVERLAPPEDWINDOW,
                X: CW_USEDEFAULT,
                Y: CW_USEDEFAULT,
                nWidth: CW_USEDEFAULT,
                nHeight: CW_USEDEFAULT,
                hWndParent: IntPtr.Zero,
                hMenu: IntPtr.Zero,
                hInstance,
                lpParam: null
            );
        }
    }
}

The csproj should resemble the following so function pointers can be used:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
    <LangVersion>preview</LangVersion>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  </PropertyGroup>

  <PropertyGroup>
    <RestoreSources>
      https://api.nuget.org/v3/index.json;
      https://dotnet.myget.org/F/roslyn/api/v3/index.json;
    </RestoreSources>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Net.Compilers.Toolset" Version="3.8.0-1.20320.1" />
  </ItemGroup>

</Project>

A more complete program will successfully display the window, handle the callbacks etc in Debug. Additionally it will all work as expected under Release if using Marshal.GetFunctionPointerForDelegate instead.

However, under Release when using UnmanagedCallersOnly, it fails with the following:

Fatal error. Invalid Program: attempted to call a UnmanagedCallersOnly method from managed code.
Repeat 2 times:
--------------------------------
   at Program.CreateWindowExW(UInt32, UInt16*, UInt16*, UInt32, Int32, Int32, Int32, Int32, IntPtr, IntPtr, IntPtr, Void*)
--------------------------------
   at Program.Main(System.String[])
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Interop-coreclr untriaged New issue has not been triaged by the area owner labels Jun 20, 2020
@tannergooding
Copy link
Member Author

CC. @AaronRobinsonMSFT, @333fred

@john-h-k
Copy link
Contributor

+1, happens to me in similar situation with CreateWindowExW callback

@jkotas
Copy link
Member

jkotas commented Jun 20, 2020

This is caused by TODO-Cleanup: setting GCState to 1 seems to be redundant here:

// TODO-Cleanup: setting GCState to 1 seems to be redundant as InsertPInvokeCallProlog will set it to zero before a

This redundant operation is actively harmful for UnmanagedCallersOnly methods.

@jkotas jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-Interop-coreclr labels Jun 20, 2020
@jkotas
Copy link
Member

jkotas commented Jun 20, 2020

@dotnet/jit-contrib Could you please take a look at this soon?

@jkotas jkotas added this to the 5.0.0 milestone Jun 20, 2020
@AaronRobinsonMSFT
Copy link
Member

Talking this over with @jkotas a repro using no Win32 APIs is below. I am planning on adding additional test cases for function pointers soon and I will make sure this case is added.

Managed:

[DllImport(...)]
static extern void Test(void* callback);
[DllImport(...)]
static extern int Test2(int a);

[UnmanagedCallersOnly]
static int Callback(int a)
{
    // Inlined P/Invoke
    return Test2(a);
}

static void Main(string[] args)
{
    var cb = (delegate* stdcall<int, int>)(delegate* managed<int, int>)&Callback;
    Test(cb);
}

Unmanaged:

EXPORT_FUNC void CALLCONV Test(int (CALLCONV *cb)(int))
{
    // Multiple calls to callback.
    ::printf("Test(): %d\n", cb(10));
    ::printf("Test(): %d\n", cb(20)); // Works if commented out.
}

EXPORT_FUNC int CALLCONV Test2(int a)
{
    ::printf("Test2(%d)\n", a);
    return a;
}

@sandreenko
Copy link
Contributor

Does this issue explain OSX failures #38189?

@AaronRobinsonMSFT
Copy link
Member

I am adding a disabled test in #38195 that can be uncommented and used for validation when this issue is fixed.

@AndyAyersMS AndyAyersMS added bug and removed untriaged New issue has not been triaged by the area owner labels Jun 22, 2020
@CarolEidt
Copy link
Contributor

@dotnet/jit-contrib I'll look at this; let me know if any of you have already begun working on it.

@CarolEidt CarolEidt self-assigned this Jun 22, 2020
CarolEidt added a commit to CarolEidt/runtime that referenced this issue Jun 23, 2020
The comment indicated this was necessary to keep the FrameListRoot live, but the code generated by `CreateFrameLinkUpdate` will do that if needed.

Fix dotnet#38192
CarolEidt added a commit that referenced this issue Jun 25, 2020
* Don't set GCState to 1 in PInvoke Epilog

Remove the incorrect setting of GCState and fix the liveness computation for compLvFrameListRoot for tailcalls.

Fix #38192
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants