[clang] 29c9287 - [AST] Fix class layout when using external layout under MS ABI.

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2022-11-16T11:17:50-08:00
New Revision: 29c92879171ddce944b177127901b8140e0a1ebf

URL: https://github.com/llvm/llvm-project/commit/29c92879171ddce944b177127901b8140e0a1ebf
DIFF: https://github.com/llvm/llvm-project/commit/29c92879171ddce944b177127901b8140e0a1ebf.diff

LOG: [AST] Fix class layout when using external layout under MS ABI.

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D137806

Added: 
    

Modified: 
    clang/lib/AST/RecordLayoutBuilder.cpp
    clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
    clang/test/CodeGenCXX/override-layout-virtual-base.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 93ade76364b42..781d4f4b3fd28 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2027,7 +2027,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
 
   // The align if the field is not packed. This is to check if the attribute
   // was unnecessary (-Wpacked).
-  CharUnits UnpackedFieldAlign = FieldAlign; 
+  CharUnits UnpackedFieldAlign = FieldAlign;
   CharUnits PackedFieldAlign = CharUnits::One();
   CharUnits UnpackedFieldOffset = FieldOffset;
   CharUnits OriginalFieldAlign = UnpackedFieldAlign;
@@ -3081,10 +3081,9 @@ void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) {
     VBPtrOffset += Offset;
 
   if (UseExternalLayout) {
-    // The class may have no bases or fields, but still have a vfptr
-    // (e.g. it's an interface class). The size was not correctly set before
-    // in this case.
-    if (FieldOffsets.empty() && Bases.empty())
+    // The class may have size 0 and a vfptr (e.g. it's an interface class). The
+    // size was not correctly set before in this case.
+    if (Size.isZero())
       Size += Offset;
     return;
   }

diff  --git a/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout b/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
index 71d88c1e603ae..d9fa1d3af9ca2 100644
--- a/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
+++ b/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
@@ -6,3 +6,11 @@ Layout: <ASTRecordLayout
   Size:64
   Alignment:64
   FieldOffsets: []>
+
+*** Dumping AST Record Layout
+Type: struct S5
+
+Layout: <ASTRecordLayout
+  Size:64
+  Alignment:64
+  FieldOffsets: []>

diff  --git a/clang/test/CodeGenCXX/override-layout-virtual-base.cpp b/clang/test/CodeGenCXX/override-layout-virtual-base.cpp
index d9e7346737d09..12ec361d7c68e 100644
--- a/clang/test/CodeGenCXX/override-layout-virtual-base.cpp
+++ b/clang/test/CodeGenCXX/override-layout-virtual-base.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck --check-prefix=SIMPLE  %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
 
 struct S1 {
   int a;
@@ -8,14 +9,48 @@ struct S2 : virtual S1 {
   virtual void foo() {}
 };
 
-// CHECK: Type: struct S3
-// CHECK:   FieldOffsets: [128]
+// SIMPLE: Type: struct S3
+// SIMPLE:   FieldOffsets: [64]
 struct S3 : S2 {
   char b;
 };
 
+struct S4 {
+};
+
+struct S5 : S4 {
+  virtual void foo() {}
+};
+
+// CHECK:      *** Dumping AST Record Layout
+// CHECK:               0 | struct S2
+// CHECK-NEXT:          0 |   (S2 vftable pointer)
+// CHECK-NEXT:          8 |   (S2 vbtable pointer)
+// CHECK-NEXT:          8 |   struct S1 (virtual base)
+// CHECK-NEXT:          8 |     int a
+// CHECK-NEXT:            | [sizeof=8, align=8,
+// CHECK-NEXT:            |  nvsize=8, nvalign=8]
+// CHECK:      *** Dumping AST Record Layout
+// CHECK:               0 | struct S3
+// CHECK-NEXT:          0 |   struct S2 (primary base)
+// CHECK-NEXT:          0 |     (S2 vftable pointer)
+// CHECK-NEXT:          8 |     (S2 vbtable pointer)
+// CHECK-NEXT:          8 |   char b
+// CHECK-NEXT:         16 |   struct S1 (virtual base)
+// CHECK-NEXT:         16 |     int a
+// CHECK-NEXT:            | [sizeof=24, align=8,
+// CHECK-NEXT:            |  nvsize=16, nvalign=8]
+// CHECK:      *** Dumping AST Record Layout
+// CHECK:               0 | struct S5
+// CHECK-NEXT:          0 |   (S5 vftable pointer)
+// CHECK-NEXT:          0 |   struct S4 (base) (empty)
+// CHECK-NEXT:            | [sizeof=8, align=8,
+// CHECK-NEXT:            |  nvsize=8, nvalign=8]
+
 void use_structs() {
   S1 s1s[sizeof(S1)];
   S2 s2s[sizeof(S2)];
   S3 s3s[sizeof(S3)];
+  S4 s4s[sizeof(S4)];
+  S5 s5s[sizeof(S5)];
 }