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

C#9 Function pointer with generic return parameter crashes with BadImageFormat #46402

Closed
fbrosseau opened this issue Dec 25, 2020 · 10 comments
Closed

Comments

@fbrosseau
Copy link

fbrosseau commented Dec 25, 2020

Description

Hello,

The following program crashes with BadImageFormatException (for simplicity, ignore the fact this simplified repro would nullref in the normal case, this bug happens before that)

        static TRet CallGenericReturn<TRet>(delegate* unmanaged<TRet> del)
            where TRet : unmanaged
        {
            return del();
        }

        static int Main(string[] args)
        {
            CallGenericReturn<int>(null);
        }
System.BadImageFormatException: 'Illegal or unimplemented ELEM_TYPE in signature. The format of the file test.dll' is invalid.'
04 0000006c`08d7d270 00007ffd`8bc33acb     coreclr!ThrowBadFormatWorkerT<Module>+0x28 [coreclr\src\vm\exceptmacros.h @ 519]
05 0000006c`08d7d2a0 00007ffd`8bc64e6b     coreclr!SigPointer::ConvertToInternalExactlyOne+0x513 [coreclr\src\vm\siginfo.cpp @ 219]
06 0000006c`08d7d350 00007ffd`8bc65149     coreclr!SigPointer::ConvertToInternalSignature+0xbb [coreclr\src\vm\siginfo.cpp @ 371]
07 0000006c`08d7d3c0 00007ffd`8bc64f6c     coreclr!NDirect::CreateHashBlob+0x18d [coreclr\src\vm\dllimport.cpp @ 4012]
08 0000006c`08d7d7f0 00007ffd`8bc64394     coreclr!ILStubCreatorHelper::ILStubCreatorHelper+0x58 [coreclr\src\vm\dllimport.h @ 667]
09 0000006c`08d7d830 00007ffd`8bc02137     coreclr!CreateInteropILStub+0x100 [coreclr\src\vm\dllimport.cpp @ 4734]
0a 0000006c`08d7de50 00007ffd`8bddb13c     coreclr!NDirect::CreateCLRToNativeILStub+0x167 [coreclr\src\vm\dllimport.cpp @ 5015]
0b 0000006c`08d7df10 00007ffd`8bddbad1     coreclr!GetILStubForCalli+0x350 [coreclr\src\vm\dllimport.cpp @ 6946]
0c 0000006c`08d7e120 00007ffd`8bd0b2be     coreclr!GenericPInvokeCalliStubWorker+0x91 [coreclr\src\vm\dllimport.cpp @ 6833]
0d 0000006c`08d7e190 00007ffd`2c1b6a7c     coreclr!GenericPInvokeCalliGenILStub+0x5e [coreclr\src\vm\amd64\PInvokeStubs.asm @ 74]

Question

Is this an unsupported scenario the same way generic delegates have never been supported in PInvoke scenarios?

Configuration

5.0.101
x64

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 25, 2020
@AaronRobinsonMSFT
Copy link
Member

Is this an unsupported scenario the same way generic delegates have never been supported in PInvoke scenarios?

@fbrosseau Not at present. There is a small comment in the code that describes the issue actually.

// @TODO GENERICS: We may be calling a varargs method from a
// generic type/method. Using an empty context will make such a
// case cause an unexpected exception. To make this work,
// we need to create a specialized signature for every instantiation
SigTypeContext typeContext;
MetaSig metasig(vaSignature, this, &typeContext);
ArgIterator argit(&metasig);

I did start on this support here but haven't gotten around to finishing it.

/cc @davidwrighton @elinor-fung @jkoritzinsky

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Jan 8, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 6.0.0 milestone Jan 8, 2021
@jkotas
Copy link
Member

jkotas commented Jan 8, 2021

This gets pretty complex with shared generic code. To fully support this, we would need to introduce a new generic dictionary type and tech JIT how to pass it around.

@jkotas
Copy link
Member

jkotas commented Jan 8, 2021

I think supporting this for blittable types may be reasonable, but non-blittable types should stay unsupported. It should avoid the problems with the shared generic code as side-effect.

@fbrosseau
Copy link
Author

fbrosseau commented Jan 8, 2021

Thanks for the replies. I thought unmanaged function pointers were limited to blittable types in the first place?? Or is this only for UnmanagedCallersOnly? I remember hitting a similar problem and working around it by using primitives only

@jkotas
Copy link
Member

jkotas commented Jan 8, 2021

It is for UnmanagedCallersOnly. Calling unmanaged function pointers works for non-blittable types for historic reasons. It worked in managed C++ since forever, and so we could not easily take away when exposing function pointers in C#. It is best to use the unmanaged function pointers with blittable types only.

@fbrosseau
Copy link
Author

fbrosseau commented Jan 8, 2021

Thanks for the details. I remember now that what I had to change at the time was ref/in/out blittables as function parameters with UnmanagedCallersOnly, so I changed to pointers. I don't want to derail my own thread, but is this something that could work, too? Or does ref/in/out require work by the marshaller at runtime, in native to managed transitions? I could create a separate issue if this is something that could realistically be supported.

@jkotas
Copy link
Member

jkotas commented Jan 8, 2021

Or does ref/in/out require work by the marshaller at runtime, in native to managed transitions?

Yes, ref/in/out requires marshalling. It can be supported in future via source generators for interop. We do not have plans for improving runtime built-in support for marshalling of non-blittable types.

@DaZombieKiller
Copy link
Contributor

Would this be considered a duplicate of #9136?

@AaronRobinsonMSFT
Copy link
Member

Would this be considered a duplicate of #9136?

@DaZombieKiller Yes, it does appear so.

Closing in lieu of #9136.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants