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

Unsupported PreserveSig = false combination prevent working with images in WinForms. #1183

Closed
kant2002 opened this issue May 29, 2021 · 5 comments
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation

Comments

@kant2002
Copy link
Contributor

Any image related code crashed with following exception

System.Exception: Method '[System.Windows.Forms.Primitives]Interop+Ole32.OleCreatePictureIndirect(PICTDESC*,Guid&,BOOL)' requires marshalling that is not yet supported by this compiler.
   at Interop.Ole32.OleCreatePictureIndirect(Interop.Ole32.PICTDESC*, Guid&, Interop.BOOL) + 0x45

Lines which crash compiler:

https://github.com/dotnet/winforms/blob/d46ad2e8dc76248739d9ae22b28b399a6a6b299e/src/System.Windows.Forms.Primitives/src/Interop/Ole32/Interop.OleCreatePictureIndirect.cs#L12-L14

@jkotas jkotas added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label May 30, 2021
@kant2002
Copy link
Contributor Author

kant2002 commented Jun 5, 2021

I see two cases left

  1. Return results is some is int-mappable value. Enum or probably any struct of size 4 bytes falls into this category.
    For example:
public static extern HRESULT GetErrorInfo(uint dwReserved, ref IErrorInfo pperrinfo);
  1. Return value is object or any custom struct.
    For example:
object OleCreatePictureIndirect(PICTDESC* pictdesc, ref Guid refiid, BOOL fOwn)

For case 1 I probably should extend condition here somehow.

if (!_flags.PreserveSig && _targetMethod.Signature.ReturnType != _targetMethod.Context.GetWellKnownType(WellKnownType.Void))
throw new NotSupportedException();

And that would be it in general.

With case 2 I'm have one idea, but not sure that this is what you want to have as part of code.
My understanding that call

object OleCreatePictureIndirect(PICTDESC* pictdesc, ref Guid refiid, BOOL fOwn)

should be translated to

int OleCreatePictureIndirect(PICTDESC* pictdesc, ref Guid refiid, BOOL fOwn, out object retval);

This increase count of marshallers by 1 then all other cases and I do not sure where modifications should be.
I can increate count of marshallers in the InitializeMarshallers method.

private static Marshaller[] InitializeMarshallers(MethodDesc targetMethod, InteropStateManager interopStateManager, PInvokeFlags flags)

Maybe some other kind of MethodThunk should be created for this case, and then I can use that as switch here

So I would like to have some advice on case 2

@jkotas
Copy link
Member

jkotas commented Jun 5, 2021

[DllImport(Libraries.Oleaut32, ExactSpelling = true, PreserveSig = false)]
public static extern HRESULT GetErrorInfo(uint dwReserved, ref IErrorInfo pperrinfo);

This is wrong PInvoke definition. PreserveSig = false should be deleted here. It works by a pure luck, and I am not sure whether it is even the case. It is used on error handling path, and it is possible that this eror handling path is never actually used and nobody noticed that it is broken. Would you like to send PR to WinForms to fix it up?

This increase count of marshallers by 1

I do not think increasing count of marshallers would work well. I think you want to keep the count of the marshallers same, and instead handle the HRESULT swapping by adding special cases as necessary. It is how CoreCLR does it - look for SF_IsHRESULTSwapping in https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/dllimport.cpp.

@kant2002
Copy link
Contributor Author

kant2002 commented Jun 5, 2021

It works by a pure luck, and I am not sure whether it is even the case.

I tried with

[DllImport("dwmapi.dll", EntryPoint = "DwmIsCompositionEnabled", PreserveSig = false)]
public static extern HRESULT DwmIsCompositionEnabled(IntPtr x);

DwmIsCompositionEnabled(IntPtr.Zero);

And this code throws ArgumentException on CoreCLR. So I assume that even this is wierd it should be supported. Isn't it?

Would you like to send PR to WinForms to fix it up?

Will do. No error handling there at all, I assume they want PreserveSig = false and change result to void

@kant2002
Copy link
Contributor Author

kant2002 commented Jun 5, 2021

@jkotas
Copy link
Member

jkotas commented Jun 5, 2021

And this code throws ArgumentException on CoreCLR. So I assume that even this is wierd it should be supported. Isn't it?

It should be transformed into int DwmIsCompositionEnabled(IntPtr x, out HRESULT x); just like the case 2. The behavior at runtime may be unpredictable just like for any other mismatched PInvoke signature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Development

No branches or pull requests

2 participants