r184883 - Don't try to get the layout of an invalid decl in getDeclAlign.

[Matt Beaumont-Gay]

Author: matthewbg
Date: Tue Jun 25 17:19:15 2013
New Revision: 184883

URL: http://llvm.org/viewvc/llvm-project?rev=184883&view=rev
Log:
Don't try to get the layout of an invalid decl in getDeclAlign.

When the decl that we're getting alignment for is a FieldDecl, and the field's
parent record is invalid, skip the actual field alignment calculation (and
return 1-byte alignment in the general case).

Also, assert in in getASTRecordLayout that the decl is valid. This was
inspired by PR16292; see also r184581 and r184751.

Modified:
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/test/SemaCXX/alignof.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=184883&r1=184882&r2=184883&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Jun 25 17:19:15 2013
@@ -1321,24 +1321,27 @@ CharUnits ASTContext::getDeclAlign(const
     // a max-field-alignment constraint (#pragma pack).  So calculate
     // the actual alignment of the field within the struct, and then
     // (as we're expected to) constrain that by the alignment of the type.
-    if (const FieldDecl *field = dyn_cast<FieldDecl>(VD)) {
-      // So calculate the alignment of the field.
-      const ASTRecordLayout &layout = getASTRecordLayout(field->getParent());
+    if (const FieldDecl *Field = dyn_cast<FieldDecl>(VD)) {
+      const RecordDecl *Parent = Field->getParent();
+      // We can only produce a sensible answer if the record is valid.
+      if (!Parent->isInvalidDecl()) {
+        const ASTRecordLayout &Layout = getASTRecordLayout(Parent);
 
-      // Start with the record's overall alignment.
-      unsigned fieldAlign = toBits(layout.getAlignment());
+        // Start with the record's overall alignment.
+        unsigned FieldAlign = toBits(Layout.getAlignment());
 
-      // Use the GCD of that and the offset within the record.
-      uint64_t offset = layout.getFieldOffset(field->getFieldIndex());
-      if (offset > 0) {
-        // Alignment is always a power of 2, so the GCD will be a power of 2,
-        // which means we get to do this crazy thing instead of Euclid's.
-        uint64_t lowBitOfOffset = offset & (~offset + 1);
-        if (lowBitOfOffset < fieldAlign)
-          fieldAlign = static_cast<unsigned>(lowBitOfOffset);
-      }
+        // Use the GCD of that and the offset within the record.
+        uint64_t Offset = Layout.getFieldOffset(Field->getFieldIndex());
+        if (Offset > 0) {
+          // Alignment is always a power of 2, so the GCD will be a power of 2,
+          // which means we get to do this crazy thing instead of Euclid's.
+          uint64_t LowBitOfOffset = Offset & (~Offset + 1);
+          if (LowBitOfOffset < FieldAlign)
+            FieldAlign = static_cast<unsigned>(LowBitOfOffset);
+        }
 
-      Align = std::min(Align, fieldAlign);
+        Align = std::min(Align, FieldAlign);
+      }
     }
   }
 

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=184883&r1=184882&r2=184883&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Jun 25 17:19:15 2013
@@ -2468,6 +2468,7 @@ ASTContext::getASTRecordLayout(const Rec
     
   D = D->getDefinition();
   assert(D && "Cannot get layout of forward declarations!");
+  assert(!D->isInvalidDecl() && "Cannot get layout of invalid decl!");
   assert(D->isCompleteDefinition() && "Cannot layout type before complete!");
 
   // Look up this layout, if already laid out, return what we have.

Modified: cfe/trunk/test/SemaCXX/alignof.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/alignof.cpp?rev=184883&r1=184882&r2=184883&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/alignof.cpp (original)
+++ cfe/trunk/test/SemaCXX/alignof.cpp Tue Jun 25 17:19:15 2013
@@ -9,7 +9,7 @@ struct S0 {
   auto test2() -> char(&)[__alignof__(x)]; // expected-error {{invalid application of 'alignof' to a field of a class still being defined}}
 };
 
-struct S1; // expected-note 5 {{forward declaration}}
+struct S1; // expected-note 6 {{forward declaration}}
 extern S1 s1;
 const int test3 = __alignof__(s1); // expected-error {{invalid application of 'alignof' to an incomplete type 'S1'}}
 
@@ -50,3 +50,11 @@ struct S4 {
   static const int test1 = __alignof__(S0::x);
   auto test2() -> char(&)[__alignof__(x)];
 };
+
+// Regression test for asking for the alignment of a field within an invalid
+// record.
+struct S5 {
+  S1 s;  // expected-error {{incomplete type}}
+  int x;
+};
+const int test8 = __alignof__(S5::x);