[clang] 879e886 - Reland "[Clang][MS] Remove assertion on BaseOffset can't be smaller than Size."
Zequan Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 15 08:34:40 PDT 2023
Author: Zequan Wu
Date: 2023-06-15T11:33:02-04:00
New Revision: 879e88693338657aec092db749ddfcb582c65491
URL: https://github.com/llvm/llvm-project/commit/879e88693338657aec092db749ddfcb582c65491
DIFF: https://github.com/llvm/llvm-project/commit/879e88693338657aec092db749ddfcb582c65491.diff
LOG: Reland "[Clang][MS] Remove assertion on BaseOffset can't be smaller than Size."
This reland 5d54213ee557a86fae82af0f75498adf02f24e82 with fixes.
Added:
clang/test/CodeGenCXX/Inputs/override-layout-ms.layout
clang/test/CodeGenCXX/override-layout-ms.cpp
Modified:
clang/include/clang/Frontend/LayoutOverrideSource.h
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Frontend/LayoutOverrideSource.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Frontend/LayoutOverrideSource.h b/clang/include/clang/Frontend/LayoutOverrideSource.h
index ea1611470a76a..c6e2d73111833 100644
--- a/clang/include/clang/Frontend/LayoutOverrideSource.h
+++ b/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;
};
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index aca50912dceac..3f836cb96be57 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/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)
diff --git a/clang/lib/Frontend/LayoutOverrideSource.cpp b/clang/lib/Frontend/LayoutOverrideSource.cpp
index 0d288db0632fd..f09444deb8d38 100644
--- a/clang/lib/Frontend/LayoutOverrideSource.cpp
+++ b/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,12 +105,13 @@ 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. */
@@ -105,8 +119,8 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
// 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,8 +129,8 @@ 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;
@@ -124,25 +138,50 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
// 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;
diff --git a/clang/test/CodeGenCXX/Inputs/override-layout-ms.layout b/clang/test/CodeGenCXX/Inputs/override-layout-ms.layout
new file mode 100644
index 0000000000000..27790e8589c46
--- /dev/null
+++ b/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: []>
diff --git a/clang/test/CodeGenCXX/override-layout-ms.cpp b/clang/test/CodeGenCXX/override-layout-ms.cpp
new file mode 100644
index 0000000000000..1e678a4576a12
--- /dev/null
+++ b/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;
\ No newline at end of file
More information about the cfe-commits
mailing list