Could not load type "that is incorrectly aligned or overlapped by a non-object field"

[wjk]

Actual behavior

When I import the PROPVARIANT type, the type is not declared property and throws a TypeLoadException when referenced. The error message is “Could not load type __Anonymous_e__Union from assembly '…' because it contains an object field at offset 0 that is incorrectly aligned or overlapped by a non-object field.”

Expected behavior

Type should work.

Repro steps

1. NativeMethods.txt content:

PROPVARIANT
IPropertyStore
SHGetPropertyStoreForWindow

2. NativeMethods.json content:

{
    "$schema": "https://aka.ms/CsWin32.schema.json",
    "className": "NativeMethods",
    "namespace": "Windows.Win32",
    "wideCharOnly": true
}

3. Any of your own code that should be shared?

This is the class that references the broken type:

FormShellProperties.cs

Context

• CsWin32 version: 0.1.422-beta
• Win32Metadata version: not explicitly set
• Target Framework: net5.0-windows
• LangVersion: 9

[AArnott]

I believe you can set allowMarshaling to false to workaround this.

CsWin32/src/Microsoft.Windows.CsWin32/settings.schema.json

Lines 40 to 44 in 66f4221

	"allowMarshaling": {
	"title": "Emit COM interfaces instead of structs, and allow generation of non-blittable structs for the sake of an easier to use API.",
	"type": "boolean",
	"default": true
	},

[wjk]

allowMarshaling does indeed fix the type-load exception. I’m getting a SEHException when I call into the object, though. I think this may be because I am using the wrong technique to turn the void* COM instance pointer returned by the function call into an IPropertyStore instance. I’m currently doing the following:

NativeMethods.SHGetPropertyStoreForWindow((HWND)form.Handle, typeof(IPropertyStore).GUID, out void* ppv).ThrowOnFailure();
return *(IPropertyStore*)ppv;

Is this incorrect? Thanks!

[AArnott]

