[PATCH] D135916: Itanium ABI: Pack non-pod members of packed types

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added reviewers: rnk, abrachet.
Herald added a project: All.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Seems there's a narrow case - where a packed type doesn't pack its base
subobjects (only fields), but when packing a field of the derived type,
GCC does pack the resulting total object - effectively packing the base
subobject.

So ensure that this non-pod type (owing to it having a base class) that
is packed, gets packed when placed in /another/ type that is also
packed.

This is a (smallish?) ABI fix to a regression introduced by D117616 <https://reviews.llvm.org/D117616> -
but that regression/ABI break hasn't been released in LLVM as-yet (it's
been reverted on the release branch from the last two LLVM releases - I
probably should've just reverted the whole patch while we hashed out
this and other issues) so this change isn't itself an ABI break, as far
as LLVM releases are concerned (for folks releasing their own copies of
LLVM from ToT/without the LLVM release branch, and didn't opt into the
clang-abi-compat 14 or below (soon to be 15 or below, I guess I should
say) then this would be an ABI break against clang from the last 9
months or so)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135916

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/SemaCXX/class-layout.cpp


Index: clang/test/SemaCXX/class-layout.cpp
===================================================================
--- clang/test/SemaCXX/class-layout.cpp
+++ clang/test/SemaCXX/class-layout.cpp
@@ -642,3 +642,17 @@
 _Static_assert(_Alignof(t1) == 1, "");
 _Static_assert(_Alignof(t2) == 1, "");
 } // namespace non_pod_packed
+
+namespace non_pod_packed_packed {
+struct B {
+  int b;
+};
+struct  FromB : B {
+} __attribute__((packed));
+struct C {
+  char a[3];
+  FromB b;
+} __attribute__((packed));
+_Static_assert(__builtin_offsetof(C, b) == 3, "");
+}
+
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1891,6 +1891,7 @@
 
   llvm::Triple Target = Context.getTargetInfo().getTriple();
   bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+                                 FieldClass->hasAttr<PackedAttr>() ||
                                  Context.getLangOpts().getClangABICompat() <=
                                      LangOptions::ClangABI::Ver14 ||
                                  Target.isPS() || Target.isOSDarwin())) ||


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135916.467599.patch
Type: text/x-patch
Size: 1209 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221013/27723fb8/attachment.bin>