Skip to content

Commit 0de29f7

Browse files
committed
[clang][bytecode] Fix a variable scope problem with continue/break jumps
Cleaning up _all_ the scopes is a little too much. Only clean up until the point here we started the scope relevant for the break/continue statement.
1 parent d6d6070 commit 0de29f7

File tree

3 files changed

+35
-17
lines changed

3 files changed

+35
-17
lines changed

clang/lib/AST/ByteCode/Compiler.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,22 @@ template <class Emitter> class LoopScope final : public LabelScope<Emitter> {
114114

115115
LoopScope(Compiler<Emitter> *Ctx, LabelTy BreakLabel, LabelTy ContinueLabel)
116116
: LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel),
117-
OldContinueLabel(Ctx->ContinueLabel) {
117+
OldContinueLabel(Ctx->ContinueLabel),
118+
OldLabelVarScope(Ctx->LabelVarScope) {
118119
this->Ctx->BreakLabel = BreakLabel;
119120
this->Ctx->ContinueLabel = ContinueLabel;
120121
}
121122

122123
~LoopScope() {
123124
this->Ctx->BreakLabel = OldBreakLabel;
124125
this->Ctx->ContinueLabel = OldContinueLabel;
126+
this->Ctx->LabelVarScope = OldLabelVarScope;
125127
}
126128

127129
private:
128130
OptLabelTy OldBreakLabel;
129131
OptLabelTy OldContinueLabel;
132+
VariableScope<Emitter> *OldLabelVarScope;
130133
};
131134

132135
// Sets the context for a switch scope, mapping labels.
@@ -140,7 +143,8 @@ template <class Emitter> class SwitchScope final : public LabelScope<Emitter> {
140143
OptLabelTy DefaultLabel)
141144
: LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel),
142145
OldDefaultLabel(this->Ctx->DefaultLabel),
143-
OldCaseLabels(std::move(this->Ctx->CaseLabels)) {
146+
OldCaseLabels(std::move(this->Ctx->CaseLabels)),
147+
OldLabelVarScope(Ctx->LabelVarScope) {
144148
this->Ctx->BreakLabel = BreakLabel;
145149
this->Ctx->DefaultLabel = DefaultLabel;
146150
this->Ctx->CaseLabels = std::move(CaseLabels);
@@ -150,12 +154,14 @@ template <class Emitter> class SwitchScope final : public LabelScope<Emitter> {
150154
this->Ctx->BreakLabel = OldBreakLabel;
151155
this->Ctx->DefaultLabel = OldDefaultLabel;
152156
this->Ctx->CaseLabels = std::move(OldCaseLabels);
157+
this->Ctx->LabelVarScope = OldLabelVarScope;
153158
}
154159

155160
private:
156161
OptLabelTy OldBreakLabel;
157162
OptLabelTy OldDefaultLabel;
158163
CaseMap OldCaseLabels;
164+
VariableScope<Emitter> *OldLabelVarScope;
159165
};
160166

161167
template <class Emitter> class StmtExprScope final {
@@ -4734,7 +4740,9 @@ bool Compiler<Emitter>::visitBreakStmt(const BreakStmt *S) {
47344740
if (!BreakLabel)
47354741
return false;
47364742

4737-
this->emitCleanup();
4743+
for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope;
4744+
C = C->getParent())
4745+
C->emitDestruction();
47384746
return this->jump(*BreakLabel);
47394747
}
47404748

@@ -4743,7 +4751,9 @@ bool Compiler<Emitter>::visitContinueStmt(const ContinueStmt *S) {
47434751
if (!ContinueLabel)
47444752
return false;
47454753

4746-
this->emitCleanup();
4754+
for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope;
4755+
C = C->getParent())
4756+
C->emitDestruction();
47474757
return this->jump(*ContinueLabel);
47484758
}
47494759

clang/lib/AST/ByteCode/Compiler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ 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;
412414
/// Point to break to.
413415
OptLabelTy BreakLabel;
414416
/// Point to continue to.

clang/test/AST/ByteCode/new-delete.cpp

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -618,22 +618,28 @@ namespace OperatorNewDelete {
618618

619619
constexpr bool mismatched(int alloc_kind, int dealloc_kind) {
620620
int *p;
621-
622-
if (alloc_kind == 0)
623-
p = new int; // both-note {{allocation performed here}}
624-
else if (alloc_kind == 1)
625-
p = new int[1]; // both-note {{allocation performed here}}
626-
else if (alloc_kind == 2)
621+
switch (alloc_kind) {
622+
case 0:
623+
p = new int; // both-note {{heap allocation performed here}}
624+
break;
625+
case 1:
626+
p = new int[1]; // both-note {{heap allocation performed here}}
627+
break;
628+
case 2:
627629
p = std::allocator<int>().allocate(1);
628-
629-
630-
if (dealloc_kind == 0)
630+
break;
631+
}
632+
switch (dealloc_kind) {
633+
case 0:
631634
delete p; // both-note {{'delete' used to delete pointer to object allocated with 'std::allocator<...>::allocate'}}
632-
else if (dealloc_kind == 1)
635+
break;
636+
case 1:
633637
delete[] p; // both-note {{'delete' used to delete pointer to object allocated with 'std::allocator<...>::allocate'}}
634-
else if (dealloc_kind == 2)
635-
std::allocator<int>().deallocate(p); // both-note 2{{in call to}}
636-
638+
break;
639+
case 2:
640+
std::allocator<int>().deallocate(p); // both-note 2{{in call}}
641+
break;
642+
}
637643
return true;
638644
}
639645
static_assert(mismatched(0, 2)); // both-error {{constant expression}} \

0 commit comments

Comments
 (0)