translate-c: Translate clang packed struct C into Zig extern struct with align(1)

[Techcable]

trafficstars

This is relevant to the fix and discussion in #12735 (and supersedes it).

@ifreund noticed that align(1) is missing from translate-c output. Hoewever that bug is present with both stage1 & stage2 and is technically independent from that other PR.

~~Until PR #12735 is applied (or some other fix to #12733), packed struct will only work with translate-c on stage1.~~

~~I'm hoping this will help work towards getting packed struct to support extern (but I don't really understand the details too much).~~

EDIT: Zig's notion of packed struct is an integer e C's notion of packed struct is essentially an extern struct with every field set to align(1) (as @ifreund already noticed below).

Relevant Mismatch with Zig Language Spec (things changed)

Since the last stable release (0.9.1), there was a change in the language spec related to the alignment of packed structures.

The docs for Zig 0.9.1 read:

Packed structs have 1-byte alignment.

So the old behavior of translate-c (not specifying any alignment) was possibly correct back then.

However the current docs read:

Packed structs have the same alignment as their backing integer

So now translate-c definitely needs to specify align(1) for every field (unless the field has an explicitly specified alignment)

Unfortunately, the current implementation of translate-c does not reflect this.

EDIT: Also, packed structs are now interpreted simply as wrappers around integers (issue #10113). This means instead of translating __attribute___(packed) struct to be an packed struct it needs to be an extern struct

Running translate-c on the following code:

struct foo {
    char a;
    int b;
} __attribute__((packed));

struct bar {
    char a;
    int b;
    short c;
    __attribute__((aligned(8))) long d;
} __attribute__((packed));

does not apply any alignment attributes to any fields except d.

Currently generated (incorrect) code

Running `zig translate-c -fstage1 test.h` on the above code (need `-fstage1` becuase packed struct support is disabled on stage2).

```zig
pub const struct_foo = packed struct {
    a: u8,
    b: c_int,
};
pub const struct_bar = packed struct {
    a: u8,
    b: c_int,
    c: c_short,
    d: c_long align(8),
};
```

Notice how the `align(1)` specifier is missing 😦
**EDIT**: And these are `packed struct` instead of `extern struct`.

Justification for C behavior of align(1)

This comes from a careful reading of the GCC docs for type & variable attributes.

The final line of the documentation for __attribute__ ((aligned)) [on types] says:

When used on a struct, or struct member, the aligned attribute can only increase the alignment; in order to decrease it, the packed attribute must be specified as well

This implies that GCC uses the packed attribute for alignment purposes in addition to eliminating padding.

The documentation for __attribute__((packed)) [on types], states that "This is equivalent to specifying the packed attribute on each of the members."

The key is resolving this indirection, and looking at the documentation for __attribute__((packed)) [on fields]:

The packed attribute specifies that a structure member should have the smallest possible alignment — one bit for a bit-field and one byte otherwise, unless a larger value is specified with the aligned attribute. The attribute does not apply to non-member objects.

(NOTE: Somewhat confusingly, field attributes are documented under "variables" in GCC...)

This means that a C structure with __attribute__((packed) is shorthand for __attribute__((packed)) on each member, which is equivalent to Zig's align(1) on each of its members.

Fixed Behavior

Sometimes attribute((packed)) is used only for alignment purposes.

In particular, it can be used as a semi-portable way to do unaligned stores

The stdlib.h headers do this on aarch64-macos.12.

The structures in the MacOS headers were being used chiefly for their alignment properties, not for padding purposes (they only had one field).

After applying this change, the translated structures have align(1) explicitly applied to all of their fields (unless explicitly overriden). This makes Zig behavior for tranlsate-c consistent with clang/GCC.

~~Here is the newly produced (correct) output for the above example~~:

EDIT: Changed from packed struct to extern struct.

pub const struct_foo = extern struct {
    a: u8 align(1),
    b: c_int align(1),
};
pub const struct_bar = extern struct {
    a: u8 align(1),
    b: c_int align(1),
    c: c_short align(1),
    d: c_long align(8),
};

This is closely related with PR #12735

EDIT 1: Updated to reflect need to change from packed struct to extern struct (see comment below).

[matu3ba]

This fails with unused variable in debug:

FAILED: CMakeFiles/zigcpp.dir/src/zig_clang.cpp.o 
/deps/local/bin/zig  c++ -target x86_64-linux-musl -mcpu=baseline  -I/deps/local/include -I../deps/SoftFloat-3e/source/include -I../ -I. -I../src -I../src/stage1 -g -std=c++14 -DZIG_LINK_MODE=Static -Werror -Wall -Werror=implicit-fallthrough -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D_GNU_SOURCE -fvisibility-inlines-hidden -fno-exceptions -fno-rtti -Werror=type-limits -Wno-missing-braces -Wno-comment -Wno-deprecated-declarations -MD -MT CMakeFiles/zigcpp.dir/src/zig_clang.cpp.o -MF CMakeFiles/zigcpp.dir/src/zig_clang.cpp.o.d -o CMakeFiles/zigcpp.dir/src/zig_clang.cpp.o -c ../src/zig_clang.cpp
../src/zig_clang.cpp:1980:10: error: unused variable 'casted_ctx' [-Werror,-Wunused-variable]
    auto casted_ctx = const_cast<clang::ASTContext *>(reinterpret_cast<const clang::ASTContext *>(ctx));
         ^
../src/zig_clang.cpp:1990:10: error: unused variable 'casted_ctx' [-Werror,-Wunused-variable]
    auto casted_ctx = const_cast<clang::ASTContext *>(reinterpret_cast<const clang::ASTContext *>(ctx));
         ^
2 errors generated.

[ifreund]

Here is the newly produced (correct) output for the above example:

Packed structs in the stage 2 compiler do not support align() on their fields and are not even structs on the C ABI boundry but rather integers. With the current semantics of the stage2 compiler, we need to translate C structs with the __packed__ attribute as extern structs instead:

pub const struct_foo = extern struct {
    a: u8 align(1),
    b: c_int align(1),
};
pub const struct_bar = extern struct {
    a: u8 align(1),
    b: c_int align(1),
    c: c_short align(1),
    d: c_long align(8),
};

[Techcable]

Here is the newly produced (correct) output for the above example:

Packed structs in the stage 2 compiler do not support align() on their fields and are not even structs on the C ABI boundry but rather integers. With the current semantics of the stage2 compiler, we need to translate C structs with the __packed__ attribute as extern structs instead:

pub const struct_foo = extern struct {
    a: u8 align(1),
    b: c_int align(1),
};
pub const struct_bar = extern struct {
    a: u8 align(1),
    b: c_int align(1),
    c: c_short align(1),
    d: c_long align(8),
};

Interesting. Would it be possible to support all packed structs this way (not having to deal with the "ABI-sized" restriction?)

[Vexu]

The ABI-sized restriction is related to Zig's packed structs which are fancy integers, not C's packed structs which are just differently aligned regular structs.

This is confusing and why IMO Zig's packed structs should not be extern compatible.

[Techcable]

The ABI-sized restriction is related to Zig's packed structs which are fancy integers, not C's packed structs which are just differently aligned regular structs.

Well then I think we should just translate C packed structs, as just extern struct with align(1) like @ifreund suggested.

~~I will close this PR and submit a new one~~

Really I should just update it.

EDIT: It's taken me a lot of comments to get this comment right. No sleep.

[Techcable]

As @ifreund and @Vexu both noticed, Zig packed struct are treated exactly as integers, while instead C packed struct should be translated to a extern struct where every field is align(1).

I've also read the GCC docs again, to try and verify that interpreting C __attribute__(packed) is equivalent to Zig's align(1). Just checking it doesn't imply any sort of other magic.

It appears __attribute__((packed)) on types is only a shorthand for packing every field.

Furthermore, the alignment requirement is the only thing stated about packing a field. This means it should be safe to translate to Zig align(1) . (Though I guess there could be additional platform-specific info the attribute implies, but I guess we'll handle that when we get there....)

I will update the commit with the new translation to extern struct and new reasoning.

(Also thanks Andrew for rebasing onto master.)

[Techcable]

Rebased onto master...

[Techcable]

Alright I've fixed conflicts, removed the stage1 checks and re-enabled the previously disabled tests.

I also added several new tests for translation of packed unions.