Skip to content

Commit cb69ae9

Browse files
tbaederrtbaederr
committed
[clang][bytecode] Check dtor instance pointers for active-ness
And diagnose if we're trying to destroy an inactive member of a union.
1 parent 1e89a76 commit cb69ae9

File tree

5 files changed

+136
-65
lines changed

5 files changed

+136
-65
lines changed

‎clang/lib/AST/ByteCode/Compiler.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4787,8 +4787,12 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
47874787
}
47884788
// Explicit calls to trivial destructors
47894789
if (const auto *DD = dyn_cast_if_present<CXXDestructorDecl>(FuncDecl);
4790-
DD && DD->isTrivial())
4791-
return true;
4790+
DD && DD->isTrivial()) {
4791+
const auto *MemberCall = cast<CXXMemberCallExpr>(E);
4792+
if (!this->visit(MemberCall->getImplicitObjectArgument()))
4793+
return false;
4794+
return this->emitCheckDestruction(E) && this->emitPopPtr(E);
4795+
}
47924796

47934797
QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
47944798
std::optional<PrimType> T = classify(ReturnType);
@@ -5829,6 +5833,9 @@ bool Compiler<Emitter>::compileDestructor(const CXXDestructorDecl *Dtor) {
58295833
if (!this->emitThis(Dtor))
58305834
return false;
58315835

5836+
if (!this->emitCheckDestruction(Dtor))
5837+
return false;
5838+
58325839
assert(R);
58335840
if (!R->isUnion()) {
58345841
// First, destroy all fields.

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

Lines changed: 69 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -202,68 +202,6 @@ static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC,
202202
S.Note(VD->getLocation(), diag::note_declared_at);
203203
}
204204

205-
static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
206-
AccessKinds AK) {
207-
if (Ptr.isActive())
208-
return true;
209-
210-
assert(Ptr.inUnion());
211-
assert(Ptr.isField() && Ptr.getField());
212-
213-
Pointer U = Ptr.getBase();
214-
Pointer C = Ptr;
215-
while (!U.isRoot() && !U.isActive()) {
216-
// A little arbitrary, but this is what the current interpreter does.
217-
// See the AnonymousUnion test in test/AST/ByteCode/unions.cpp.
218-
// GCC's output is more similar to what we would get without
219-
// this condition.
220-
if (U.getRecord() && U.getRecord()->isAnonymousUnion())
221-
break;
222-
223-
C = U;
224-
U = U.getBase();
225-
}
226-
assert(C.isField());
227-
228-
// Consider:
229-
// union U {
230-
// struct {
231-
// int x;
232-
// int y;
233-
// } a;
234-
// }
235-
//
236-
// When activating x, we will also activate a. If we now try to read
237-
// from y, we will get to CheckActive, because y is not active. In that
238-
// case, our U will be a (not a union). We return here and let later code
239-
// handle this.
240-
if (!U.getFieldDesc()->isUnion())
241-
return true;
242-
243-
// Get the inactive field descriptor.
244-
assert(!C.isActive());
245-
const FieldDecl *InactiveField = C.getField();
246-
assert(InactiveField);
247-
248-
// Find the active field of the union.
249-
const Record *R = U.getRecord();
250-
assert(R && R->isUnion() && "Not a union");
251-
252-
const FieldDecl *ActiveField = nullptr;
253-
for (const Record::Field &F : R->fields()) {
254-
const Pointer &Field = U.atField(F.Offset);
255-
if (Field.isActive()) {
256-
ActiveField = Field.getField();
257-
break;
258-
}
259-
}
260-
261-
const SourceInfo &Loc = S.Current->getSource(OpPC);
262-
S.FFDiag(Loc, diag::note_constexpr_access_inactive_union_member)
263-
<< AK << InactiveField << !ActiveField << ActiveField;
264-
return false;
265-
}
266-
267205
static bool CheckTemporary(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
268206
AccessKinds AK) {
269207
if (auto ID = Ptr.getDeclID()) {
@@ -375,7 +313,68 @@ bool CheckBCPResult(InterpState &S, const Pointer &Ptr) {
375313

376314
if (const Expr *Base = Ptr.getDeclDesc()->asExpr())
377315
return isa<StringLiteral>(Base);
316+
return false;
317+
}
318+
319+
bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
320+
AccessKinds AK) {
321+
if (Ptr.isActive())
322+
return true;
378323

324+
assert(Ptr.inUnion());
325+
assert(Ptr.isField() && Ptr.getField());
326+
327+
Pointer U = Ptr.getBase();
328+
Pointer C = Ptr;
329+
while (!U.isRoot() && !U.isActive()) {
330+
// A little arbitrary, but this is what the current interpreter does.
331+
// See the AnonymousUnion test in test/AST/ByteCode/unions.cpp.
332+
// GCC's output is more similar to what we would get without
333+
// this condition.
334+
if (U.getRecord() && U.getRecord()->isAnonymousUnion())
335+
break;
336+
337+
C = U;
338+
U = U.getBase();
339+
}
340+
assert(C.isField());
341+
342+
// Consider:
343+
// union U {
344+
// struct {
345+
// int x;
346+
// int y;
347+
// } a;
348+
// }
349+
//
350+
// When activating x, we will also activate a. If we now try to read
351+
// from y, we will get to CheckActive, because y is not active. In that
352+
// case, our U will be a (not a union). We return here and let later code
353+
// handle this.
354+
if (!U.getFieldDesc()->isUnion())
355+
return true;
356+
357+
// Get the inactive field descriptor.
358+
assert(!C.isActive());
359+
const FieldDecl *InactiveField = C.getField();
360+
assert(InactiveField);
361+
362+
// Find the active field of the union.
363+
const Record *R = U.getRecord();
364+
assert(R && R->isUnion() && "Not a union");
365+
366+
const FieldDecl *ActiveField = nullptr;
367+
for (const Record::Field &F : R->fields()) {
368+
const Pointer &Field = U.atField(F.Offset);
369+
if (Field.isActive()) {
370+
ActiveField = Field.getField();
371+
break;
372+
}
373+
}
374+
375+
const SourceInfo &Loc = S.Current->getSource(OpPC);
376+
S.FFDiag(Loc, diag::note_constexpr_access_inactive_union_member)
377+
<< AK << InactiveField << !ActiveField << ActiveField;
379378
return false;
380379
}
381380

@@ -1355,6 +1354,11 @@ static bool checkConstructor(InterpState &S, CodePtr OpPC, const Function *Func,
13551354
return false;
13561355
}
13571356

1357+
static bool checkDestructor(InterpState &S, CodePtr OpPC, const Function *Func,
1358+
const Pointer &ThisPtr) {
1359+
return CheckActive(S, OpPC, ThisPtr, AK_Destroy);
1360+
}
1361+
13581362
bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
13591363
uint32_t VarArgSize) {
13601364
if (Func->hasThisPointer()) {
@@ -1433,13 +1437,15 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
14331437
} else {
14341438
if (!CheckInvoke(S, OpPC, ThisPtr))
14351439
return cleanup();
1436-
if (!Func->isConstructor() &&
1440+
if (!Func->isConstructor() && !Func->isDestructor() &&
14371441
!CheckActive(S, OpPC, ThisPtr, AK_MemberCall))
14381442
return false;
14391443
}
14401444

14411445
if (Func->isConstructor() && !checkConstructor(S, OpPC, Func, ThisPtr))
14421446
return false;
1447+
if (Func->isDestructor() && !checkDestructor(S, OpPC, Func, ThisPtr))
1448+
return false;
14431449
}
14441450

14451451
if (!CheckCallable(S, OpPC, Func))

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC,
137137
bool CheckDeleteSource(InterpState &S, CodePtr OpPC, const Expr *Source,
138138
const Pointer &Ptr);
139139

140+
bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
141+
AccessKinds AK);
142+
140143
/// Sets the given integral value to the pointer, which is of
141144
/// a std::{weak,partial,strong}_ordering type.
142145
bool SetThreeWayComparisonField(InterpState &S, CodePtr OpPC,
@@ -3105,6 +3108,11 @@ bool GetTypeid(InterpState &S, CodePtr OpPC, const Type *TypePtr,
31053108
bool GetTypeidPtr(InterpState &S, CodePtr OpPC, const Type *TypeInfoType);
31063109
bool DiagTypeid(InterpState &S, CodePtr OpPC);
31073110

3111+
inline bool CheckDestruction(InterpState &S, CodePtr OpPC) {
3112+
const auto &Ptr = S.Stk.peek<Pointer>();
3113+
return CheckActive(S, OpPC, Ptr, AK_Destroy);
3114+
}
3115+
31083116
//===----------------------------------------------------------------------===//
31093117
// Read opcode arguments
31103118
//===----------------------------------------------------------------------===//

‎clang/lib/AST/ByteCode/Opcodes.td‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,3 +865,5 @@ def BitCast : Opcode;
865865
def GetTypeid : Opcode { let Args = [ArgTypePtr, ArgTypePtr]; }
866866
def GetTypeidPtr : Opcode { let Args = [ArgTypePtr]; }
867867
def DiagTypeid : Opcode;
868+
869+
def CheckDestruction : Opcode;

‎clang/test/AST/ByteCode/unions.cpp‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,4 +522,52 @@ namespace MemberCalls {
522522
static_assert(foo()); // both-error {{not an integral constant expression}} \
523523
// both-note {{in call to}}
524524
}
525+
526+
namespace InactiveDestroy {
527+
struct A {
528+
constexpr ~A() {}
529+
};
530+
union U {
531+
A a;
532+
constexpr ~U() {
533+
}
534+
};
535+
536+
constexpr bool foo() { // both-error {{never produces a constant expression}}
537+
U u;
538+
u.a.~A(); // both-note 2{{destruction of member 'a' of union with no active member}}
539+
return true;
540+
}
541+
static_assert(foo()); // both-error {{not an integral constant expression}} \
542+
// both-note {{in call to}}
543+
}
544+
545+
namespace InactiveTrivialDestroy {
546+
struct A {};
547+
union U {
548+
A a;
549+
};
550+
551+
constexpr bool foo() { // both-error {{never produces a constant expression}}
552+
U u;
553+
u.a.~A(); // both-note 2{{destruction of member 'a' of union with no active member}}
554+
return true;
555+
}
556+
static_assert(foo()); // both-error {{not an integral constant expression}} \
557+
// both-note {{in call to}}
558+
}
559+
560+
namespace ActiveDestroy {
561+
struct A {};
562+
union U {
563+
A a;
564+
};
565+
constexpr bool foo2() {
566+
U u{};
567+
u.a.~A();
568+
return true;
569+
}
570+
static_assert(foo2());
571+
}
572+
525573
#endif

0 commit comments

Comments
 (0)