llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ziqing Luo (ziqingluo-90) <details> <summary>Changes</summary> In `VisitObjCForCollectionStmt`, the function does `evalLocation` for the current element at the original source state `Pred`. The evaluation may result in a new state, say `PredNew`. I.e., there is a transition: `Pred -> PredNew`, though it is a very rare case that `Pred` is NOT identical to `PredNew`. (This explains why the bug exists for many years but no one noticed until recently a crash observed downstream.) Later, the original code does NOT use `PredNew` as the new source state in `StmtNodeBuilder` for next transitions. In cases `Pred != PredNew`, the program ill behaves. (rdar://143280254) --- Full diff: https://github.com/llvm/llvm-project/pull/124477.diff 1 Files Affected: - (modified) clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp (+16-14) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index f075df3ab5e4d6..9aa5cee71c1374 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -124,24 +124,26 @@ void ExprEngine::VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S, bool isContainerNull = state->isNull(collectionV).isConstrainedTrue(); - ExplodedNodeSet dstLocation; - evalLocation(dstLocation, S, elem, Pred, state, elementV, false); + ExplodedNodeSet NewPreds; // evalLocation may alter `Pred` + evalLocation(NewPreds, S, elem, Pred, state, elementV, false); - ExplodedNodeSet Tmp; - StmtNodeBuilder Bldr(Pred, Tmp, *currBldrCtx); + for (ExplodedNode *Pred : NewPreds) { + ExplodedNodeSet PredSingleton{Pred}, Tmp; + StmtNodeBuilder Bldr(Pred, Tmp, *currBldrCtx); - if (!isContainerNull) - populateObjCForDestinationSet(dstLocation, svalBuilder, S, elem, elementV, - SymMgr, currBldrCtx, Bldr, - /*hasElements=*/true); + if (!isContainerNull) + populateObjCForDestinationSet(PredSingleton, svalBuilder, S, elem, + elementV, SymMgr, currBldrCtx, Bldr, + /*hasElements=*/true); - populateObjCForDestinationSet(dstLocation, svalBuilder, S, elem, elementV, - SymMgr, currBldrCtx, Bldr, - /*hasElements=*/false); + populateObjCForDestinationSet(PredSingleton, svalBuilder, S, elem, elementV, + SymMgr, currBldrCtx, Bldr, + /*hasElements=*/false); - // Finally, run any custom checkers. - // FIXME: Eventually all pre- and post-checks should live in VisitStmt. - getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this); + // Finally, run any custom checkers. + // FIXME: Eventually all pre- and post-checks should live in VisitStmt. + getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this); + } } void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, `````````` </details> https://github.com/llvm/llvm-project/pull/124477 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
