https://github.com/tbaederr created
https://github.com/llvm/llvm-project/pull/135530
When emitting the jump for e.g. a for loop condition, we used to jump out of
the CondScope, leaving the scope initialized, because we skipped the
corresponding Destroy opcode. If that loop was in a loop itself, that outer
loop could then iterate once more, leading to us initializing a scope that was
still initialized.
Fix this by also destroying the scope after the EndLabel.
>From 52b7e8519cb4612f0b4dd803762d010145144e24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com>
Date: Sat, 12 Apr 2025 17:40:19 +0200
Subject: [PATCH] [clang][bytecode] Fix an inconsistency with loop condition
jumps
When emitting the jump for e.g. a for loop condition, we used to jump
out of the CondScope, leaving the scope initialized, because we skipped
the corresponding Destroy opcode. If that loop was in a loop itself,
that outer loop could then iterate once more, leading to us initializing
a scope that was still initialized.
Fix this by also destroying the scope after the EndLabel.
---
clang/lib/AST/ByteCode/Compiler.cpp | 31 +++++++++++++++--------------
clang/lib/AST/ByteCode/Compiler.h | 3 ++-
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp
b/clang/lib/AST/ByteCode/Compiler.cpp
index 86b43585cd292..376daec5cd0d2 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -5431,21 +5431,21 @@ bool Compiler<Emitter>::visitForStmt(const ForStmt *S) {
this->fallthrough(CondLabel);
this->emitLabel(CondLabel);
- {
- LocalScope<Emitter> CondScope(this);
- if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt())
- if (!visitDeclStmt(CondDecl))
- return false;
+ // Start of loop body.
+ LocalScope<Emitter> CondScope(this);
+ if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt())
+ if (!visitDeclStmt(CondDecl))
+ return false;
- if (Cond) {
- if (!this->visitBool(Cond))
- return false;
- if (!this->jumpFalse(EndLabel))
- return false;
- }
+ if (!this->maybeEmitDeferredVarInit(S->getConditionVariable()))
+ return false;
- if (!this->maybeEmitDeferredVarInit(S->getConditionVariable()))
+ if (Cond) {
+ if (!this->visitBool(Cond))
+ return false;
+ if (!this->jumpFalse(EndLabel))
return false;
+ }
if (Body && !this->visitStmt(Body))
return false;
@@ -5457,13 +5457,14 @@ bool Compiler<Emitter>::visitForStmt(const ForStmt *S) {
if (!CondScope.destroyLocals())
return false;
- }
if (!this->jump(CondLabel))
return false;
+ // End of loop body.
- this->fallthrough(EndLabel);
this->emitLabel(EndLabel);
- return true;
+ // If we jumped out of the loop above, we still need to clean up the
condition
+ // scope.
+ return CondScope.destroyLocals();
}
template <class Emitter>
diff --git a/clang/lib/AST/ByteCode/Compiler.h
b/clang/lib/AST/ByteCode/Compiler.h
index 256e917728886..858957367d85d 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -531,9 +531,10 @@ template <class Emitter> class LocalScope : public
VariableScope<Emitter> {
if (!Idx)
return true;
+ // NB: We are *not* resetting Idx here as to allow multiple
+ // calls to destroyLocals().
bool Success = this->emitDestructors(E);
this->Ctx->emitDestroy(*Idx, E);
- this->Idx = std::nullopt;
return Success;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
