Author: martinboehme Date: 2024-01-16T15:48:44+01:00 New Revision: 23bfc271a316345459809427d98e942455d0e2b6
URL: https://github.com/llvm/llvm-project/commit/23bfc271a316345459809427d98e942455d0e2b6 DIFF: https://github.com/llvm/llvm-project/commit/23bfc271a316345459809427d98e942455d0e2b6.diff LOG: [clang][dataflow] Use `ignoreCFGOmittedNodes()` in `setValue()`. (#78245) This is to be consistent with `getValue()`, which also uses `ignoreCFGOmittedNodes()`. Before this fix, it was not possible to retrieve a `Value` from a "CFG omitted" node that had previously been set using `setValue()`; see the accompanying test, which fails without the fix. I discovered this issue while running internal integration tests on https://github.com/llvm/llvm-project/pull/78127. Added: Modified: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index a50ee57a3c11b4..57572e5b4e2f13 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -803,13 +803,15 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) { } void Environment::setValue(const Expr &E, Value &Val) { + const Expr &CanonE = ignoreCFGOmittedNodes(E); + if (auto *RecordVal = dyn_cast<RecordValue>(&Val)) { - assert(isOriginalRecordConstructor(E) || - &RecordVal->getLoc() == &getResultObjectLocation(E)); + assert(isOriginalRecordConstructor(CanonE) || + &RecordVal->getLoc() == &getResultObjectLocation(CanonE)); } - assert(E.isPRValue()); - ExprToVal[&E] = &Val; + assert(CanonE.isPRValue()); + ExprToVal[&CanonE] = &Val; } Value *Environment::getValue(const StorageLocation &Loc) const { diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 003434a58b1075..8799d03dfd3c58 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -58,6 +58,52 @@ TEST_F(EnvironmentTest, FlowCondition) { EXPECT_FALSE(Env.allows(NotX)); } +TEST_F(EnvironmentTest, SetAndGetValueOnCfgOmittedNodes) { + // Check that we can set a value on an expression that is omitted from the CFG + // (see `ignoreCFGOmittedNodes()`), then retrieve that same value from the + // expression. This is a regression test; `setValue()` and `getValue()` + // previously did not use `ignoreCFGOmittedNodes()` consistently. + + using namespace ast_matchers; + + std::string Code = R"cc( + struct S { + int f(); + }; + void target() { + // Method call on a temporary produces an `ExprWithCleanups`. + S().f(); + (1); + } + )cc"; + + auto Unit = + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + const ExprWithCleanups *WithCleanups = selectFirst<ExprWithCleanups>( + "cleanups", + match(exprWithCleanups(hasType(isInteger())).bind("cleanups"), Context)); + ASSERT_NE(WithCleanups, nullptr); + + const ParenExpr *Paren = selectFirst<ParenExpr>( + "paren", match(parenExpr(hasType(isInteger())).bind("paren"), Context)); + ASSERT_NE(Paren, nullptr); + + Environment Env(DAContext); + IntegerValue *Val1 = + cast<IntegerValue>(Env.createValue(Unit->getASTContext().IntTy)); + Env.setValue(*WithCleanups, *Val1); + EXPECT_EQ(Env.getValue(*WithCleanups), Val1); + + IntegerValue *Val2 = + cast<IntegerValue>(Env.createValue(Unit->getASTContext().IntTy)); + Env.setValue(*Paren, *Val2); + EXPECT_EQ(Env.getValue(*Paren), Val2); +} + TEST_F(EnvironmentTest, CreateValueRecursiveType) { using namespace ast_matchers; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
