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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/107738.diff


3 Files Affected:

- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+14-4) 
- (modified) clang/lib/AST/ByteCode/Compiler.h (+2) 
- (modified) clang/test/AST/ByteCode/new-delete.cpp (+19-13) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 46f9c98d59befc..cd25ccdd930237 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -114,7 +114,8 @@ 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;
   }
@@ -122,11 +123,13 @@ template <class Emitter> class LoopScope final : public LabelScope<Emitter> {
   ~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,7 +143,8 @@ 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);
@@ -150,12 +154,14 @@ template <class Emitter> class SwitchScope final : public LabelScope<Emitter> {
     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 {
@@ -4734,7 +4740,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);
 }
 
@@ -4743,7 +4751,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}} \

``````````

</details>


https://github.com/llvm/llvm-project/pull/107738