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 fails in Debug mode, with SuppressGCTransition and certain signatures #46184

Closed
kevzhao2 opened this issue Dec 17, 2020 · 2 comments · Fixed by #47320
Closed

Comments

@kevzhao2
Copy link

Description

The following code results in Fatal error. Invalid Program: attempted to call a UnmanagedCallersOnly method from managed code..

using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

unsafe class Program
{
    [DllImport("NativeDll", CallingConvention = CallingConvention.Cdecl)]
    public static extern void set_callback(delegate* unmanaged[Cdecl]<int, void> callback);

    [DllImport("NativeDll", CallingConvention = CallingConvention.Cdecl), SuppressGCTransition]
    public static extern void set_value(void* unused, int value);

    [DllImport("NativeDll", CallingConvention = CallingConvention.Cdecl)]
    public static extern void call_callback(void* unused, int unused2);

    [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })]
    private static void PrintInt(int value)
    {
        Console.WriteLine(value);
        Console.ReadKey(true);
    }

    static void Main()
    {
        set_callback(&PrintInt);
        set_value(null, 1234);
        call_callback(null, 1234);  // crashes here!
    }
}
void (*globalCallback)(int value) = nullptr;
int globalValue = 0;

extern "C" __declspec(dllexport) void set_callback(void (*callback)(int value)) {
	globalCallback = callback;
}

extern "C" __declspec(dllexport) void set_value(void*, int value) {
	globalValue = value;
}

extern "C" __declspec(dllexport) void call_callback(void*, int) {
	globalCallback(globalValue);
}

Configuration

.NET Version: 5.0.101
OS: Windows 10 (18363.1256)
Architecture: x64

Regression?

N/A, since a lot of this stuff didn't even exist on 3.1.

Other information

Notably, if at least one of the following is true, then the crash no longer occurs:

  1. x86 is used instead of x64.
  2. SuppressGCTransition is removed from set_value.
  3. set_value is modified to take a long instead of an int.
  4. call_callback is modified to take a long instead of an int.
  5. The code is run under Release mode.
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Interop-coreclr untriaged New issue has not been triaged by the area owner labels Dec 17, 2020
@jkotas jkotas added the bug label Jan 5, 2021
@jkotas
Copy link
Member

jkotas commented Jan 5, 2021

@AaronRobinsonMSFT @jkoritzinsky There seems to be a bug in interop sharing of IL marshalling stubs. We do not include SuppressGCTransition in the sharing key, and thus can end up using SuppressGCTransition IL marshaling stub for non-SuppressGCTransition case, or vice versa.

I just helped @trylek to debug a crash that was caused by this problem. I think we should consider fixing it in servicing.

@AaronRobinsonMSFT
Copy link
Member

@jkotas Agreed. I was chatting with @elinor-fung about this today actually and she was thinking it was a caching bug. Not taking this attribute into consideration is definitely an issue - boo.

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Jan 5, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 6.0.0 milestone Jan 5, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 22, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 27, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants