Skip to content

Commit 6f67c38

Browse files
authored
[clang][bytecode] Fix a variable scope problem with continue/break jumps (#107738)
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 f5b7c10 commit 6f67c38

File tree

3 files changed

+37
-17
lines changed

3 files changed

+37
-17
lines changed

clang/lib/AST/ByteCode/Compiler.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,23 @@ 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;
121+
this->Ctx->LabelVarScope = this->Ctx->VarScope;
120122
}
121123

122124
~LoopScope() {
123125
this->Ctx->BreakLabel = OldBreakLabel;
124126
this->Ctx->ContinueLabel = OldContinueLabel;
127+
this->Ctx->LabelVarScope = OldLabelVarScope;
125128
}
126129

127130
private:
128131
OptLabelTy OldBreakLabel;
129132
OptLabelTy OldContinueLabel;
133+
VariableScope<Emitter> *OldLabelVarScope;
130134
};
131135

132136
// Sets the context for a switch scope, mapping labels.
@@ -140,22 +144,26 @@ template <class Emitter> class SwitchScope final : public LabelScope<Emitter> {
140144
OptLabelTy DefaultLabel)
141145
: LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel),
142146
OldDefaultLabel(this->Ctx->DefaultLabel),
143-
OldCaseLabels(std::move(this->Ctx->CaseLabels)) {
147+
OldCaseLabels(std::move(this->Ctx->CaseLabels)),
148+
OldLabelVarScope(Ctx->LabelVarScope) {
144149
this->Ctx->BreakLabel = BreakLabel;
145150
this->Ctx->DefaultLabel = DefaultLabel;
146151
this->Ctx->CaseLabels = std::move(CaseLabels);
152+
this->Ctx->LabelVarScope = this->Ctx->VarScope;
147153
}
148154

149155
~SwitchScope() {
150156
this->Ctx->BreakLabel = OldBreakLabel;
151157
this->Ctx->DefaultLabel = OldDefaultLabel;
152158
this->Ctx->CaseLabels = std::move(OldCaseLabels);
159+
this->Ctx->LabelVarScope = OldLabelVarScope;
153160
}
154161

155162
private:
156163
OptLabelTy OldBreakLabel;
157164
OptLabelTy OldDefaultLabel;
158165
CaseMap OldCaseLabels;
166+
VariableScope<Emitter> *OldLabelVarScope;
159167
};
160168

161169
template <class Emitter> class StmtExprScope final {
@@ -4752,7 +4760,9 @@ bool Compiler<Emitter>::visitBreakStmt(const BreakStmt *S) {
47524760
if (!BreakLabel)
47534761
return false;
47544762

4755-
this->emitCleanup();
4763+
for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope;
4764+
C = C->getParent())
4765+
C->emitDestruction();
47564766
return this->jump(*BreakLabel);
47574767
}
47584768

@@ -4761,7 +4771,9 @@ bool Compiler<Emitter>::visitContinueStmt(const ContinueStmt *S) {
47614771
if (!ContinueLabel)
47624772
return false;
47634773

4764-
this->emitCleanup();
4774+
for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope;
4775+
C = C->getParent())
4776+
C->emitDestruction();
47654777
return this->jump(*ContinueLabel);
47664778
}
47674779

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)