Author: Timm Baeder Date: 2025-04-13T12:25:29+02:00 New Revision: 09588e93bbe486ce782de9fba604f5cd184ec446
URL: https://github.com/llvm/llvm-project/commit/09588e93bbe486ce782de9fba604f5cd184ec446 DIFF: https://github.com/llvm/llvm-project/commit/09588e93bbe486ce782de9fba604f5cd184ec446.diff LOG: [clang][bytecode] Fix an inconsistency with loop condition jumps (#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. Added: Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Compiler.h Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 86b43585cd292..2e22c85ed5f6d 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -5431,39 +5431,39 @@ 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; - - if (Cond) { - if (!this->visitBool(Cond)) - return false; - if (!this->jumpFalse(EndLabel)) - return false; - } - - if (!this->maybeEmitDeferredVarInit(S->getConditionVariable())) - return false; - - if (Body && !this->visitStmt(Body)) + // Start of loop body. + LocalScope<Emitter> CondScope(this); + if (const DeclStmt *CondDecl = S->getConditionVariableDeclStmt()) + if (!visitDeclStmt(CondDecl)) return false; - this->fallthrough(IncLabel); - this->emitLabel(IncLabel); - if (Inc && !this->discard(Inc)) + if (Cond) { + if (!this->visitBool(Cond)) return false; - - if (!CondScope.destroyLocals()) + if (!this->jumpFalse(EndLabel)) return false; } + if (!this->maybeEmitDeferredVarInit(S->getConditionVariable())) + return false; + + if (Body && !this->visitStmt(Body)) + return false; + + this->fallthrough(IncLabel); + this->emitLabel(IncLabel); + if (Inc && !this->discard(Inc)) + return false; + + 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
