terrafx / terrafx.interop.windows

Interop bindings for Windows.
MIT License
242 stars 31 forks source link

`MEMORY_BASIC_INFORMATION` is not a valid portable definition #352

Closed nike4613 closed 6 months ago

nike4613 commented 1 year ago

The MEMORY_BASIC_INFORMATION definition is not portable between 32- and 64-bit. The PartitionId field is only present in Win64, and causes the layout to be incorrect on 32-bit, where that field does not exist, and there is no padding there.

The C definition:

typedef struct _MEMORY_BASIC_INFORMATION {
    PVOID BaseAddress;
    PVOID AllocationBase;
    DWORD AllocationProtect;
#if defined (_WIN64)
    WORD   PartitionId;
#endif
    SIZE_T RegionSize;
    DWORD State;
    DWORD Protect;
    DWORD Type;
} MEMORY_BASIC_INFORMATION, *PMEMORY_BASIC_INFORMATION;

The *32 and *64 variants also shed light on this problem:

typedef struct _MEMORY_BASIC_INFORMATION32 {
    DWORD BaseAddress;
    DWORD AllocationBase;
    DWORD AllocationProtect;
    DWORD RegionSize;
    DWORD State;
    DWORD Protect;
    DWORD Type;
} MEMORY_BASIC_INFORMATION32, *PMEMORY_BASIC_INFORMATION32;

typedef struct DECLSPEC_ALIGN(16) _MEMORY_BASIC_INFORMATION64 {
    ULONGLONG BaseAddress;
    ULONGLONG AllocationBase;
    DWORD     AllocationProtect;
    DWORD     __alignment1;
    ULONGLONG RegionSize;
    DWORD     State;
    DWORD     Protect;
    DWORD     Type;
    DWORD     __alignment2;
} MEMORY_BASIC_INFORMATION64, *PMEMORY_BASIC_INFORMATION64;

Notice how neither of these variants have PartitionId, though MEMORY_BASIC_INFORMATION64 has an explicit alignment field in its place. For 32-bit targets, MEMORY_BASIC_INFORMATION should be equivalent to MEMORY_BASIC_INFORMATION32, and on 64-bit targets, it should be equivalent to MEMORY_BASIC_INFORMATION64. The current definition is not.

A correct definition would be this:

public unsafe partial struct MEMORY_BASIC_INFORMATION
{
    [NativeTypeName("PVOID")]
    public void* BaseAddress;

    [NativeTypeName("PVOID")]
    public void* AllocationBase;

    [NativeTypeName("DWORD")]
    public uint AllocationProtect;

    // The automatic padding to align RegionSize fills all the space PartitionId would take up,
    // but ensures that this definition is fully portable.

    [NativeTypeName("SIZE_T")]
    public nuint RegionSize;

    [NativeTypeName("DWORD")]
    public uint State;

    [NativeTypeName("DWORD")]
    public uint Protect;

    [NativeTypeName("DWORD")]
    public uint Type;
}
tannergooding commented 1 year ago

This is one of the few types where it is impossible to define something valid in C# without building per architecture.

The closest thing I could do here is to provide nested structs users must use per platform instead. I can't provide say MEMORY_BASIC_INFORMATION32 and MEMORY_BASIC_INFORMATION64 because the SDK defines those themselves and they're used for a different purpose

rickbrew commented 1 year ago

Does TerraFX.Interop.Windows still support 32-bit?

rickbrew commented 1 year ago

If not then it might be worth adding a static constructor like so,

public unsafe partial struct MEMORY_BASIC_INFORMATION
{
    static MEMORY_BASIC_INFORMATION()
    {
        if (sizeof(IntPtr) == 4) throw new PlatformNotSupportedException("Only 64-bit is supported 🥲");
    }
}
tannergooding commented 1 year ago

Does TerraFX.Interop.Windows still support 32-bit?

It doesn't "not support" 32-bit.

For the vast majority of types exposed they will seamlessly work regardless of target platform. There are a small subset of types, such as MEMORY_BASIC_INFORMATION, which do differ in definition between 32-bit and 64-bit such that problems may creep in. I've tried to handle most of the cases I was aware of, but apparently missed this one in particular

tannergooding commented 6 months ago

Going to close this one as "by design".

The vast majority of types have identical layouts and fields between 32-bit and 64-bit. There are a few exceptions such as MEMORY_BASIC_INFORMATION or various types defined by d3d9, shellapi, and setupapi.

32-bit support is rapidly falling out of support for new applications. In .NET, you effectively only have support on Windows and it is no longer the default.

For Linux there is community maintained support for x86 and official support for Arm32, but there's been discussion of dropping the latter with it only being kept for IoT at the moment. There is little to no support for MacOS and for most modern operating systems, there is no longer a 32-bit option even for client SKUs.

As such, almost all computers that support .NET are running 64-bit Operating Systems and Windows is largely the only platform where running 32-bit apps continues to mostly "just work" (particularly without going through complex VM or other hosting mechanisms).

It is not worth the additional effort required to manually maintain these handful of types and worsen the experience for 64-bit application developers just to provide correct support for the minor handful of types that persist. Developers who are in this minor subset can manually define their own bindings for the scenarios that require it.