llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: None (martinboehme) <details> <summary>Changes</summary> `ConstantExpr` does not appear as a `CFGStmt` in the CFG, so `StmtToEnvMap::getEnvironment()` was not finding an entry for it in the map, causing a crash when we tried to access the iterator resulting from the map lookup. The fix is to make `ignoreCFGOmittedNodes()` ignore `ConstantExpr`, but in addition, I'm hardening `StmtToEnvMap::getEnvironment()` to make sure release builds don't crash in similar situations in the future. --- Full diff: https://github.com/llvm/llvm-project/pull/90112.diff 3 Files Affected: - (modified) clang/lib/Analysis/FlowSensitive/ASTOps.cpp (+12-4) - (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+5-1) - (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+32) ``````````diff diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index 619bf772bba5ee..bd1676583ecccd 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp @@ -33,12 +33,20 @@ namespace clang::dataflow { const Expr &ignoreCFGOmittedNodes(const Expr &E) { const Expr *Current = &E; - if (auto *EWC = dyn_cast<ExprWithCleanups>(Current)) { - Current = EWC->getSubExpr(); + const Expr *Last = nullptr; + while (Current != Last) { + Last = Current; + if (auto *EWC = dyn_cast<ExprWithCleanups>(Current)) { + Current = EWC->getSubExpr(); + assert(Current != nullptr); + } + if (auto *CE = dyn_cast<ConstantExpr>(Current)) { + Current = CE->getSubExpr(); + assert(Current != nullptr); + } + Current = Current->IgnoreParens(); assert(Current != nullptr); } - Current = Current->IgnoreParens(); - assert(Current != nullptr); return *Current; } diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 43fdfa5abcbb51..fd224aeb79b151 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -41,7 +41,11 @@ namespace dataflow { const Environment *StmtToEnvMap::getEnvironment(const Stmt &S) const { auto BlockIt = ACFG.getStmtToBlock().find(&ignoreCFGOmittedNodes(S)); - assert(BlockIt != ACFG.getStmtToBlock().end()); + if (BlockIt == ACFG.getStmtToBlock().end()) { + assert(false); + // Return null to avoid dereferencing the end iterator in non-assert builds. + return nullptr; + } if (!ACFG.isBlockReachable(*BlockIt->getSecond())) return nullptr; if (BlockIt->getSecond()->getBlockID() == CurBlockID) diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index d204700919d315..301bec32c0cf1d 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -5357,6 +5357,38 @@ TEST(TransferTest, ConditionalOperatorLocation) { }); } +TEST(TransferTest, ConditionalOperatorOnConstantExpr) { + // This is a regression test: We used to crash when a `ConstantExpr` was used + // in the branches of a conditional operator. + std::string Code = R"cc( + consteval bool identity(bool B) { return B; } + void target(bool Cond) { + bool JoinTrueTrue = Cond ? identity(true) : identity(true); + bool JoinTrueFalse = Cond ? identity(true) : identity(false); + // [[p]] + } + )cc"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + Environment Env = getEnvironmentAtAnnotation(Results, "p").fork(); + + auto &JoinTrueTrue = + getValueForDecl<BoolValue>(ASTCtx, Env, "JoinTrueTrue"); + // FIXME: This test documents the current behavior, namely that we + // don't actually use the constant result of the `ConstantExpr` and + // instead treat it like a normal function call. + EXPECT_EQ(JoinTrueTrue.formula().kind(), Formula::Kind::AtomRef); + // EXPECT_TRUE(JoinTrueTrue.formula().literal()); + + auto &JoinTrueFalse = + getValueForDecl<BoolValue>(ASTCtx, Env, "JoinTrueFalse"); + EXPECT_EQ(JoinTrueFalse.formula().kind(), Formula::Kind::AtomRef); + }, + LangStandard::lang_cxx20); +} + TEST(TransferTest, IfStmtBranchExtendsFlowCondition) { std::string Code = R"( void target(bool Foo) { `````````` </details> https://github.com/llvm/llvm-project/pull/90112 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
