Skip to content

Commit 15f0272

Browse files
tbaederrtbaederr
committed
[clang][Interp] Improve support for virtual bases
Fix initializing virtual bases. We only consider their base size, not their virtual size because they get flattened into the Record hierarchy when we create them. Fix that and also virtual derived-to-base casts.
1 parent e0a5155 commit 15f0272

File tree

6 files changed

+192
-27
lines changed

6 files changed

+192
-27
lines changed

‎clang/lib/AST/Interp/ByteCodeExprGen.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,29 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
110110
if (!this->visit(SubExpr))
111111
return false;
112112

113-
unsigned DerivedOffset =
114-
collectBaseOffset(CE->getType(), SubExpr->getType());
113+
const auto extractRecordDecl = [](QualType Ty) -> const CXXRecordDecl * {
114+
if (const auto *PT = dyn_cast<PointerType>(Ty))
115+
return PT->getPointeeType()->getAsCXXRecordDecl();
116+
return Ty->getAsCXXRecordDecl();
117+
};
115118

116-
return this->emitGetPtrBasePop(DerivedOffset, CE);
119+
// FIXME: We can express a series of non-virtual casts as a single
120+
// GetPtrBasePop op.
121+
QualType CurType = SubExpr->getType();
122+
for (const CXXBaseSpecifier *B : CE->path()) {
123+
if (B->isVirtual()) {
124+
if (!this->emitGetPtrVirtBasePop(extractRecordDecl(B->getType()), CE))
125+
return false;
126+
CurType = B->getType();
127+
} else {
128+
unsigned DerivedOffset = collectBaseOffset(B->getType(), CurType);
129+
if (!this->emitGetPtrBasePop(DerivedOffset, CE))
130+
return false;
131+
CurType = B->getType();
132+
}
133+
}
134+
135+
return true;
117136
}
118137

119138
case CK_BaseToDerived: {

‎clang/lib/AST/Interp/ByteCodeStmtGen.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,14 +189,24 @@ bool ByteCodeStmtGen<Emitter>::visitFunc(const FunctionDecl *F) {
189189
if (!emitFieldInitializer(F, F->Offset, InitExpr))
190190
return false;
191191
} else if (const Type *Base = Init->getBaseClass()) {
192-
// Base class initializer.
193-
// Get This Base and call initializer on it.
194192
const auto *BaseDecl = Base->getAsCXXRecordDecl();
195193
assert(BaseDecl);
196-
const Record::Base *B = R->getBase(BaseDecl);
197-
assert(B);
198-
if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
199-
return false;
194+
195+
if (Init->isBaseVirtual()) {
196+
const Record::Base *B = R->getVirtualBase(BaseDecl);
197+
assert(B);
198+
if (!this->emitGetPtrThisVirtBase(BaseDecl, InitExpr))
199+
return false;
200+
201+
} else {
202+
// Base class initializer.
203+
// Get This Base and call initializer on it.
204+
const Record::Base *B = R->getBase(BaseDecl);
205+
assert(B);
206+
if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
207+
return false;
208+
}
209+
200210
if (!this->visitInitializer(InitExpr))
201211
return false;
202212
if (!this->emitFinishInitPop(InitExpr))

‎clang/lib/AST/Interp/Descriptor.cpp

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -136,28 +136,66 @@ static void moveArrayDesc(Block *B, const std::byte *Src, std::byte *Dst,
136136
}
137137
}
138138

139+
static void initField(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
140+
bool IsActive, const Descriptor *D,
141+
unsigned FieldOffset) {
142+
bool IsUnion = false; // FIXME
143+
auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + FieldOffset) - 1;
144+
Desc->Offset = FieldOffset;
145+
Desc->Desc = D;
146+
Desc->IsInitialized = D->IsArray;
147+
Desc->IsBase = false;
148+
Desc->IsActive = IsActive && !IsUnion;
149+
Desc->IsConst = IsConst || D->IsConst;
150+
Desc->IsFieldMutable = IsMutable || D->IsMutable;
151+
152+
if (auto Fn = D->CtorFn)
153+
Fn(B, Ptr + FieldOffset, Desc->IsConst, Desc->IsFieldMutable,
154+
Desc->IsActive, D);
155+
}
156+
157+
static void initBase(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
158+
bool IsActive, const Descriptor *D, unsigned FieldOffset,
159+
bool IsVirtualBase) {
160+
assert(D);
161+
assert(D->ElemRecord);
162+
163+
bool IsUnion = D->ElemRecord->isUnion();
164+
auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + FieldOffset) - 1;
165+
Desc->Offset = FieldOffset;
166+
Desc->Desc = D;
167+
Desc->IsInitialized = D->IsArray;
168+
Desc->IsBase = true;
169+
Desc->IsActive = IsActive && !IsUnion;
170+
Desc->IsConst = IsConst || D->IsConst;
171+
Desc->IsFieldMutable = IsMutable || D->IsMutable;
172+
173+
for (const auto &V : D->ElemRecord->bases())
174+
initBase(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, V.Desc,
175+
V.Offset, false);
176+
for (const auto &F : D->ElemRecord->fields())
177+
initField(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, F.Desc,
178+
F.Offset);
179+
180+
// If this is initializing a virtual base, we do NOT want to consider its
181+
// virtual bases, those are already flattened into the parent record when
182+
// creating it.
183+
if (IsVirtualBase)
184+
return;
185+
186+
for (const auto &V : D->ElemRecord->virtual_bases())
187+
initBase(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, V.Desc,
188+
V.Offset, true);
189+
}
190+
139191
static void ctorRecord(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
140192
bool IsActive, const Descriptor *D) {
141-
const bool IsUnion = D->ElemRecord->isUnion();
142-
auto CtorSub = [=](unsigned SubOff, const Descriptor *F, bool IsBase) {
143-
auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + SubOff) - 1;
144-
Desc->Offset = SubOff;
145-
Desc->Desc = F;
146-
Desc->IsInitialized = F->IsArray && !IsBase;
147-
Desc->IsBase = IsBase;
148-
Desc->IsActive = IsActive && !IsUnion;
149-
Desc->IsConst = IsConst || F->IsConst;
150-
Desc->IsFieldMutable = IsMutable || F->IsMutable;
151-
if (auto Fn = F->CtorFn)
152-
Fn(B, Ptr + SubOff, Desc->IsConst, Desc->IsFieldMutable, Desc->IsActive,
153-
F);
154-
};
155-
for (const auto &B : D->ElemRecord->bases())
156-
CtorSub(B.Offset, B.Desc, /*isBase=*/true);
193+
for (const auto &V : D->ElemRecord->bases())
194+
initBase(B, Ptr, IsConst, IsMutable, IsActive, V.Desc, V.Offset, false);
157195
for (const auto &F : D->ElemRecord->fields())
158-
CtorSub(F.Offset, F.Desc, /*isBase=*/false);
196+
initField(B, Ptr, IsConst, IsMutable, IsActive, F.Desc, F.Offset);
159197
for (const auto &V : D->ElemRecord->virtual_bases())
160-
CtorSub(V.Offset, V.Desc, /*isBase=*/true);
198+
initBase(B, Ptr, IsConst, IsMutable, IsActive, V.Desc, V.Offset, true);
161199
}
162200

