From 1423a5cfd74eec46580bb5e655e5967b732b8c0d Mon Sep 17 00:00:00 2001
From: Douglas Gregor <dgregor@apple.com>
Date: Fri, 26 Oct 2012 22:31:14 +0000
Subject: [PATCH] When an externally-supplied record layout has a size that
 clearly doesn't include padding up to the alignment of the record, take this
 as a cue that the alignment of the record should (conservatively) be set to
 1. This is similar to other the other cues we use to determine that the
 record has a lower alignment, e.g., that the externally-supplied layout
 places fields at lower offsets than we would. Fixes
 <rdar://problem/12582052>; test case in LLDB.

llvm-svn: 166824
---
 clang/lib/AST/RecordLayoutBuilder.cpp     | 48 ++++++++++++++++-------
 clang/test/CodeGenCXX/override-layout.cpp | 15 ++++++-
 2 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 2bdc1e14e67b..4dfffc45e49c 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1557,6 +1557,13 @@ CharUnits RecordLayoutBuilder::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().RoundUpToAlignment(BaseAlign)){
+      // 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()) {
@@ -1616,7 +1623,6 @@ void RecordLayoutBuilder::InitializeLayout(const Decl *D) {
       if (ExternalLayout) {
         if (ExternalAlign > 0) {
           Alignment = Context.toCharUnitsFromBits(ExternalAlign);
-          UnpackedAlignment = Alignment;
         } else {
           // The external source didn't have alignment information; infer it.
           InferAlignment = true;
@@ -2166,11 +2172,6 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
 }
 
 void RecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
-  if (ExternalLayout) {
-    setSize(ExternalSize);
-    return;
-  }
-  
   // In C++, records cannot be of size 0.
   if (Context.getLangOpts().CPlusPlus && getSizeInBits() == 0) {
     if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
@@ -2184,20 +2185,37 @@ void RecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
       setSize(CharUnits::One());
   }
 
+  // Finally, round the size of the record up to the alignment of the
+  // record itself.
+  uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastByte;
+  uint64_t UnpackedSizeInBits =
+  llvm::RoundUpToAlignment(getSizeInBits(),
+                           Context.toBits(UnpackedAlignment));
+  CharUnits UnpackedSize = Context.toCharUnitsFromBits(UnpackedSizeInBits);
+  uint64_t RoundedSize
+    = llvm::RoundUpToAlignment(getSizeInBits(), Context.toBits(Alignment));
+
+  if (ExternalLayout) {
+    // If we're inferring alignment, and the external size is smaller than
+    // our size after we've rounded up to alignment, conservatively set the
+    // alignment to 1.
+    if (InferAlignment && ExternalSize < RoundedSize) {
+      Alignment = CharUnits::One();
+      InferAlignment = false;
+    }
+    setSize(ExternalSize);
+    return;
+  }
+
+
   // MSVC doesn't round up to the alignment of the record with virtual bases.
   if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
     if (isMicrosoftCXXABI() && RD->getNumVBases())
       return;
   }
 
-  // Finally, round the size of the record up to the alignment of the
-  // record itself.
-  uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastByte;
-  uint64_t UnpackedSizeInBits = 
-    llvm::RoundUpToAlignment(getSizeInBits(), 
-                             Context.toBits(UnpackedAlignment));
-  CharUnits UnpackedSize = Context.toCharUnitsFromBits(UnpackedSizeInBits);
-  setSize(llvm::RoundUpToAlignment(getSizeInBits(), Context.toBits(Alignment)));
+  // Set the size to the final size.
+  setSize(RoundedSize);
 
   unsigned CharBitNum = Context.getTargetInfo().getCharWidth();
   if (const RecordDecl *RD = dyn_cast<RecordDecl>(D)) {
@@ -2255,7 +2273,7 @@ RecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
   if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
     // The externally-supplied field offset is before the field offset we
     // computed. Assume that the structure is packed.
-    Alignment = CharUnits::fromQuantity(1);
+    Alignment = CharUnits::One();
     InferAlignment = false;
   }
   
diff --git a/clang/test/CodeGenCXX/override-layout.cpp b/clang/test/CodeGenCXX/override-layout.cpp
index d432885c5848..aba4c9179a6d 100644
--- a/clang/test/CodeGenCXX/override-layout.cpp
+++ b/clang/test/CodeGenCXX/override-layout.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fdump-record-layouts-simple %s 2> %t.layouts
 // RUN: %clang_cc1 -fdump-record-layouts-simple %s > %t.before 2>&1
 // RUN: %clang_cc1 -DPACKED= -DALIGNED16= -fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after 2>&1
-// RUN: diff %t.before %t.after
+// RUN: diff -u %t.before %t.after
 // RUN: FileCheck %s < %t.after
 
 // If not explicitly disabled, set PACKED to the packed attribute.
@@ -54,11 +54,24 @@ struct PACKED X4 {
   X4();
 };
 
+// CHECK: Type: struct X5
+struct PACKED X5 {
+  union {
+    long a;
+    long b;
+  };
+  short l;
+  short r;
+};
+
 void use_structs() {
   X0 x0s[sizeof(X0)];
   X1 x1s[sizeof(X1)];
   X2 x2s[sizeof(X2)];
   X3 x3s[sizeof(X3)];
   X4 x4s[sizeof(X4)];
+  X5 x5s[sizeof(X5)];
   x4s[1].a = 1;
+  x5s[1].a = 17;
 }
+