Reland "[Clang][MS] Remove assertion on BaseOffset can't be smaller t…

…han Size."

This reland 5d54213 with fixes.

clang/include/clang/Frontend/LayoutOverrideSource.h

@@ -30,6 +30,12 @@ namespace clang {
       /// The alignment of the record.
       uint64_t Align;
 
+      /// The offsets of non-virtual base classes in the record.
+      SmallVector<CharUnits, 8> BaseOffsets;
+
+      /// The offsets of virtual base classes in the record.
+      SmallVector<CharUnits, 8> VBaseOffsets;
+
       /// The offsets of the fields, in source order.
       SmallVector<uint64_t, 8> FieldOffsets;
     };

clang/lib/AST/RecordLayoutBuilder.cpp

@@ -2926,8 +2926,7 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
   bool FoundBase = false;
   if (UseExternalLayout) {
     FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset);
-    if (FoundBase) {
-      assert(BaseOffset >= Size && "base offset already allocated");
+    if (BaseOffset > Size) {
       Size = BaseOffset;
     }
   }
@@ -3723,6 +3722,28 @@ void ASTContext::DumpRecordLayout(const RecordDecl *RD, raw_ostream &OS,
   if (Target->defaultsToAIXPowerAlignment())
     OS << "  PreferredAlignment:" << toBits(Info.getPreferredAlignment())
        << "\n";
+  if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+    OS << "  BaseOffsets: [";
+    const CXXRecordDecl *Base = nullptr;
+    for (auto I : CXXRD->bases()) {
+      if (I.isVirtual())
+        continue;
+      if (Base)
+        OS << ", ";
+      Base = I.getType()->getAsCXXRecordDecl();
+      OS << Info.CXXInfo->BaseOffsets[Base].getQuantity();
+    }
+    OS << "]>\n";
+    OS << "  VBaseOffsets: [";
+    const CXXRecordDecl *VBase = nullptr;
+    for (auto I : CXXRD->vbases()) {
+      if (VBase)
+        OS << ", ";
+      VBase = I.getType()->getAsCXXRecordDecl();
+      OS << Info.CXXInfo->VBaseOffsets[VBase].VBaseOffset.getQuantity();
+    }
+    OS << "]>\n";
+  }
   OS << "  FieldOffsets: [";
   for (unsigned i = 0, e = Info.getFieldCount(); i != e; ++i) {
     if (i)

clang/lib/Frontend/LayoutOverrideSource.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 #include "clang/Frontend/LayoutOverrideSource.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/raw_ostream.h"
 #include <fstream>
@@ -26,6 +27,18 @@ static std::string parseName(StringRef S) {
   return S.substr(0, Offset).str();
 }
 
+/// Parse an unsigned integer and move S to the next non-digit character.
+static bool parseUnsigned(StringRef &S, unsigned long long &ULL) {
+  if (S.empty() || !isDigit(S[0]))
+    return false;
+  unsigned Idx = 1;
+  while (Idx < S.size() && isDigit(S[Idx]))
+    ++Idx;
+  (void)S.substr(0, Idx).getAsInteger(10, ULL);
+  S = S.substr(Idx);
+  return true;
+}
+
 LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
   std::ifstream Input(Filename.str().c_str());
   if (!Input.is_open())
@@ -80,8 +93,8 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
       LineStr = LineStr.substr(Pos + strlen(" Size:"));
 
       unsigned long long Size = 0;
-      (void)LineStr.getAsInteger(10, Size);
-      CurrentLayout.Size = Size;
+      if (parseUnsigned(LineStr, Size))
+        CurrentLayout.Size = Size;
       continue;
     }
 
@@ -92,21 +105,22 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
       LineStr = LineStr.substr(Pos + strlen("Alignment:"));
 
       unsigned long long Alignment = 0;
-      (void)LineStr.getAsInteger(10, Alignment);
-      CurrentLayout.Align = Alignment;
+      if (parseUnsigned(LineStr, Alignment))
+        CurrentLayout.Align = Alignment;
       continue;
     }
 
