arocc icon indicating copy to clipboard operation
arocc copied to clipboard

Calculate record size and alignment

Open Vexu opened this issue 4 years ago • 9 comments

Currently both just default to 1.

Note: the size calculation should happen after the attributes after the type are parsed since they may contain a packed attribute.

Vexu avatar Dec 08 '21 13:12 Vexu

There's a rust crate that does this, which is MIT licensed, which could possibly be used as inspiration: https://github.com/mahkoh/repr-c/tree/master/repc/facade

Should this be its own project? Seems big enough to be, plus it could also be used by the Zig compiler when it needs to know C ABI layout independently of translate-c

ehaas avatar Dec 08 '21 18:12 ehaas

There's a rust crate that does this, which is MIT licensed, which could possibly be used as inspiration: https://github.com/mahkoh/repr-c/tree/master/repc/facade

That does seem like the best place to start.

Should this be its own project? Seems big enough to be, plus it could also be used by the Zig compiler when it needs to know C ABI layout independently of translate-c

I guess it could be, though if the Zig compiler needs it it will have access to it via the Aro dependency anyways.

Vexu avatar Dec 08 '21 18:12 Vexu

I started poking around w/ this (unsolicited. sorry if this is redundant).

The rust repro keeps in tests in one DSL and the results in a different DSL. To generate the reference compiler version, it creates C from the input DSL then runs it through the compilers and converts the compilers debug output (DWARF) to a DSL. The resulting compiler DSL's are checked into the repro.

I modified the generation code to just output the C files. I put them in this repro https://github.com/TwoClocks/repr-c-test-files.

AROCC compiles about 1/2 of them. It currently doesn't parse __attribute__, which are the majority of the errors.

The bulk of the rest look compiler-specific.

I'm not sure what the plan for AROCC is regarding the intended C compiler (this is different from ABI?).

Looking at the repr-c code, it seems like you can do a fair bit of layout correctly w/o knowing the intended compiler. But for anything non-vanilla, the intended compiler must be known.

At a casual glance, more info is needed to do layout correctly than arocc currently has in it's Type. Or I'm misreading the code (this is likely).

TwoClocks avatar Jan 23 '22 00:01 TwoClocks

I'm not sure what the plan for AROCC is regarding the intended C compiler (this is different from ABI?).

One of the main goals of Aro is to replace clang as the C frontend of Zig's translate-c feature and eventually even being an alternative frontend for zig cc so by default it should be clang compatible (which should be gcc compatible).

At a casual glance, more info is needed to do layout correctly than arocc currently has in it's Type. Or I'm misreading the code (this is likely).

We now have type sizes, packed and alignment attributes which be enough to get started.

Vexu avatar Jan 24 '22 18:01 Vexu

Mahkoh's rust layout seems to imply that clang and gcc layout bitfields differently in some circumstances. As seen in this code snippit :

mahkoh/repr-c

For alignment, it doesn't seem like much more work to support the gcc/clang edge cases. At a minimum, it seems easy to detect an edge case and warn the user. Not sure if you want to add a flag for the intended compiler or not.

We now have type sizes, packed and alignment attributes which be enough to get started.

If you have them for both the record and the individual fields, then I think you have everything Mahkoh's layout code expects.

Is there anything I can do that would be helpful?

TwoClocks avatar Jan 25 '22 00:01 TwoClocks

Mahkoh's rust layout seems to imply that clang and gcc layout bitfields differently in some circumstances. As seen in this code snippit :

That does seem to imply something but I couldn't find an actual test case that demonstrated that behavior.

Not sure if you want to add a flag for the intended compiler or not.

We do already have a target which includes ABI that can be used to detect GNU or MSVC but if that bitfield thing is different for clang then we'll need to also add a target compiler flag.

Is there anything I can do that would be helpful?

If you want you can start implementing this, I don't think anyone else is actively working on it.

Vexu avatar Jan 26 '22 11:01 Vexu

I'd be happy to take a swing at it. I've poked around Aro a bit. Where in the code would you see this happening?

TwoClocks avatar Jan 26 '22 17:01 TwoClocks

It'd probably be best to add a new file which does record layout calculations and call it from recordSpec in Parser.zig where there currently is a TODO comment.

Vexu avatar Jan 26 '22 20:01 Vexu

That does seem to imply something but I couldn't find an actual test case that demonstrated that behavior.

Here's an example:

#pragma pack(1)
typedef struct {
    char c:1;
    char d:1 __attribute__((aligned(2))) __attribute__((packed));
    char e:1;
} A006;
_Static_assert(sizeof(A006) == 1, "wrong size");

This code will trip the _Static_assert in x86_64 gcc because the struct size is 2. In clang with -target x86_64-linux-gnu the struct size is 1, but it does print a warning about packed attribute being ignored on bit-fields with single-byte alignment in older versions of GCC and Clang

ehaas avatar Jan 27 '22 04:01 ehaas

Implemented in 1c85563bc4f8ac6465a1aa44f57b8ee5d2693e65

Vexu avatar Aug 04 '22 16:08 Vexu