[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1890
 
-  bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
+  llvm::Triple Target = Context.getTargetInfo().getTriple();
+  bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
----------------
I think GCC implements this by ignoring the packed attribute on classes with such non-C++98-pod fields. See this case for example:
https://gcc.godbolt.org/z/fe8x1ne7o
```
class NonPod {
    void method();
    int x;
};
struct __attribute__((packed)) PackedNonPod {
    char a;
    NonPod b;
    char c;
    int d;
};
PackedNonPod gv;

static_assert(sizeof(PackedNonPod) == 4 * 4, "drop packed");
```
-->
```
<source>:7:12: warning: ignoring packed attribute because of unpacked non-POD field 'NonPod PackedNonPod::b'
    7 |     NonPod b;
```

So, in this case, the entire record is unpacked. `d` appears at offset 12, and the overall size is 16. Your code, as I understand it, handles each field individually, which isn't quite the same.

I think the fix is in Sema somewhere to drop the attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117616/new/

https://reviews.llvm.org/D117616