-    // Check for the size/alignment of the type.
+    // Check for the size/alignment of the type. The number follows "size=" or
+    // "align=" indicates number of bytes.
     Pos = LineStr.find("sizeof=");
     if (Pos != StringRef::npos) {
       /* Skip past the sizeof= prefix. */
       LineStr = LineStr.substr(Pos + strlen("sizeof="));
 
       // Parse size.
       unsigned long long Size = 0;
-      (void)LineStr.getAsInteger(10, Size);
-      CurrentLayout.Size = Size;
+      if (parseUnsigned(LineStr, Size))
+        CurrentLayout.Size = Size * 8;
 
       Pos = LineStr.find("align=");
       if (Pos != StringRef::npos) {
@@ -115,34 +129,59 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
 
         // Parse alignment.
         unsigned long long Alignment = 0;
-        (void)LineStr.getAsInteger(10, Alignment);
-        CurrentLayout.Align = Alignment;
+        if (parseUnsigned(LineStr, Alignment))
+          CurrentLayout.Align = Alignment * 8;
       }
 
       continue;
     }
 
     // Check for the field offsets of the type.
     Pos = LineStr.find("FieldOffsets: [");
-    if (Pos == StringRef::npos)
-      continue;
+    if (Pos != StringRef::npos) {
+      LineStr = LineStr.substr(Pos + strlen("FieldOffsets: ["));
+      while (!LineStr.empty() && isDigit(LineStr[0])) {
+        unsigned long long Offset = 0;
+        if (parseUnsigned(LineStr, Offset))
+          CurrentLayout.FieldOffsets.push_back(Offset);
+
+        // Skip over this offset, the following comma, and any spaces.
+        LineStr = LineStr.substr(1);
+        while (!LineStr.empty() && isWhitespace(LineStr[0]))
+          LineStr = LineStr.substr(1);
+      }
+    }
 
-    LineStr = LineStr.substr(Pos + strlen("FieldOffsets: ["));
-    while (!LineStr.empty() && isDigit(LineStr[0])) {
-      // Parse this offset.
-      unsigned Idx = 1;
-      while (Idx < LineStr.size() && isDigit(LineStr[Idx]))
-        ++Idx;
+    // Check for the base offsets.
+    Pos = LineStr.find("BaseOffsets: [");
+    if (Pos != StringRef::npos) {
+      LineStr = LineStr.substr(Pos + strlen("BaseOffsets: ["));
+      while (!LineStr.empty() && isDigit(LineStr[0])) {
+        unsigned long long Offset = 0;
+        if (parseUnsigned(LineStr, Offset))
+          CurrentLayout.BaseOffsets.push_back(CharUnits::fromQuantity(Offset));
 
-      unsigned long long Offset = 0;
-      (void)LineStr.substr(0, Idx).getAsInteger(10, Offset);
+        // Skip over this offset, the following comma, and any spaces.
+        LineStr = LineStr.substr(1);
+        while (!LineStr.empty() && isWhitespace(LineStr[0]))
+          LineStr = LineStr.substr(1);
+      }
+    }
 
-      CurrentLayout.FieldOffsets.push_back(Offset);
+    // Check for the virtual base offsets.
+    Pos = LineStr.find("VBaseOffsets: [");
+    if (Pos != StringRef::npos) {
+      LineStr = LineStr.substr(Pos + strlen("VBaseOffsets: ["));
+      while (!LineStr.empty() && isDigit(LineStr[0])) {
+        unsigned long long Offset = 0;
+        if (parseUnsigned(LineStr, Offset))
+          CurrentLayout.VBaseOffsets.push_back(CharUnits::fromQuantity(Offset));
 
-      // Skip over this offset, the following comma, and any spaces.
-      LineStr = LineStr.substr(Idx + 1);
-      while (!LineStr.empty() && isWhitespace(LineStr[0]))
+        // Skip over this offset, the following comma, and any spaces.
         LineStr = LineStr.substr(1);
+        while (!LineStr.empty() && isWhitespace(LineStr[0]))
+          LineStr = LineStr.substr(1);
+      }
     }
   }
 
