type with packed pragma is not properly packed

[krux02]

I currently try to implement offsetof, alignof, sizeof as a compile time feature of my compiler (#5664), and in my tests I found out, that the original nim code generation does not properly pack its types.

This is the type I use to test the correctness of my offsetof implementation:

type
  MyEnum {.pure.} = enum
    ValueA
    ValueB
    ValueC

  RecursiveStuff {.packed.} = object
    case kind: MyEnum
    of MyEnum.ValueA:
      a: int16
    of MyEnum.ValueB:
      b: int32
    of MyEnum.ValueC:
      case kind2: MyEnum
      of MyEnum.ValueA:
        ca1: int8
        ca2: int32
      of MyEnum.ValueB:
        cb: int32
      of MyEnum.ValueC:
        cc: int64
      d1: int8
      d2: int64

This test shows the original generated C coder for this exact type:

#include <stdint.h>
#include <stddef.h>
#include <stdio.h>

typedef int8_t  NI8;
typedef int16_t NI16;
typedef int32_t NI32;
typedef int64_t NI64;
typedef uint8_t MyEnum_uQexRw9c9cTrab31ooVCph5g;

/* this is the original type generated by the Nim compiler */
/* the __packed__ attribute is at the top level struct, none of it's members has it */

struct __attribute__((__packed__)) RecursiveStuff {
MyEnum_uQexRw9c9cTrab31ooVCph5g kind;
union{
struct {NI16 a;
} S1;
struct {NI32 b;
} S2;
struct {MyEnum_uQexRw9c9cTrab31ooVCph5g kind2;
union{
struct {NI8 ca1;
NI32 ca2;
} S1;
struct {NI32 cb;
} S2;
struct {NI64 cc;
} S3;
} kind2U;
NI8 d1;
NI64 d2;
} S3;
} kindU;
};

void f1() {
  printf("\n\nf1:\n\n");
  printf("% 4ld  kind\n",                   offsetof(struct RecursiveStuff, kind));
  printf("% 4ld  kindU\n",                  offsetof(struct RecursiveStuff, kindU));
  printf("% 4ld  kindU.S1\n",               offsetof(struct RecursiveStuff, kindU.S1));
  printf("% 4ld  kindU.S1.a1\n",            offsetof(struct RecursiveStuff, kindU.S1.a));
  printf("% 4ld  kindU.S2\n",               offsetof(struct RecursiveStuff, kindU.S2));
  printf("% 4ld  kindU.S3\n",               offsetof(struct RecursiveStuff, kindU.S3));
  printf("% 4ld  kindU.S3.kind2\n",         offsetof(struct RecursiveStuff, kindU.S3.kind2));
  printf("% 4ld  kindU.S3.kind2U\n",        offsetof(struct RecursiveStuff, kindU.S3.kind2U));
  printf("% 4ld  kindU.S3.d1\n",            offsetof(struct RecursiveStuff, kindU.S3.d1));
  printf("% 4ld  kindU.S3.d2\n",            offsetof(struct RecursiveStuff, kindU.S3.d2));
  printf("% 4ld  kindU.S3.kind2U.S1.ca1\n", offsetof(struct RecursiveStuff, kindU.S3.kind2U.S1.ca1));
  printf("% 4ld  kindU.S3.kind2U.S1.ca2\n", offsetof(struct RecursiveStuff, kindU.S3.kind2U.S1.ca2));
}

/* this is a manually modified version of the RecursiveStuff struct, so that all */
/* anonymous member structs also have the __packed__ attribute */

struct __attribute__((__packed__)) RecursiveStuffFixed {
MyEnum_uQexRw9c9cTrab31ooVCph5g kind;
union{
struct __attribute__((__packed__)) {NI16 a;
} S1;
struct __attribute__((__packed__)) {NI32 b;
} S2;
struct __attribute__((__packed__)) {MyEnum_uQexRw9c9cTrab31ooVCph5g kind2;
union{
struct __attribute__((__packed__)) {NI8 ca1;
NI32 ca2;
} S1;
struct __attribute__((__packed__)) {NI32 cb;
} S2;
struct __attribute__((__packed__)) {NI64 cc;
} S3;
} kind2U;
NI8 d1;
NI64 d2;
} S3;
} kindU;
};

void f2() {
  printf("\n\nf2:\n\n");
  printf("% 4ld  kind\n",                   offsetof(struct RecursiveStuffFixed, kind));
  printf("% 4ld  kindU\n",                  offsetof(struct RecursiveStuffFixed, kindU));
  printf("% 4ld  kindU.S1\n",               offsetof(struct RecursiveStuffFixed, kindU.S1));
  printf("% 4ld  kindU.S1.a1\n",            offsetof(struct RecursiveStuffFixed, kindU.S1.a));
  printf("% 4ld  kindU.S2\n",               offsetof(struct RecursiveStuffFixed, kindU.S2));
  printf("% 4ld  kindU.S3\n",               offsetof(struct RecursiveStuffFixed, kindU.S3));
  printf("% 4ld  kindU.S3.kind2\n",         offsetof(struct RecursiveStuffFixed, kindU.S3.kind2));
  printf("% 4ld  kindU.S3.kind2U\n",        offsetof(struct RecursiveStuffFixed, kindU.S3.kind2U));
  printf("% 4ld  kindU.S3.d1\n",            offsetof(struct RecursiveStuffFixed, kindU.S3.d1));
  printf("% 4ld  kindU.S3.d2\n",            offsetof(struct RecursiveStuffFixed, kindU.S3.d2));
  printf("% 4ld  kindU.S3.kind2U.S1.ca1\n", offsetof(struct RecursiveStuffFixed, kindU.S3.kind2U.S1.ca1));
  printf("% 4ld  kindU.S3.kind2U.S1.ca2\n", offsetof(struct RecursiveStuffFixed, kindU.S3.kind2U.S1.ca2));
}

int main() {
  f1();
  f2();
}

annotated output:

clang test.c && ./a.out


f1:

   0  kind
   1  kindU
   1  kindU.S1
   1  kindU.S1.a1
   1  kindU.S2
   1  kindU.S3
   1  kindU.S3.kind2
   9  kindU.S3.kind2U  // kind2 is of type uint8_t, this should not be with offset 9
  17  kindU.S3.d1
  25  kindU.S3.d2
   9  kindU.S3.kind2U.S1.ca1
  13  kindU.S3.kind2U.S1.ca2


f2:

   0  kind
   1  kindU
   1  kindU.S1
   1  kindU.S1.a1
   1  kindU.S2
   1  kindU.S3
   1  kindU.S3.kind2
   2  kindU.S3.kind2U
  10  kindU.S3.d1               // d1 actually has size one
  11  kindU.S3.d2               // therefore the offset of d2 should be the offset of d1 plus one
   2  kindU.S3.kind2U.S1.ca1    // ca1 actually has size one
   3  kindU.S3.kind2U.S1.ca2    // therefore the offset of ca2 should be the offset of ca1 plus one

As you can see, only when the packed attribute is actually propagated to all implicit subtypes it can correctly handle the entire object type.

[markus-oberhumer]

I think this is a syntax issue - the "attribute" must come last when using gcc:

struct Foo { char a; int b; } __attribute__((__packed__));

[krux02]

@markus-oberhumer
I just tested it. there is no difference if I put the attribute last, ore where I have put it. So this is not a problem.

[markus-oberhumer]

@krux02 You are right - but I recall there was a syntax problem with some older gcc version...

Looking at your source once more:

I don't think that __attribute__((__packed__)) applies recursively (at a minimum this is poorly specified), so indeed the C codegen should emit "packed" for internal unions/structs as well.

[krux02]

The packed attribute for unions doesn't really make a lot of sense to me. All members of a union have the relative offset of 0, thererefore there is not much it could align.

One thing I should note is that the most outer struct does not align the members, but the inner structs to it, but only relative to their size. In other words they add padding bytes so that in case when the inner struct would be allocated aligned in memory the members would be aligned. Just that the inner members are not allocated aligned in memory. Therefore nothing is aligned and the padding bytes are just pure waste.

[markus-oberhumer]

Yes, but all this "packed" handling is historically somewhat unstable/buggy ( https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=attribute%20packed lists 30 open and several hundred closed bugs ), so it probably helps if Nim's codegen does emit as many __attribute__((__packed__)) as possible.