[clang][RecordLayoutBuilder] Be stricter about inferring packed-ness in ExternalLayouts

This patch is motivated by the LLDB support required for:
#93069

In the presence of `[[no_unique_address]]`, LLDB may ask Clang
to lay out types with overlapping field offsets. Because we don't
have attributes such as `packed` or `no_unique_address` in the LLDB
AST, the `RecordLayoutBuilder` supports an `InferAlignment` mode, which,
in the past, attempted to detect layouts which came from packed
structures in order to provide a conservative estimate of alignment for
it (since `DW_AT_alignment` isn't emitted unless explicitly changed with
`alignas`, etc.).

However, in the presence of overlapping fields due to `no_unique_address`,
`InferAlignment` would set the alignment of structures to `1` for which
that's incorrect. This poses issues in some LLDB formatters that
synthesize new Clang types and rely on the layout builder to get the
`FieldOffset` of structures right that we did have DWARF offsets for.
The result of this is that if we get the alignment wrong, LLDB reads
out garbage data from the wrong field offsets.

There are a couple of solutions to this that we considered:
1. Make LLDB formatters not do the above, and make them more robust
   to inaccurate alignment.
2. Remove `InferAlignment` entirely and rely on Clang emitting
   `DW_AT_alignment` for packed structures.
3. Remove `InferAlignment` and detect packedness from within LLDB.
4. Make the `InferAlignment` logic account for overlapping fields.

Option (1) turned out quite hairy and it's not clear we can achieve
this with the tools available for certain STL formatters (particularly
`std::map`). But I would still very much like to simplify this if we
can.

Option (2) wouldn't help with GCC-compiled binaries, and if we can
get away with LLDB not needing the alignment, then we wouldn't need
to increase debug-info size.

Option (3), AFAICT, would require us to reimplement some of the layout
logic in the layout builder. Would be great if we can avoid this added
complexity.

Option (4) seemed like the best option in the interim. As part of this
change I also removed one of the `InferAlignment` blocks. The test-cases
associated with this code-path pass regardless, and from the description
of the change that introduced it it's not clear why specifically the
base offsets would influence the `Alignment` field, and how it would
imply packedness. But happy to be proven wrong. Ultimately it would be
great if we can get rid of the `InferAlignment` infrastructure and
support our use-cases in LLDB or DWARF instead.

Author: Michael137

clang/lib/AST/RecordLayoutBuilder.cpp

@@ -802,7 +802,8 @@ class ItaniumRecordLayoutBuilder {
   /// \param Field The field whose offset is being queried.
   /// \param ComputedOffset The offset that we've computed for this field.
   uint64_t updateExternalFieldOffset(const FieldDecl *Field,
-                                     uint64_t ComputedOffset);
+                                     uint64_t ComputedOffset,
+                                     uint64_t PreviousOffset);
 
   void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset,
                           uint64_t UnpackedOffset, unsigned UnpackedAlign,
@@ -1296,13 +1297,6 @@ ItaniumRecordLayoutBuilder::LayoutBase(const BaseSubobjectInfo *Base) {
     bool Allowed = EmptySubobjects->CanPlaceBaseAtOffset(Base, Offset);
     (void)Allowed;
     assert(Allowed && "Base subobject externally placed at overlapping offset");
-
-    if (InferAlignment && Offset < getDataSize().alignTo(AlignTo)) {
-      // The externally-supplied base offset is before the base offset we
-      // computed. Assume that the structure is packed.
-      Alignment = CharUnits::One();
-      InferAlignment = false;
-    }
   }
 
   if (!Base->Class->isEmpty()) {
@@ -1770,7 +1764,8 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
   // If we're using external layout, give the external layout a chance
   // to override this information.
   if (UseExternalLayout)
-    FieldOffset = updateExternalFieldOffset(D, FieldOffset);
+    FieldOffset = updateExternalFieldOffset(
+        D, FieldOffset, FieldOffsets.empty() ? 0 : FieldOffsets.back());
 
   // Okay, place the bitfield at the calculated offset.
   FieldOffsets.push_back(FieldOffset);
@@ -2063,8 +2058,9 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
   UnpackedFieldOffset = UnpackedFieldOffset.alignTo(UnpackedFieldAlign);
 
   if (UseExternalLayout) {
-    FieldOffset = Context.toCharUnitsFromBits(
-        updateExternalFieldOffset(D, Context.toBits(FieldOffset)));
+    FieldOffset = Context.toCharUnitsFromBits(updateExternalFieldOffset(
+        D, Context.toBits(FieldOffset),
+        FieldOffsets.empty() ? 0 : FieldOffsets.back()));
 
     if (!IsUnion && EmptySubobjects) {
       // Record the fact that we're placing a field at this offset.
@@ -2250,14 +2246,18 @@ void ItaniumRecordLayoutBuilder::UpdateAlignment(
   }
 }
 
-uint64_t
-ItaniumRecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
-                                                      uint64_t ComputedOffset) {
+uint64_t ItaniumRecordLayoutBuilder::updateExternalFieldOffset(
+    const FieldDecl *Field, uint64_t ComputedOffset, uint64_t PreviousOffset) {
   uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);
 
-  if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
-    // The externally-supplied field offset is before the field offset we
-    // computed. Assume that the structure is packed.
+  // If the externally-supplied field offset is before the field offset we
+  // computed. Check against the previous field offset to make sure we don't
+  // misinterpret overlapping fields as packedness of the structure.
+  const bool assume_packed = ExternalFieldOffset > 0 &&
+                             ExternalFieldOffset < ComputedOffset &&
+                             ExternalFieldOffset > PreviousOffset;
+
+  if (InferAlignment && assume_packed) {
     Alignment = CharUnits::One();
     PreferredAlignment = CharUnits::One();
     InferAlignment = false;

lldb/test/Shell/SymbolFile/DWARF/x86/no_unique_address-alignment.cpp

@@ -1,5 +1,3 @@
-// XFAIL: *
-
 // RUN: %clang --target=x86_64-apple-macosx -c -gdwarf -o %t %s
 // RUN: %lldb %t \
 // RUN:   -o "expr alignof(OverlappingFields)" \

lldb/test/Shell/SymbolFile/DWARF/x86/no_unique_address-base-alignment.cpp

@@ -1,5 +1,3 @@
-// XFAIL: *
-
 // RUN: %clang --target=x86_64-apple-macosx -c -gdwarf -o %t %s
 // RUN: %lldb %t \
 // RUN:   -o "expr alignof(OverlappingDerived)" \