@@ -182,6 +221,24 @@ LayoutOverrideSource::layoutRecordType(const RecordDecl *Record,
   if (NumFields != Known->second.FieldOffsets.size())
     return false;
 
+  // Provide base offsets.
+  if (const auto *RD = dyn_cast<CXXRecordDecl>(Record)) {
+    unsigned NumNB = 0;
+    unsigned NumVB = 0;
+    for (const auto &I : RD->vbases()) {
+      if (NumVB >= Known->second.VBaseOffsets.size())
+        continue;
+      const CXXRecordDecl *VBase = I.getType()->getAsCXXRecordDecl();
+      VirtualBaseOffsets[VBase] = Known->second.VBaseOffsets[NumVB++];
+    }
+    for (const auto &I : RD->bases()) {
+      if (I.isVirtual() || NumNB >= Known->second.BaseOffsets.size())
+        continue;
+      const CXXRecordDecl *Base = I.getType()->getAsCXXRecordDecl();
+      BaseOffsets[Base] = Known->second.BaseOffsets[NumNB++];
+    }
+  }
+
   Size = Known->second.Size;
   Alignment = Known->second.Align;
   return true;

clang/test/CodeGenCXX/Inputs/override-layout-ms.layout

@@ -0,0 +1,49 @@
+*** Dumping AST Record Layout
+Type: struct E1
+
+Layout: <ASTRecordLayout
+  Size:8
+  Alignment:8
+  BaseOffsets: []>
+  VBaseOffsets: []>
+  FieldOffsets: []>
+
+*** Dumping AST Record Layout
+Type: struct Mid
+
+Layout: <ASTRecordLayout
+  Size:64
+  Alignment:64
+  BaseOffsets: []>
+  VBaseOffsets: []>
+  FieldOffsets: [0]>
+
+*** Dumping AST Record Layout
+Type: struct E2
+
+Layout: <ASTRecordLayout
+  Size:8
+  Alignment:8
+  BaseOffsets: []>
+  VBaseOffsets: []>
+  FieldOffsets: []>
+
+*** Dumping AST Record Layout
+Type: struct Combine
+
+Layout: <ASTRecordLayout
+  Size:64
+  Alignment:64
+  BaseOffsets: [0, 0, 0]>
+  VBaseOffsets: []>
+  FieldOffsets: []>
+
+*** Dumping AST Record Layout
+Type: struct Combine2
+
+Layout: <ASTRecordLayout
+  Size:128
+  Alignment:64
+  BaseOffsets: [0, 8]>
+  VBaseOffsets: []>
+  FieldOffsets: []>

clang/test/CodeGenCXX/override-layout-ms.cpp

@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts -foverride-record-layout=%S/Inputs/override-layout-ms.layout %s | FileCheck  %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts %s | FileCheck  %s
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK:          0 | struct E1 (empty)
+// CHECK:            | [sizeof=1, align=1,
+// CHECK:            |  nvsize=0, nvalign=1]
+// CHECK: *** Dumping AST Record Layout
+// CHECK:          0 | struct Mid
+// CHECK:          0 |   void * p
+// CHECK:            | [sizeof=8, align=8,
+// CHECK:            |  nvsize=8, nvalign=8]
+// CHECK: *** Dumping AST Record Layout
+// CHECK:          0 | struct E2 (empty)
+// CHECK:            | [sizeof=1, align=1,
+// CHECK:            |  nvsize=0, nvalign=1]
+// CHECK: *** Dumping AST Record Layout
+// CHECK:          0 | struct Combine
+// CHECK:          0 |   struct E1 (base) (empty)
+// CHECK:          0 |   struct Mid (base)
+// CHECK:          0 |     void * p
+// CHECK:          0 |   struct E2 (base) (empty)
+// CHECK:            | [sizeof=8, align=8,
+// CHECK:            |  nvsize=8, nvalign=8]
+// CHECK: *** Dumping AST Record Layout
+// CHECK:          0 | struct Combine2
+// CHECK:          0 |   struct VB1 (primary base)
+// CHECK:          0 |     (VB1 vftable pointer)
+// CHECK:          8 |   struct VB2 (base)
+// CHECK:          8 |     (VB2 vftable pointer)
+// CHECK:            | [sizeof=16, align=8,
+// CHECK:            |  nvsize=16, nvalign=8]
+
+
+struct E1 {};
+struct E2 {};
+struct Mid {void *p; };
+struct __declspec(empty_bases) Combine : E1, Mid, E2 {};
+struct VB1 { virtual void foo() {}};
+struct VB2 { virtual void bar() {}};
+struct Combine2: VB1, VB2 {};
+Combine g;
+Combine2 f;