[clang] 9c56035 - Revert "[Clang][MS] Remove assertion on BaseOffset can't be smaller than Size."
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 14 16:20:22 PDT 2023
Author: Nico Weber
Date: 2023-06-14T16:19:48-07:00
New Revision: 9c560350dd57d0dcb849c5915fa7c50b139ce671
URL: https://github.com/llvm/llvm-project/commit/9c560350dd57d0dcb849c5915fa7c50b139ce671
DIFF: https://github.com/llvm/llvm-project/commit/9c560350dd57d0dcb849c5915fa7c50b139ce671.diff
LOG: Revert "[Clang][MS] Remove assertion on BaseOffset can't be smaller than Size."
This reverts commit 5d54213ee557a86fae82af0f75498adf02f24e82.
Breaks check-clang on Windows, see https://reviews.llvm.org/D152472#4422913
Added:
Modified:
clang/include/clang/Frontend/LayoutOverrideSource.h
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Frontend/LayoutOverrideSource.cpp
Removed:
clang/test/CodeGenCXX/Inputs/override-layout-ms.layout
clang/test/CodeGenCXX/override-layout-ms.cpp
################################################################################
diff --git a/clang/include/clang/Frontend/LayoutOverrideSource.h b/clang/include/clang/Frontend/LayoutOverrideSource.h
index c6e2d73111833..ea1611470a76a 100644
--- a/clang/include/clang/Frontend/LayoutOverrideSource.h
+++ b/clang/include/clang/Frontend/LayoutOverrideSource.h
@@ -30,12 +30,6 @@ 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 3f836cb96be57..aca50912dceac 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2926,7 +2926,8 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
bool FoundBase = false;
if (UseExternalLayout) {
FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset);
- if (BaseOffset > Size) {
+ if (FoundBase) {
+ assert(BaseOffset >= Size && "base offset already allocated");
Size = BaseOffset;
}
}
@@ -3722,28 +3723,6 @@ 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 f09444deb8d38..0d288db0632fd 100644
--- a/clang/lib/Frontend/LayoutOverrideSource.cpp
+++ b/clang/lib/Frontend/LayoutOverrideSource.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#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>
@@ -27,18 +26,6 @@ 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())
@@ -93,8 +80,8 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
LineStr = LineStr.substr(Pos + strlen(" Size:"));
unsigned long long Size = 0;
- if (parseUnsigned(LineStr, Size))
- CurrentLayout.Size = Size;
+ (void)LineStr.getAsInteger(10, Size);
+ CurrentLayout.Size = Size;
continue;
}
@@ -105,13 +92,12 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
LineStr = LineStr.substr(Pos + strlen("Alignment:"));
unsigned long long Alignment = 0;
- if (parseUnsigned(LineStr, Alignment))
- CurrentLayout.Align = Alignment;
+ (void)LineStr.getAsInteger(10, Alignment);
+ CurrentLayout.Align = Alignment;
continue;
}
- // Check for the size/alignment of the type. The number follows "size=" or
- // "align=" indicates number of bytes.
+ // Check for the size/alignment of the type.
Pos = LineStr.find("sizeof=");
if (Pos != StringRef::npos) {
/* Skip past the sizeof= prefix. */
@@ -119,8 +105,8 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
// Parse size.
unsigned long long Size = 0;
- if (parseUnsigned(LineStr, Size))
- CurrentLayout.Size = Size * 8;
+ (void)LineStr.getAsInteger(10, Size);
+ CurrentLayout.Size = Size;
Pos = LineStr.find("align=");
if (Pos != StringRef::npos) {
@@ -129,8 +115,8 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
// Parse alignment.
unsigned long long Alignment = 0;
- if (parseUnsigned(LineStr, Alignment))
- CurrentLayout.Align = Alignment * 8;
+ (void)LineStr.getAsInteger(10, Alignment);
+ CurrentLayout.Align = Alignment;
}
continue;
@@ -138,50 +124,25 @@ LayoutOverrideSource::LayoutOverrideSource(StringRef Filename) {
// Check for the field offsets of the type.
Pos = LineStr.find("FieldOffsets: [");
- 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);
- }
- }
+ if (Pos == StringRef::npos)
+ continue;
- // 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));
+ 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;
- // Skip over this offset, the following comma, and any spaces.
- LineStr = LineStr.substr(1);
- while (!LineStr.empty() && isWhitespace(LineStr[0]))
- LineStr = LineStr.substr(1);
- }
- }
+ unsigned long long Offset = 0;
+ (void)LineStr.substr(0, Idx).getAsInteger(10, 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));
+ CurrentLayout.FieldOffsets.push_back(Offset);
- // Skip over this offset, the following comma, and any spaces.
+ // Skip over this offset, the following comma, and any spaces.
+ LineStr = LineStr.substr(Idx + 1);
+ while (!LineStr.empty() && isWhitespace(LineStr[0]))
LineStr = LineStr.substr(1);
- while (!LineStr.empty() && isWhitespace(LineStr[0]))
- LineStr = LineStr.substr(1);
- }
}
}
@@ -221,24 +182,6 @@ 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
deleted file mode 100644
index 27790e8589c46..0000000000000
--- a/clang/test/CodeGenCXX/Inputs/override-layout-ms.layout
+++ /dev/null
@@ -1,49 +0,0 @@
-*** 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
deleted file mode 100644
index 1e678a4576a12..0000000000000
--- a/clang/test/CodeGenCXX/override-layout-ms.cpp
+++ /dev/null
@@ -1,43 +0,0 @@
-// 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