Author: Timm Baeder Date: 2024-09-08T19:22:18+02:00 New Revision: 6f67c386845be85cfcbf5c90949edcdaf40a0ef7
URL: https://github.com/llvm/llvm-project/commit/6f67c386845be85cfcbf5c90949edcdaf40a0ef7 DIFF: https://github.com/llvm/llvm-project/commit/6f67c386845be85cfcbf5c90949edcdaf40a0ef7.diff LOG: [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. Added: Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Compiler.h clang/test/AST/ByteCode/new-delete.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 43268bafc387bd..fddd6db4f4814c 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -114,19 +114,23 @@ template <class Emitter> class LoopScope final : public LabelScope<Emitter> { LoopScope(Compiler<Emitter> *Ctx, LabelTy BreakLabel, LabelTy ContinueLabel) : LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel), - OldContinueLabel(Ctx->ContinueLabel) { + OldContinueLabel(Ctx->ContinueLabel), + OldLabelVarScope(Ctx->LabelVarScope) { this->Ctx->BreakLabel = BreakLabel; this->Ctx->ContinueLabel = ContinueLabel; + this->Ctx->LabelVarScope = this->Ctx->VarScope; } ~LoopScope() { this->Ctx->BreakLabel = OldBreakLabel; this->Ctx->ContinueLabel = OldContinueLabel; + this->Ctx->LabelVarScope = OldLabelVarScope; } private: OptLabelTy OldBreakLabel; OptLabelTy OldContinueLabel; + VariableScope<Emitter> *OldLabelVarScope; }; // Sets the context for a switch scope, mapping labels. @@ -140,22 +144,26 @@ template <class Emitter> class SwitchScope final : public LabelScope<Emitter> { OptLabelTy DefaultLabel) : LabelScope<Emitter>(Ctx), OldBreakLabel(Ctx->BreakLabel), OldDefaultLabel(this->Ctx->DefaultLabel), - OldCaseLabels(std::move(this->Ctx->CaseLabels)) { + OldCaseLabels(std::move(this->Ctx->CaseLabels)), + OldLabelVarScope(Ctx->LabelVarScope) { this->Ctx->BreakLabel = BreakLabel; this->Ctx->DefaultLabel = DefaultLabel; this->Ctx->CaseLabels = std::move(CaseLabels); + this->Ctx->LabelVarScope = this->Ctx->VarScope; } ~SwitchScope() { this->Ctx->BreakLabel = OldBreakLabel; this->Ctx->DefaultLabel = OldDefaultLabel; this->Ctx->CaseLabels = std::move(OldCaseLabels); + this->Ctx->LabelVarScope = OldLabelVarScope; } private: OptLabelTy OldBreakLabel; OptLabelTy OldDefaultLabel; CaseMap OldCaseLabels; + VariableScope<Emitter> *OldLabelVarScope; }; template <class Emitter> class StmtExprScope final { @@ -4752,7 +4760,9 @@ bool Compiler<Emitter>::visitBreakStmt(const BreakStmt *S) { if (!BreakLabel) return false; - this->emitCleanup(); + for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope; + C = C->getParent()) + C->emitDestruction(); return this->jump(*BreakLabel); } @@ -4761,7 +4771,9 @@ bool Compiler<Emitter>::visitContinueStmt(const ContinueStmt *S) { if (!ContinueLabel) return false; - this->emitCleanup(); + for (VariableScope<Emitter> *C = VarScope; C != LabelVarScope; + C = C->getParent()) + C->emitDestruction(); return this->jump(*ContinueLabel); } diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 39c0736cb4e27e..ac4c08c4d0ffb6 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -409,6 +409,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, /// Switch case mapping. CaseMap CaseLabels; + /// Scope to cleanup until when chumping to one of the labels. + VariableScope<Emitter> *LabelVarScope = nullptr; /// Point to break to. OptLabelTy BreakLabel; /// Point to continue to. diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp index 902ab4aab10fb5..76858aa94bb37d 100644 --- a/clang/test/AST/ByteCode/new-delete.cpp +++ b/clang/test/AST/ByteCode/new-delete.cpp @@ -618,22 +618,28 @@ namespace OperatorNewDelete { constexpr bool mismatched(int alloc_kind, int dealloc_kind) { int *p; - - if (alloc_kind == 0) - p = new int; // both-note {{allocation performed here}} - else if (alloc_kind == 1) - p = new int[1]; // both-note {{allocation performed here}} - else if (alloc_kind == 2) + switch (alloc_kind) { + case 0: + p = new int; // both-note {{heap allocation performed here}} + break; + case 1: + p = new int[1]; // both-note {{heap allocation performed here}} + break; + case 2: p = std::allocator<int>().allocate(1); - - - if (dealloc_kind == 0) + break; + } + switch (dealloc_kind) { + case 0: delete p; // both-note {{'delete' used to delete pointer to object allocated with 'std::allocator<...>::allocate'}} - else if (dealloc_kind == 1) + break; + case 1: delete[] p; // both-note {{'delete' used to delete pointer to object allocated with 'std::allocator<...>::allocate'}} - else if (dealloc_kind == 2) - std::allocator<int>().deallocate(p); // both-note 2{{in call to}} - + break; + case 2: + std::allocator<int>().deallocate(p); // both-note 2{{in call}} + break; + } return true; } static_assert(mismatched(0, 2)); // both-error {{constant expression}} \ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