It looks like you're trying to return an IPropertyStore struct to your caller. When allowMarshaling is set to false, you can't do that. Since the IPropertyStore is a struct`, but the COM object is actually an object on the heap somewhere, you have to keep it as a pointer:

NativeMethods.SHGetPropertyStoreForWindow((HWND)form.Handle, typeof(IPropertyStore).GUID, out void* ppv).ThrowOnFailure();
-return *(IPropertyStore*)ppv;
+return (IPropertyStore*)ppv;

Then when you're done using it, you have to call propertyStore->Release() yourself.

[wjk]

That did it! Thanks!

[AArnott]

We should probably keep this active to track the bug when allowMarshaling: true, yes?

[AArnott]

@AaronRobinsonMSFT @tannergooding I'm hoping you can help me out here. What should the C# projection do with structs with overlapping (union) fields where the fields represent a mixture of objects and values? The CLR doesn't allow that. I think my only option would be to declare all the fields as value types, including perhaps generating structs for what might otherwise be a .NET interface or object. That won't be a great experience for the user, but is there an alternative?

And just as one particular case study, I was looking at VARDESC which strikes me as particularly strange because the unioned fields have different sizes. The metadata describes it as:

public struct VARDESC
{
	[StructLayout(LayoutKind.Explicit)]
	public struct _Anonymous_e__Union
	{
		[FieldOffset(0)]
		public uint oInst;

		[FieldOffset(0)]
		public unsafe VARIANT* lpvarValue;
	}
	public int memid;
	public PWSTR lpstrSchema;
	public _Anonymous_e__Union Anonymous;
	public ELEMDESC elemdescVar;
	public ushort wVarFlags;
	public VARKIND varkind;
}

Note how one field is exactly 4 bytes in length while the other is pointer-sized. Is that really a stable layout, considering various CPU architectures?

[davkean]

Note how one field is exactly 4 bytes in length while the other is pointer-sized. Is that really a stable layout, considering various CPU architectures?

@AArnott Looking around on the interweb, VC compiler & GCC appear to pick the size of the largest union field, ie the size of that union will always be pointer size (before factoring in padding).

[AaronRobinsonMSFT]

@davkean is correct. The union is defined to be as large as the largest field in bytes. How to interact with the union is slightly different between C and C++, but that shouldn't be an issue in this case. I've create something in godbolt so you can see the layout generated by MSVC - https://godbolt.org/z/c7M4v6EvP.

Which field is an object ref in this case? I don't see it.

[AArnott]

@AaronRobinsonMSFT the object field is VARIANT*, which becomes object or object[].

[tannergooding]

You cannot union a value and reference type. You really shouldn't union incompatible reference types (its akin to Unsafe.As<T>(object obj) and is undefined behavior when obj isn't actually a T).

At a minimum you're going to need to break it apart so that the value and reference types aren't overlapping, and you'll need to handle that as part of the marshalling logic.

[AArnott]

Thanks, @tannergooding. The metadata doesn't give me the information necessary to know which field has the real value. The docs may, but cswin32 would need something from the metadata to automate that. I can't change the memory layout of the struct so they don't overlap either as that would break interop with the native code. So I'm not sure what you're proposing here.

[AArnott]

VARDESC in particular is odd because the field type is VARIANT* instead of just VARIANT. Since VARIANT would naturally translate to object, our projection rules lead us to 'box' the managed object by declaring it as being inside an array. That's how we get from VARIANT* to object[] in this case. However I guess we just need to declare VARIANT as a struct and leave this as a VARIANT* when the field overlaps with other value types.

I guess having two reference typed fields would be problematic as well, right? So perhaps the logic should be if one or more fields in a union struct are reference types, generate them as value types instead. Or more simply put: "never emit ref types in union fields". We should either fail generation (since it'll fail at runtime anyway) or fallback to allowMarshaling: false behavior for those particular structs.

[tannergooding]

I can't change the memory layout of the struct so they don't overlap either as that would break interop with the native code. So I'm not sure what you're proposing here.

The data is already not blittable and so is already incompatible with native code. The built-in marshalling system isn't really recommended for "new code", the DllImport Source Generator is going to be the recommended thing moving forward.

But in either case, the built-in marshalling system can't support such a definition anyways because the type system fundamentally doesn't support it. You will have to define and implement manual marshalling here if your type is both a union and that union contains any reference types.

[AArnott]

Thanks. That sounds like support for my prior comment, that I emit interop types that don't activate the .NET marshaler. Optionally I could add additional help to automate the marshaling in my own generated code, but since I lack the metadata to do so, it'll just be left as an exercise to the reader.

[AArnott]

Unfortunately we have these types like BINDPTR which contains one ref type to an interface that can generally be a ref type, but cannot appear as one at this particular location. I guess I'll have to generate ITypeComp both as an interface and (with some mangled name) as an unmanaged struct as well. What a painful API this will be.

But maybe there's an opportunity at least for me to generate some methods on the ITypeComp struct to make it easier to safely obtain an ITypeComp interface instance based on it, complete with a ReleaseRef call on the original pointer.

[tannergooding]

That sounds like support for my prior comment, that I emit interop types that don't activate the .NET marshaler

This would just be defining 100% blittable data types (which is what TerraFX.Interop.Windows does). Anything else is a "convenience wrapper" on top of this that deals with things like pinning, marshalling, etc (which is what DllImport Source Generator is responsible for generating from a given non-blittable interop signature).

But maybe there's an opportunity at least for me to generate some methods on the ITypeComp struct to make it easier to safely obtain an ITypeComp interface instance based on it, complete with a ReleaseRef call on the original pointer.

The way I handle this in TerraFX.Interop.Windows: https://source.terrafx.dev/#TerraFX.Interop.Windows/Windows/um/oaidl/ITypeComp.cs,5a439f56db1bf1c9

There is the base struct:

public unsafe partial struct ITypeComp : ITypeComp.Interface
{
    public void** lpVtbl;

    // ...

Various helper methods exposing the VTBL members and account for ABI differences (returning HRESULT isn't ABI compatible, so you'll note that the fnptr returns int and there is an implicit conversion to the user friendly type):

    // ...

    public HRESULT QueryInterface(Guid* riid, void** ppvObject)
    {
        return ((delegate* unmanaged<ITypeComp*, Guid*, void**, int>)(lpVtbl[0]))((ITypeComp*)Unsafe.AsPointer(ref this), riid, ppvObject);
    }

    // ...

An interface defining the required exposed members and the inheritance heirarchy:

    // ...

    public interface Interface : IUnknown.Interface
    {
        HRESULT Bind(ushort* szName, uint lHashVal, ushort wFlags, ITypeInfo** ppTInfo, DESCKIND* pDescKind, BINDPTR* pBindPtr);
        HRESULT BindType(ushort* szName, uint lHashVal, ITypeInfo** ppTInfo, ITypeComp** ppTComp);
    }

   // ...

and finally a VTBL declaration for convenience:

    // ...

    public partial struct Vtbl<TSelf>
        where TSelf : unmanaged, Interface
    {
        public delegate* unmanaged<TSelf*, Guid*, void**, int> QueryInterface;
        public delegate* unmanaged<TSelf*, uint> AddRef;
        public delegate* unmanaged<TSelf*, uint> Release;
        public delegate* unmanaged<TSelf*, ushort*, uint, ushort, ITypeInfo**, DESCKIND*, BINDPTR*, int> Bind;
        public delegate* unmanaged<TSelf*, ushort*, uint, ITypeInfo**, ITypeComp**, int> BindType;
    }
}

All of these combined allow robust and convenient access/usability of the types and data, even while being unsafe. There are various attributs like VtblIndex and NativeTypeName which track "lost metadata" represented in C/C++ and which are stripped in Release builds.

[AaronRobinsonMSFT]

@AaronRobinsonMSFT the object field is VARIANT*, which becomes object or object[].

Ah yes. I forgot this was "conveniently" converted for users.

I've got nothing to add to what @tannergooding has suggested.

[AArnott]

CsWin32 already has the ability to emit only blittable types, and looks very similar to your structs with a vtbl field in them. But we don't do both modes at once right now. The user has to choose one or the other. Your approach of always emitting both is interesting, and something that CsWin32 has to get at least closer to.
Thanks for you tips and links. I'll drill more into this.