llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/135530.diff 2 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+16-15) - (modified) clang/lib/AST/ByteCode/Compiler.h (+2-1) ``````````diff 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; } `````````` </details> https://github.com/llvm/llvm-project/pull/135530 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
