Skip to content

Commit 3928ede

Browse files
tbaederrtbaederr
authored
[clang][bytecode] Fix local destructor order (llvm#107951)
Add appropriate scopes and use reverse-order iteration in LocalScope::emitDestructors().
1 parent 4926835 commit 3928ede

File tree

3 files changed

+132
-34
lines changed

3 files changed

+132
-34
lines changed

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

+51-31
Original file line numberDiff line numberDiff line change
@@ -115,22 +115,26 @@ template <class Emitter> class LoopScope final : public LabelScope<Emitter> {
115115
LoopScope(Compiler<Emitter> *Ctx, LabelTy BreakLabel, LabelTy ContinueLabel)
116116
: LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel),
117117
OldContinueLabel(Ctx->ContinueLabel),
118-
OldLabelVarScope(Ctx->LabelVarScope) {
118+
OldBreakVarScope(Ctx->BreakVarScope),
119+
OldContinueVarScope(Ctx->ContinueVarScope) {
119120
this->Ctx->BreakLabel = BreakLabel;
120121
this->Ctx->ContinueLabel = ContinueLabel;
121-
this->Ctx->LabelVarScope = this->Ctx->VarScope;
122+
this->Ctx->BreakVarScope = this->Ctx->VarScope;
123+
this->Ctx->ContinueVarScope = this->Ctx->VarScope;
122124
}
123125

124126
~LoopScope() {
125127
this->Ctx->BreakLabel = OldBreakLabel;
126128
this->Ctx->ContinueLabel = OldContinueLabel;
127-
this->Ctx->LabelVarScope = OldLabelVarScope;
129+
this->Ctx->ContinueVarScope = OldContinueVarScope;
130+
this->Ctx->BreakVarScope = OldBreakVarScope;
128131
}
129132

130133
private:
131134
OptLabelTy OldBreakLabel;
132135
OptLabelTy OldContinueLabel;
133-
VariableScope<Emitter> *OldLabelVarScope;
136+
VariableScope<Emitter> *OldBreakVarScope;
137+
VariableScope<Emitter> *OldContinueVarScope;
134138
};
135139

136140
// Sets the context for a switch scope, mapping labels.
@@ -145,18 +149,18 @@ template <class Emitter> class SwitchScope final : public LabelScope<Emitter> {
145149
: LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel),
146150
OldDefaultLabel(this->Ctx->DefaultLabel),
147151
OldCaseLabels(std::move(this->Ctx->CaseLabels)),
148-
OldLabelVarScope(Ctx->LabelVarScope) {
152+
OldLabelVarScope(Ctx->BreakVarScope) {
149153
this->Ctx->BreakLabel = BreakLabel;
150154
this->Ctx->DefaultLabel = DefaultLabel;
151155
this->Ctx->CaseLabels = std::move(CaseLabels);
152-
this->Ctx->LabelVarScope = this->Ctx->VarScope;
156+
this->Ctx->BreakVarScope = this->Ctx->VarScope;
153157
}
154158

155159
~SwitchScope() {
156160
this->Ctx->BreakLabel = OldBreakLabel;
157161
this->Ctx->DefaultLabel = OldDefaultLabel;
158162
this->Ctx->CaseLabels = std::move(OldCaseLabels);
159-
this->Ctx->LabelVarScope = OldLabelVarScope;
163+
this->Ctx->BreakVarScope = OldLabelVarScope;
160164
}
161165

162166
private:
@@ -4605,18 +4609,23 @@ bool Compiler<Emitter>::visitWhileStmt(const WhileStmt *S) {
46054609
this->fallthrough(CondLabel);
46064610
this->emitLabel(CondLabel);
46074611

4608-
if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt())
4609-
if (!visitDeclStmt(CondDecl))
4610-
return false;
4612+
{
4613+
LocalScope<Emitter> CondScope(this);
4614+
if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt())
4615+
if (!visitDeclStmt(CondDecl))
4616+
return false;
46114617

4612-
if (!this->visitBool(Cond))
4613-
return false;
4614-
if (!this->jumpFalse(EndLabel))
4615-
return false;
4618+
if (!this->visitBool(Cond))
4619+
return false;
4620+
if (!this->jumpFalse(EndLabel))
4621+
return false;
46164622

4617-
if (!this->visitStmt(Body))
4618-
return false;
4623+
if (!this->visitStmt(Body))
4624+
return false;
46194625

4626+
if (!CondScope.destroyLocals())
4627+
return false;
4628+
}
46204629
if (!this->jump(CondLabel))
46214630
return false;
46224631
this->fallthrough(EndLabel);
@@ -4636,13 +4645,18 @@ template <class Emitter> bool Compiler<Emitter>::visitDoStmt(const DoStmt *S) {
46364645

46374646
this->fallthrough(StartLabel);
46384647
this->emitLabel(StartLabel);
4648+
46394649
{
4650+
LocalScope<Emitter> CondScope(this);
46404651
if (!this->visitStmt(Body))
46414652
return false;
46424653
this->fallthrough(CondLabel);
46434654
this->emitLabel(CondLabel);
46444655
if (!this->visitBool(Cond))
46454656
return false;
4657+
4658+
if (!CondScope.destroyLocals())
4659+
return false;
46464660
}
46474661
if (!this->jumpTrue(StartLabel))
46484662
return false;
@@ -4671,29 +4685,33 @@ bool Compiler<Emitter>::visitForStmt(const ForStmt *S) {
46714685
this->fallthrough(CondLabel);
46724686
this->emitLabel(CondLabel);
46734687

4674-
if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt())
4675-
if (!visitDeclStmt(CondDecl))
4676-
return false;
4688+
{
4689+
LocalScope<Emitter> CondScope(this);
4690+
if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt())
4691+
if (!visitDeclStmt(CondDecl))
4692+
return false;
46774693

4678-
if (Cond) {
4679-
if (!this->visitBool(Cond))
4680-
return false;
4681-
if (!this->jumpFalse(EndLabel))
4682-
return false;
4683-
}
4694+
if (Cond) {
4695+
if (!this->visitBool(Cond))
4696+
return false;
4697+
if (!this->jumpFalse(EndLabel))
4698+
return false;
4699+
}
46844700

4685-
{
46864701
if (Body && !this->visitStmt(Body))
46874702
return false;
46884703

46894704
this->fallthrough(IncLabel);
46904705
this->emitLabel(IncLabel);
46914706
if (Inc && !this->discard(Inc))
46924707
return false;
4693-
}
46944708

4709+
if (!CondScope.destroyLocals())
4710+
return false;
4711+
}
46954712
if (!this->jump(CondLabel))
46964713
return false;
4714+
46974715
this->fallthrough(EndLabel);
46984716
this->emitLabel(EndLabel);
46994717
return true;
@@ -4760,7 +4778,7 @@ bool Compiler<Emitter>::visitBreakStmt(const BreakStmt *S) {
47604778
if (!BreakLabel)
47614779
return false;
47624780

4763-
for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope;
4781+
for (VariableScope<Emitter> *C = VarScope; C != BreakVarScope;
47644782
C = C->getParent())
47654783
C->emitDestruction();
47664784
return this->jump(*BreakLabel);
@@ -4771,8 +4789,8 @@ bool Compiler<Emitter>::visitContinueStmt(const ContinueStmt *S) {
47714789
if (!ContinueLabel)
47724790
return false;
47734791

4774-
for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope;
4775-
C = C->getParent())
4792+
for (VariableScope<Emitter> *C = VarScope;
4793+
C && C->getParent() != ContinueVarScope; C = C->getParent())
47764794
C->emitDestruction();
47774795
return this->jump(*ContinueLabel);
47784796
}
@@ -4781,6 +4799,7 @@ template <class Emitter>
47814799
bool Compiler<Emitter>::visitSwitchStmt(const SwitchStmt *S) {
47824800
const Expr *Cond = S->getCond();
47834801
PrimType CondT = this->classifyPrim(Cond->getType());
4802+
LocalScope<Emitter> LS(this);
47844803

47854804
LabelTy EndLabel = this->getLabel();
47864805
OptLabelTy DefaultLabel = std::nullopt;
@@ -4844,7 +4863,8 @@ bool Compiler<Emitter>::visitSwitchStmt(const SwitchStmt *S) {
48444863
if (!this->visitStmt(S->getBody()))
48454864
return false;
48464865
this->emitLabel(EndLabel);
4847-
return true;
4866+
4867+
return LS.destroyLocals();
48484868
}
48494869

48504870
template <class Emitter>

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

+5-3
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,12 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
409409
/// Switch case mapping.
410410
CaseMap CaseLabels;
411411

412-
/// Scope to cleanup until when chumping to one of the labels.
413-
VariableScope<Emitter> *LabelVarScope = nullptr;
412+
/// Scope to cleanup until when we see a break statement.
413+
VariableScope<Emitter> *BreakVarScope = nullptr;
414414
/// Point to break to.
415415
OptLabelTy BreakLabel;
416+
/// Scope to cleanup until when we see a continue statement.
417+
VariableScope<Emitter> *ContinueVarScope = nullptr;
416418
/// Point to continue to.
417419
OptLabelTy ContinueLabel;
418420
/// Default case label.
@@ -533,7 +535,7 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
533535
return true;
534536
// Emit destructor calls for local variables of record
535537
// type with a destructor.
536-
for (Scope::Local &Local : this->Ctx->Descriptors[*Idx]) {
538+
for (Scope::Local &Local : llvm::reverse(this->Ctx->Descriptors[*Idx])) {
537539
if (!Local.Desc->isPrimitive() && !Local.Desc->isPrimitiveArray()) {
538540
if (!this->Ctx->emitGetPtrLocal(Local.Offset, E))
539541
return false;

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

+76
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,79 @@ namespace Covariant {
3434
constexpr const Covariant1 *cb1 = &cb;
3535
static_assert(cb1->f()->a == 'Z');
3636
}
37+
38+
namespace DtorOrder {
39+
struct Buf {
40+
char buf[64];
41+
int n = 0;
42+
constexpr void operator+=(char c) { buf[n++] = c; }
43+
constexpr bool operator==(const char *str) const {
44+
if (str[n] != 0)
45+
return false;
46+
47+
for (int i = 0; i < n; ++i) {
48+
if (buf[n] != str[n])
49+
return false;
50+
}
51+
return true;
52+
53+
return __builtin_memcmp(str, buf, n) == 0;
54+
}
55+
constexpr bool operator!=(const char *str) const { return !operator==(str); }
56+
};
57+
58+
struct A {
59+
constexpr A(Buf &buf, char c) : buf(buf), c(c) { buf += c; }
60+
constexpr ~A() { buf += (c - 32);}
61+
constexpr operator bool() const { return true; }
62+
Buf &buf;
63+
char c;
64+
};
65+
66+
constexpr void abnormal_termination(Buf &buf) {
67+
struct Indestructible {
68+
constexpr ~Indestructible(); // not defined
69+
};
70+
A a(buf, 'a');
71+
A(buf, 'b');
72+
int n = 0;
73+
74+
for (A &&c = A(buf, 'c'); A d = A(buf, 'd'); A(buf, 'e')) {
75+
switch (A f(buf, 'f'); A g = A(buf, 'g')) { // both-warning {{boolean}}
76+
case false: {
77+
A x(buf, 'x');
78+
}
79+
80+
case true: {
81+
A h(buf, 'h');
82+
switch (n++) {
83+
case 0:
84+
break;
85+
case 1:
86+
continue;
87+
case 2:
88+
return;
89+
}
90+
break;
91+
}
92+
93+
default:
94+
Indestructible indest;
95+
}
96+
97+
A j = (A(buf, 'i'), A(buf, 'j'));
98+
}
99+
}
100+
101+
constexpr bool check_abnormal_termination() {
102+
Buf buf = {};
103+
abnormal_termination(buf);
104+
return buf ==
105+
"abBc"
106+
"dfgh" /*break*/ "HGFijIJeED"
107+
"dfgh" /*continue*/ "HGFeED"
108+
"dfgh" /*return*/ "HGFD"
109+
"CA";
110+
}
111+
static_assert(check_abnormal_termination());
112+
}

0 commit comments

Comments
 (0)