Packing not emitted for some structs

[sotteson1]

I'm traversing Include\10.0.19041.0\um\minidumpapiset.h, and in it I see:

#include <pshpack4.h>

Some emitted structs have Pack = 4 and some don't like the two below.

    [StructLayout(LayoutKind.Sequential, Pack = 4)]
    public unsafe partial struct MINIDUMP_VM_POST_READ_CALLBACK
    {
        [NativeTypeName("ULONG64")]
        public ulong Offset;

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

        [NativeTypeName("ULONG")]
        public uint Size;

        [NativeTypeName("ULONG")]
        public uint Completed;

        [NativeTypeName("HRESULT")]
        public int Status;
    }

    public partial struct MINIDUMP_CALLBACK_INPUT
    {
        [NativeTypeName("ULONG")]
        public uint ProcessId;

        [NativeTypeName("HANDLE")]
        public IntPtr ProcessHandle;

        [NativeTypeName("ULONG")]
        public uint CallbackType;

        [NativeTypeName("_MINIDUMP_CALLBACK_INPUT::(anonymous union at D:/repos/win32metadata/artifacts/InstalledPackages/Microsoft.Windows.SDK.CPP.10.0.19041.5/c/Include/10.0.19041.0/um/minidumpapiset.h:1141:5)")]
        public _Anonymous_e__Union Anonymous;

        [StructLayout(LayoutKind.Explicit)]
        public partial struct _Anonymous_e__Union
        {
...

I tried rolling my own C versions of these and debugging to see if it makes a difference, and it does. The offsets to CallbackType are different between these two:

typedef struct _MINIDUMP_CALLBACK_INPUT
{
    UINT ProcessId;

    HANDLE ProcessHandle;

    UINT CallbackType;
} MINIDUMP_CALLBACK_INPUT;


#pragma pack(4)
typedef struct _MINIDUMP_CALLBACK_INPUT4
{
    UINT ProcessId;

    HANDLE ProcessHandle;

    UINT CallbackType;
} MINIDUMP_CALLBACK_INPUT4;

[sotteson1]

I debugged it, and clang is giving us the wrong information:

        private void VisitRecordDecl(RecordDecl recordDecl)
        {
...
                var alignment = recordDecl.TypeForDecl.Handle.AlignOf;

Even though though MINIDUMP_VM_POST_READ_CALLBACK and MINIDUMP_CALLBACK_INPUT are right next to each other with no change in packing, alignment is 4 for the former and 16 for the latter.

[tannergooding]

I'm going to recheck this once https://github.com/microsoft/ClangSharp/pull/235 is merged. I'm hoping that this was resolved with Clang 12, but if not I'll look at potential workarounds.

[tannergooding]

@sotteson1, I have a fix for this here: https://github.com/microsoft/ClangSharp/pull/258

[sotteson1]

@sotteson1, I have a fix for this here: #258

Thanks @tannergooding!

[tannergooding]

This is being reopened by https://github.com/microsoft/ClangSharp/pull/263.

The fix ended up regressing a number of key scenarios and so I've started looking into an alternative fix, but said fix will require rebuilding libClangSharp and so needs some more work first.

[tannergooding]

Upon further investigation, it looks like Clang doesn't expose the necessary metadata in any fashion I can discern.

The only way I can determine to correctly detect packing here is to explicitly parse a file as both 32-bit and 64-bit and to compare the specified alignment against the computed "natural" alignment.

[tannergooding]

Closing this. As indicated above, Clang doesn't and cannot surface the relevant metadata.

Much of the time the #pragma pack directives are under an #ifdef in which case they cannot be surfaced without compiling under a scenario where that will be true.

In the cases where it is explicitly specified, but matches the default, Clang doesn't (but could) surface the relevant metadata.

[riverar]

Hey @tannergooding can we revisit this issue?

Consider

#define DECLSPEC_ALIGN(x)   __declspec(align(x))
typedef struct DECLSPEC_ALIGN(16) _BAZ {
    unsigned int Foo;
} BAZ;

This produces an AST of

TranslationUnitDecl
|-CXXRecordDecl <line:2:9, line:4:1> line:2:35 struct _BAZ definition
| |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
| | |-DefaultConstructor exists trivial needs_implicit
| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| |-AlignedAttr <line:1:40, col:47> align
| | `-ConstantExpr <line:2:31> 'int'
| |   |-value: Int 16
| |   `-IntegerLiteral <col:31> 'int' 16
| |-CXXRecordDecl <col:9, col:35> col:35 implicit struct _BAZ
| `-FieldDecl <line:3:5, col:18> col:18 Foo 'unsigned int'
`-TypedefDecl <line:2:1, line:4:3> col:3 BAZ 'struct _BAZ':'_BAZ'
  `-ElaboratedType 'struct _BAZ' sugar
    `-RecordType '_BAZ'
      `-CXXRecord '_BAZ'

(godbolt)

I searched through the ClangSharp codebase for AlignedAttr but didn't see much. Is this something we can add support for?

[riverar]

In hindsight, alignment != packing and I see you already attempted a solution with MaxFieldAlignmentAttr. So never mind.