ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.
Ensure that the expressions associated with terminators are associated with a
value. Otherwise, we can generate degenerate flow conditions, where both
branches share the same condition.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D123858
Files:
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2890,6 +2890,68 @@
});
}
+TEST_F(TransferTest, OpaqueFlowConditionMergesToOpaqueBool) {
+ std::string Code = R"(
+ bool foo();
+
+ void target() {
+ bool Bar = true;
+ if (foo())
+ Bar = false;
+ (void)0;
+ /*[[p]]*/
+ }
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ ASSERT_THAT(BarDecl, NotNull());
+
+ auto &BarVal =
+ *cast<BoolValue>(Env.getValue(*BarDecl, SkipPast::Reference));
+
+ EXPECT_FALSE(Env.flowConditionImplies(BarVal));
+ });
+}
+
+TEST_F(TransferTest, OpaqueFlowConditionInsideBranchMergesToOpaqueBool) {
+ std::string Code = R"(
+ bool foo();
+
+ void target(bool Cond) {
+ bool Bar = true;
+ if (Cond) {
+ if (foo())
+ Bar = false;
+ (void)0;
+ /*[[p]]*/
+ }
+ }
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ ASSERT_THAT(BarDecl, NotNull());
+
+ auto &BarVal =
+ *cast<BoolValue>(Env.getValue(*BarDecl, SkipPast::Reference));
+
+ EXPECT_FALSE(Env.flowConditionImplies(BarVal));
+ });
+}
+
TEST_F(TransferTest, CorrelatedBranches) {
std::string Code = R"(
void target(bool B, bool C) {
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -32,6 +32,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
namespace clang {
namespace dataflow {
@@ -108,8 +109,32 @@
auto *Val =
cast_or_null<BoolValue>(Env.getValue(Cond, SkipPast::Reference));
- if (Val == nullptr)
- return;
+ // Value merging depends on flow conditions from different environments
+ // being mutually exclusive. So, we need *some* value for the condition
+ // expression, even if just an atom.
+ if (Val == nullptr) {
+ Val = &Env.makeAtomicBoolValue();
+ QualType Type = Cond.getType();
+ assert(Type->isBooleanType() || Type->isReferenceType());
+ if (Type->isBooleanType()) {
+ auto &Loc = Env.createStorageLocation(Cond);
+ Env.setStorageLocation(Cond, Loc);
+ Env.setValue(Loc, *Val);
+ } else if (auto *CondVal = cast_or_null<ReferenceValue>(
+ Env.getValue(Cond, SkipPast::None))) {
+ Env.setValue(CondVal->getPointeeLoc(), *Val);
+ } else {
+ QualType PointeeType = Type->castAs<ReferenceType>()->getPointeeType();
+ StorageLocation &PointeeLoc = Env.createStorageLocation(PointeeType);
+ Env.setValue(PointeeLoc, *Val);
+ auto *Loc = Env.getStorageLocation(Cond, SkipPast::None);
+ // `Cond` must have been processed already (and so must have an
+ // associated location) since it came from the predecessor block.
+ assert(Loc != nullptr);
+ Env.setValue(*Loc, Env.takeOwnership(
+ std::make_unique<ReferenceValue>(PointeeLoc)));
+ }
+ }
// The condition must be inverted for the successor that encompasses the
// "else" branch, if such exists.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
