[clang] 6f67c38 - [clang][bytecode] Fix a variable scope problem with continue/break jumps (#107738)

[via cfe-commits]

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}} \