io_src offsets wrong for __attribute__((packed)) structs on Ubuntu 20.04

[dbankieris]

In the struct below, with natural alignment, on a 64-bit machine where words are 8 bytes wide, the double will be placed at offset 8 despite the int being only 4 bytes. This allows b to occupy a single word and be read with only one instruction.

struct Foo {
    int    a = 0; // offset = 0
    double b = 1; // offset = 8 (on a 64-bit machine)
}; 

gdb confirms the size and offsets.

(gdb) p sizeof(Foo)
$6 = 16
(gdb) p &sandbox.foo.a
$7 = (int *) 0xa28a58 <sandbox+120>
(gdb) p &sandbox.foo.b
$8 = (double *) 0xa28a60 <sandbox+128>

The io_src code has the correct offsets (first entry on the fourth line).

ATTRIBUTES attrFoo[] = { 
{"a", "int", "1", "", "", 
  "", 
  15,TRICK_INTEGER, sizeof(int), 0, 0, Language_CPP, 0,
  0, NULL, 0, {{0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}},
  NULL, NULL, NULL, NULL},
{"b", "double", "1", "", "", 
  "", 
  15,TRICK_DOUBLE, sizeof(double), 0, 0, Language_CPP, 0,
  8, NULL, 0, {{0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}},
  NULL, NULL, NULL, NULL},

__attribute__((packed)) allows us to compress the struct.

struct Foo {
    int    a = 0; // offset = 0
    double b = 1; // offset = 4 (now spans two words)
} __attribute__((packed)); 

gdb confirms that the struct is 4 bytes smaller and the double has moved by the same amount.

(gdb) p sizeof(Foo)
$1 = 12
(gdb) p &sandbox.foo.a
$2 = (int *) 0xa28a58 <sandbox+120>
(gdb) p &sandbox.foo.b
$3 = (double *) 0xa28a5c <sandbox+124>

However, the io_src code on Ubuntu 20.04 using clang 10.0.0-4ubuntu1 is unchanged, and the value in TrickView is wrong.

Setting TRICK_CXX=clang++ produces the same behavior, and lldb agrees with gdb about the memory layout. Adding some printouts to FieldVisitor::VisitFieldDecl shows that the offset being returned from clang is wrong. This problem does not occur on CentOS 7 using clang 3.4.2.

Quite by accident, I discovered an apparent workaround of removing the trailing underscores from __attribute. I can't find any documentation about it, but both gdb and clang appear to treat the trailing underscores as optional, and it fixes the problem on Ubuntu.

Replacing setIgnoreAllWarnings(true) with setEnableAllWarnings(true) reveals that clang is indeed padding Foo despite __attribute__((packed)).

In file included from /home/dbankier/SIM_test/S_source.hh:27:
/home/dbankier/SIM_test/models/Foo.hh:5:12: warning: padding struct 'Foo' with 4 bytes to align 'b'
    double b = 1;

However, with __attribute((packed)), it correctly packs Foo, instead padding Sandbox, which contains a Foo.

/home/dbankier/SIM_test/S_source.hh:851:7: warning: padding size of 'Sandbox' with 4 bytes to alignment boundary
class Sandbox : public Trick::SimObject {

@alexlin0 @spfennell My guess is there's some option we're missing when we configure clang within ICG.

[tfleming-osr]

Any chance you are doing something like #define __attribute__(x) in a header that's only getting pulled up on the Ubuntu box? Because that would be one reason that __attribute(packed) would work but the first form would not.

[dbankieris]

I am not, and I don't think Trick is either. It's only within ICG's invocation of clang that __attribute__((packed)) isn't working. When the sim is actually compiled (with gcc or clang), the struct is correctly packed, and the resulting offsets differ from what ICG recorded, hence the issue!

[alexlin0]

I've recreated your results where attribute does not work but __attribute does. Interesting.

[miltondean]

I seem to recall we had this same issue with using attribute((packed)) at the end for structs many years ago. We found that if you declared it this way, trick would correctly handle the packed structure. So we have just always defined it this way in our project. typedef struct attribute((packed)) structnamewhatever { ... } structnamewhatever_t;

[dbankieris]

This was apparently fixed by 3cd2a160. My guess is that it had something to do with the GNUCVERSION. A unit test comparing the information returned by & and offsetof to what's in the Memory Manager might be helpful in the future.

[shaferandrew]

Here's a minimum working example to see if the bug exists:

S_define

#include "sim_objects/default_trick_sys.sm"

class TrickTestSimObject : public Trick::SimObject {
  public:
    // a offset = 0, b offset = 8 (on 64-bit machine)
    struct FooNatural {
        int a{0};
        double b{1};
    };

    // a offset = 0, b offset = 4 (crosses a word boundary)
    struct FooPacked {
        int a{0};
        double b{1};
    } __attribute__((packed));

    FooNatural fooNatural;
    FooPacked fooPacked;

    void TrickTestSimInit() {
        std::cout << "Address of fooNatural.a: " << &fooNatural.a << " Trick Address: "
                  << trick_MM->ref_attributes("myTrickTestSimObject.fooNatural.a")->address << "\n";
        std::cout << "Address of fooNatural.b: " << &fooNatural.b << " Trick Address: "
                  << trick_MM->ref_attributes("myTrickTestSimObject.fooNatural.b")->address << "\n";
        std::cout << "Address of  fooPacked.a: " << &fooPacked.a << " Trick Address: "
                  << trick_MM->ref_attributes("myTrickTestSimObject.fooPacked.a")->address << "\n";
        std::cout << "Address of  fooPacked.b: " << &fooPacked.b << " Trick Address: "
                  << trick_MM->ref_attributes("myTrickTestSimObject.fooPacked.b")->address << "\n";
        exit(0);
    }

    TrickTestSimObject() {
        ("initialization") TrickTestSimInit();
    }
};

TrickTestSimObject myTrickTestSimObject;

This bug is confirmed to exist for Ubuntu 20.04/LLVM 10 in 19.2.0 -> 19.3.1. Builds before 19.2.0 do not support Ubuntu 20.04/LLVM 10. Builds after 19.4.0 do NOT have the bug.