Runtime: A structure with a virtual table and a double field incorrectly marshalled with a sequential layout

[ddobrev]

I have the following C++ class:

struct __declspec(dllexport) TestProperties
{
public:
    int Field;
    const int& ConstRefField;
    bool isVirtual();
    virtual void setVirtual(bool value);

protected:
    const int ArchiveName;

private:
    int FieldValue;
    double _refToPrimitiveInSetter;
    int _getterAndSetterWithTheSameName;
    int _setterReturnsBoolean;
    int _virtualSetterReturnsBoolean;
    int _conflict;
    int(*_callback)(int);
};

I've created the following structure to marshal it:

C# [StructLayout(LayoutKind.Sequential)] public partial struct __Internal { internal IntPtr vfptr_TestProperties; internal int Field; internal IntPtr ConstRefField; internal int ArchiveName; internal int FieldValue; internal double _refToPrimitiveInSetter; internal int _getterAndSetterWithTheSameName; internal int _setterReturnsBoolean; internal int _virtualSetterReturnsBoolean; internal int _conflict; internal IntPtr _callback; }

On x86 Windows with MSVC, as I've specified below, the second field of Field has offset of 8. The sequential layout above, however, calculates it as 4 which causes crashes at run-time.

Configuration

.NET Framework 4.7.2
Windows x86
MSVC

Other information

I can use explicit layouts, or I can pad the missing 4 bytes with fixed byte[4]. However, I much prefer sequential layouts as they are simpler and more portable. I also think they should be able to handle the specific padding and alignment of the underlying platform - perhaps by providing more properties such as the alignment as suggested at https://github.com/dotnet/runtime/issues/9089 and https://github.com/dotnet/runtime/issues/22990.

[Dotnet-GitSync-Bot]

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.

[tannergooding]

For reference: https://godbolt.org/z/xddv7c

[tannergooding]

It's worth noting however, that for a struct without a vtbl but an explicit first member that is a void* the struct is marshalled correctly: https://godbolt.org/z/1zP5qj

There would need to be some feature which explicitly annotates that a given field is the vtbl and therefore should be marshalled differently.

[EgorBo]

I can use explicit layouts, or I can pad the missing 4 bytes with fixed byte[4]

Or like this:

[StructLayout(LayoutKind.Sequential)]
public partial struct __Internal
{
    private long __vfptr_TestProperties;
    internal IntPtr vfptr_TestProperties => (IntPtr)__vfptr_TestProperties;
    internal int Field;
    ...

[ddobrev]

@tannergooding thank you for your replies. Do you think such a fix would be difficult or a breaking change?

[tannergooding]

Do you think such a fix would be difficult or a breaking change?

That would likely be a question for @jkoritzinsky and @AaronRobinsonMSFT.

It's also worth noting that the above trick using long only works on Windows. It looks like Linux (and possibly other platforms/architectures) have a slightly different layout: https://godbolt.org/z/h5PT81

[ddobrev]

Indeed, Linux pads at the end instead.
@jkoritzinsky @AaronRobinsonMSFT could you share your opinions, please?

[AaronRobinsonMSFT]

@ddobrev Thanks for bringing this issue to our attention. It is yet another C++ ABI issue that makes life... interesting is perhaps the word. My initial thought would be what @tannergooding suggested above:

There would need to be some feature which explicitly annotates that a given field is the vtbl and therefore should be marshalled differently.

C++ object layout follows C in most cases so defining vanilla non-virtual objects and then marshalling them typically "just works". When an object has a non-virtual member that compatibility is completely broken in my experience. I have observed surprising object layouts and they are valid because the C++ ABI is undefined and compilers can do as they please. I imagine adding inheritance. or worst yet virtual inheritance, to the scenario will further complicate this situation to a point where it is practically impossible to express in a reasonable way.

The above leads me to the conclusion that this isn't something we can do reasonably well. That isn't to say some industrious person couldn't create an algorithm that computes the appropriate object layout for proper marshalling for all supported compilers, but I don't think we would want to encode that in the runtime. Even the annotation mentioned above likely wouldn't always be right because of inheritance or some other aspect of layout that was missed at the time the algorithm was encoded which would then become a feature and expected to remain. This last point is why we are working on tooling to generate marshalling code outside of the runtime during compile time.

I think there are some options here:
1) @EgorBo's suggestion above - https://github.com/dotnet/runtime/issues/44378#issuecomment-723471045
1) Instead of marshalling the object itself, keep the object entirely in native and instead marshal a pointer to it. We have added some optimized memory APIs in .NET Core that would help reading from it.
1) The source generated tooling is expected to have an extensibility model that the aforementioned "industrious person" could leverage and write the algorithm for determining an object's layout.

[ddobrev]

@AaronRobinsonMSFT thank you for your exhaustive answer. We'll use our own manual approaches for the time being until the new generator of marshalling code.