From f3e86a7a550542940772981ed5e2e3d8630594b4 Mon Sep 17 00:00:00 2001
From: John McCall <rjmccall@apple.com>
Date: Tue, 28 Apr 2015 00:17:18 +0000
Subject: [PATCH] ms_struct does not imply the MS base-layout ABI; separate
 these conditions in the IRGen struct layout code.

rdar://20636558

llvm-svn: 235949
---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 29 ++++++++++++++----
 clang/test/CodeGenCXX/ms_struct.cpp         | 33 +++++++++++++++++++++
 2 files changed, 56 insertions(+), 6 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/ms_struct.cpp

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index c33d2d9f86d3..cebe38877ac4 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -99,10 +99,25 @@ struct CGRecordLowering {
   MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
     return MemberInfo(Offset, MemberInfo::Field, Data);
   }
-  bool useMSABI() {
+
+  /// The Microsoft bitfield layout rule allocates discrete storage
+  /// units of the field's formal type and only combines adjacent
+  /// fields of the same formal type.  We want to emit a layout with
+  /// these discrete storage units instead of combining them into a
+  /// continuous run.
+  bool isDiscreteBitFieldABI() {
     return Context.getTargetInfo().getCXXABI().isMicrosoft() ||
            D->isMsStruct(Context);
   }
+
+  /// The Itanium base layout rule allows virtual bases to overlap
+  /// other bases, which complicates layout in specific ways.
+  ///
+  /// Note specifically that the ms_struct attribute doesn't change this.
+  bool isOverlappingVBaseABI() {
+    return !Context.getTargetInfo().getCXXABI().isMicrosoft();
+  }
+
   /// \brief Wraps llvm::Type::getIntNTy with some implicit arguments.
   llvm::Type *getIntNType(uint64_t NumBits) {
     return llvm::Type::getIntNTy(Types.getLLVMContext(),
@@ -119,8 +134,9 @@ struct CGRecordLowering {
   /// for itanium bitfields that are smaller than their declared type.
   llvm::Type *getStorageType(const FieldDecl *FD) {
     llvm::Type *Type = Types.ConvertTypeForMem(FD->getType());
-    return useMSABI() || !FD->isBitField() ? Type :
-        getIntNType(std::min(FD->getBitWidthValue(Context),
+    if (!FD->isBitField()) return Type;
+    if (isDiscreteBitFieldABI()) return Type;
+    return getIntNType(std::min(FD->getBitWidthValue(Context),
                              (unsigned)Context.toBits(getSize(Type))));
   }
   /// \brief Gets the llvm Basesubobject type from a CXXRecordDecl.
@@ -365,7 +381,7 @@ CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
   // used to determine if the ASTRecordLayout is treating these two bitfields as
   // contiguous.  StartBitOffset is offset of the beginning of the Run.
   uint64_t StartBitOffset, Tail = 0;
-  if (useMSABI()) {
+  if (isDiscreteBitFieldABI()) {
     for (; Field != FieldEnd; ++Field) {
       uint64_t BitOffset = getFieldBitOffset(*Field);
       // Zero-width bitfields end runs.
@@ -465,7 +481,7 @@ void CGRecordLowering::accumulateVBases() {
   // smaller than the nvsize.  Here we check to see if such a base is placed
   // before the nvsize and set the scissor offset to that, instead of the
   // nvsize.
-  if (!useMSABI())
+  if (isOverlappingVBaseABI())
     for (const auto &Base : RD->vbases()) {
       const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
       if (BaseDecl->isEmpty())
@@ -486,7 +502,8 @@ void CGRecordLowering::accumulateVBases() {
     CharUnits Offset = Layout.getVBaseClassOffset(BaseDecl);
     // If the vbase is a primary virtual base of some base, then it doesn't
     // get its own storage location but instead lives inside of that base.
-    if (!useMSABI() && Context.isNearlyEmpty(BaseDecl) &&
+    if (isOverlappingVBaseABI() &&
+        Context.isNearlyEmpty(BaseDecl) &&
         !hasOwnStorage(RD, BaseDecl)) {
       Members.push_back(MemberInfo(Offset, MemberInfo::VBase, nullptr,
                                    BaseDecl));
diff --git a/clang/test/CodeGenCXX/ms_struct.cpp b/clang/test/CodeGenCXX/ms_struct.cpp
new file mode 100644
index 000000000000..32307bad64d8
--- /dev/null
+++ b/clang/test/CodeGenCXX/ms_struct.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm %s -o - | FileCheck %s
+
+// rdar://20636558
+
+#pragma GCC diagnostic ignored "-Wincompatible-ms-struct"
+#define ATTR __attribute__((__ms_struct__))
+
+struct ATTR VBase {
+  virtual void foo() = 0;
+};
+
+struct ATTR Base : virtual VBase {
+  virtual void bar() = 0;
+};
+
+struct ATTR Derived : Base {
+  Derived();
+  void foo();
+  void bar();
+  int value;
+};
+
+// CHECK: [[DERIVED:%.*]] = type <{ [[BASE:%.*]], i32, [4 x i8] }>
+// CHECK: [[BASE]] = type { [[VBASE:%.*]] }
+// CHECK: [[VBASE]] = type { i32 (...)** }
+
+// CHECK: define void @_ZN7DerivedC2Ev
+// CHECK:   [[SELF:%.*]] = load [[DERIVED]]*
+// CHECK:   [[T0:%.*]] = bitcast [[DERIVED]]* [[SELF]] to [[BASE]]*
+// CHECK:   call void @_ZN4BaseC2Ev([[BASE]]* [[T0]], i8**
+// CHECK:   [[T0:%.*]] = getelementptr inbounds {{.*}} [[SELF]], i32 0, i32 1
+// CHECK:   store i32 20, i32* [[T0]],
+Derived::Derived() : value(20) {}