163201
static void dtorRecord(Block *B, std::byte *Ptr, const Descriptor *D) {

‎clang/lib/AST/Interp/Interp.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,9 @@ inline bool GetPtrVirtBasePop(InterpState &S, CodePtr OpPC,
13661366
const Pointer &Ptr = S.Stk.pop<Pointer>();
13671367
if (!CheckNull(S, OpPC, Ptr, CSK_Base))
13681368
return false;
1369+
if (Ptr.isDummy()) // FIXME: Once we have type info for dummy pointers, this
1370+
// needs to go.
1371+
return false;
13691372
return VirtBaseHelper(S, OpPC, D, Ptr);
13701373
}
13711374

‎clang/test/AST/Interp/cxx23.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,19 @@ struct check_ice {
141141
};
142142
};
143143
static_assert(check_ice<42>::x == 42);
144+
145+
146+
namespace VirtualBases {
147+
namespace One {
148+
struct U { int n; };
149+
struct V : U { int n; };
150+
struct A : virtual V { int n; };
151+
struct Aa { int n; };
152+
struct B : virtual A, Aa {};
153+
struct C : virtual A, Aa {};
154+
struct D : B, C {};
155+
156+
/// Calls the constructor of D.
157+
D d;
158+
}
159+
}

‎clang/test/AST/Interp/records.cpp

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,3 +1330,82 @@ namespace UnnamedBitFields {
13301330
static_assert(a.f == 1.0, "");
13311331
static_assert(a.c == 'a', "");
13321332
}
1333+
1334+
/// FIXME: This still doesn't work in the new interpreter because
1335+
/// we lack type information for dummy pointers.
1336+
namespace VirtualBases {
1337+
/// This used to crash.
1338+
namespace One {
1339+
class A {
1340+
protected:
1341+
int x;
1342+
};
1343+
class B : public virtual A {
1344+
public:
1345+
int getX() { return x; } // ref-note {{declared here}}
1346+
};
1347+
1348+
class DV : virtual public B{};
1349+
1350+
void foo() {
1351+
DV b;
1352+
int a[b.getX()]; // both-warning {{variable length arrays}} \
1353+
// ref-note {{non-constexpr function 'getX' cannot be used}}
1354+
}
1355+
}
1356+
1357+
namespace Two {
1358+
struct U { int n; };
1359+
struct A : virtual U { int n; };
1360+
struct B : A {};
1361+
B a;
1362+
static_assert((U*)(A*)(&a) == (U*)(&a), "");
1363+
1364+
struct C : virtual A {};
1365+
struct D : B, C {};
1366+
D d;
1367+
constexpr B *p = &d;
1368+
constexpr C *q = &d;
1369+
static_assert((A*)p == (A*)q, ""); // both-error {{failed}}
1370+
}
1371+
1372+
namespace Three {
1373+
struct U { int n; };
1374+
struct V : U { int n; };
1375+
struct A : virtual V { int n; };
1376+
struct Aa { int n; };
1377+
struct B : virtual A, Aa {};
1378+
1379+
struct C : virtual A, Aa {};
1380+
1381+
struct D : B, C {};
1382+
1383+
D d;
1384+
1385+
constexpr B *p = &d;
1386+
constexpr C *q = &d;
1387+
1388+
static_assert((void*)p != (void*)q, "");
1389+
static_assert((A*)p == (A*)q, "");
1390+
static_assert((Aa*)p != (Aa*)q, "");
1391+
1392+
constexpr V *v = p;
1393+
constexpr V *w = q;
1394+
constexpr V *x = (A*)p;
1395+
static_assert(v == w, "");
1396+
static_assert(v == x, "");
1397+
1398+
static_assert((U*)&d == p, "");
1399+
static_assert((U*)&d == q, "");
1400+
static_assert((U*)&d == v, "");
1401+
static_assert((U*)&d == w, "");
1402+
static_assert((U*)&d == x, "");
1403+
1404+
struct X {};
1405+
struct Y1 : virtual X {};
1406+
struct Y2 : X {};
1407+
struct Z : Y1, Y2 {};
1408+
Z z;
1409+
static_assert((X*)(Y1*)&z != (X*)(Y2*)&z, "");
1410+
}
1411+
}

0 commit comments

Comments
 (0)