[clang] 19e2188 - [clang][dataflow] Return a solution from the solver when `Constraints` are `Satisfiable`.
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 7 11:53:54 PDT 2022
Author: Wei Yi Tee
Date: 2022-07-07T20:53:47+02:00
New Revision: 19e21887eb18aa019000c2384ea7f2c91d937489
URL: https://github.com/llvm/llvm-project/commit/19e21887eb18aa019000c2384ea7f2c91d937489
DIFF: https://github.com/llvm/llvm-project/commit/19e21887eb18aa019000c2384ea7f2c91d937489.diff
LOG: [clang][dataflow] Return a solution from the solver when `Constraints` are `Satisfiable`.
A truth assignment to atomic boolean values which satisfy `Constraints` will be returned if found by the solver.
This gives us more information which can be helpful for debugging or constructing warning messages.
Reviewed By: hlopko, gribozavr2, sgatev
Differential Revision: https://reviews.llvm.org/D129180
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/Solver.h
clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index d87b9cc37b996..358ace0430f62 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -271,17 +271,18 @@ class DataflowAnalysisContext {
AtomicBoolValue &Token, llvm::DenseSet<BoolValue *> &Constraints,
llvm::DenseSet<AtomicBoolValue *> &VisitedTokens);
- /// Returns the result of satisfiability checking on `Constraints`.
- /// Possible return values are:
- /// - `Satisfiable`: There exists a satisfying assignment for `Constraints`.
- /// - `Unsatisfiable`: There is no satisfying assignment for `Constraints`.
- /// - `TimedOut`: The solver gives up on finding a satisfying assignment.
+ /// Returns the outcome of satisfiability checking on `Constraints`.
+ /// Possible outcomes are:
+ /// - `Satisfiable`: A satisfying assignment exists and is returned.
+ /// - `Unsatisfiable`: A satisfying assignment does not exist.
+ /// - `TimedOut`: The search for a satisfying assignment was not completed.
Solver::Result querySolver(llvm::DenseSet<BoolValue *> Constraints);
/// Returns true if the solver is able to prove that there is no satisfying
/// assignment for `Constraints`
bool isUnsatisfiable(llvm::DenseSet<BoolValue *> Constraints) {
- return querySolver(std::move(Constraints)) == Solver::Result::Unsatisfiable;
+ return querySolver(std::move(Constraints)).getStatus() ==
+ Solver::Result::Status::Unsatisfiable;
}
/// Returns a boolean value as a result of substituting `Val` and its sub
diff --git a/clang/include/clang/Analysis/FlowSensitive/Solver.h b/clang/include/clang/Analysis/FlowSensitive/Solver.h
index 6b685b9b3c9a7..b7a14f3484e42 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Solver.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Solver.h
@@ -15,7 +15,9 @@
#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SOLVER_H
#include "clang/Analysis/FlowSensitive/Value.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
namespace clang {
namespace dataflow {
@@ -23,17 +25,58 @@ namespace dataflow {
/// An interface for a SAT solver that can be used by dataflow analyses.
class Solver {
public:
- enum class Result {
- /// Indicates that there exists a satisfying assignment for a boolean
+ struct Result {
+ enum class Status {
+ /// Indicates that there exists a satisfying assignment for a boolean
+ /// formula.
+ Satisfiable,
+
+ /// Indicates that there is no satisfying assignment for a boolean
+ /// formula.
+ Unsatisfiable,
+
+ /// Indicates that the solver gave up trying to find a satisfying
+ /// assignment for a boolean formula.
+ TimedOut,
+ };
+
+ /// A boolean value is set to true or false in a truth assignment.
+ enum class Assignment : uint8_t { AssignedFalse = 0, AssignedTrue = 1 };
+
+ /// Constructs a result indicating that the queried boolean formula is
+ /// satisfiable. The result will hold a solution found by the solver.
+ static Result
+ Satisfiable(llvm::DenseMap<AtomicBoolValue *, Assignment> Solution) {
+ return Result(Status::Satisfiable, std::move(Solution));
+ }
+
+ /// Constructs a result indicating that the queried boolean formula is
+ /// unsatisfiable.
+ static Result Unsatisfiable() { return Result(Status::Unsatisfiable, {}); }
+
+ /// Constructs a result indicating that satisfiability checking on the
+ /// queried boolean formula was not completed.
+ static Result TimedOut() { return Result(Status::TimedOut, {}); }
+
+ /// Returns the status of satisfiability checking on the queried boolean
/// formula.
- Satisfiable,
+ Status getStatus() const { return Status; }
- /// Indicates that there is no satisfying assignment for a boolean formula.
- Unsatisfiable,
+ /// Returns a truth assignment to boolean values that satisfies the queried
+ /// boolean formula if available. Otherwise, an empty optional is returned.
+ llvm::Optional<llvm::DenseMap<AtomicBoolValue *, Assignment>>
+ getSolution() const {
+ return Solution;
+ }
- /// Indicates that the solver gave up trying to find a satisfying assignment
- /// for a boolean formula.
- TimedOut,
+ private:
+ Result(
+ enum Status Status,
+ llvm::Optional<llvm::DenseMap<AtomicBoolValue *, Assignment>> Solution)
+ : Status(Status), Solution(std::move(Solution)) {}
+
+ Status Status;
+ llvm::Optional<llvm::DenseMap<AtomicBoolValue *, Assignment>> Solution;
};
virtual ~Solver() = default;
@@ -44,9 +87,6 @@ class Solver {
/// Requirements:
///
/// All elements in `Vals` must not be null.
- ///
- /// FIXME: Consider returning a model in case the conjunction of `Vals` is
- /// satisfiable so that it can be used to generate warning messages.
virtual Result solve(llvm::DenseSet<BoolValue *> Vals) = 0;
};
diff --git a/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp b/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
index 0e6e70d6d5d47..9b1b7a821405c 100644
--- a/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
+++ b/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
@@ -120,7 +120,13 @@ struct BooleanFormula {
/// clauses in the formula start from the element at index 1.
std::vector<ClauseID> NextWatched;
- explicit BooleanFormula(Variable LargestVar) : LargestVar(LargestVar) {
+ /// Stores the variable identifier and value location for atomic booleans in
+ /// the formula.
+ llvm::DenseMap<Variable, AtomicBoolValue *> Atomics;
+
+ explicit BooleanFormula(Variable LargestVar,
+ llvm::DenseMap<Variable, AtomicBoolValue *> Atomics)
+ : LargestVar(LargestVar), Atomics(std::move(Atomics)) {
Clauses.push_back(0);
ClauseStarts.push_back(0);
NextWatched.push_back(0);
@@ -180,28 +186,47 @@ BooleanFormula buildBooleanFormula(const llvm::DenseSet<BoolValue *> &Vals) {
// Map each sub-value in `Vals` to a unique variable.
llvm::DenseMap<BoolValue *, Variable> SubValsToVar;
+ // Store variable identifiers and value location of atomic booleans.
+ llvm::DenseMap<Variable, AtomicBoolValue *> Atomics;
Variable NextVar = 1;
{
std::queue<BoolValue *> UnprocessedSubVals;
for (BoolValue *Val : Vals)
UnprocessedSubVals.push(Val);
while (!UnprocessedSubVals.empty()) {
+ Variable Var = NextVar;
BoolValue *Val = UnprocessedSubVals.front();
UnprocessedSubVals.pop();
- if (!SubValsToVar.try_emplace(Val, NextVar).second)
+ if (!SubValsToVar.try_emplace(Val, Var).second)
continue;
++NextVar;
// Visit the sub-values of `Val`.
- if (auto *C = dyn_cast<ConjunctionValue>(Val)) {
+ switch (Val->getKind()) {
+ case Value::Kind::Conjunction: {
+ auto *C = cast<ConjunctionValue>(Val);
UnprocessedSubVals.push(&C->getLeftSubValue());
UnprocessedSubVals.push(&C->getRightSubValue());
- } else if (auto *D = dyn_cast<DisjunctionValue>(Val)) {
+ break;
+ }
+ case Value::Kind::Disjunction: {
+ auto *D = cast<DisjunctionValue>(Val);
UnprocessedSubVals.push(&D->getLeftSubValue());
UnprocessedSubVals.push(&D->getRightSubValue());
- } else if (auto *N = dyn_cast<NegationValue>(Val)) {
+ break;
+ }
+ case Value::Kind::Negation: {
+ auto *N = cast<NegationValue>(Val);
UnprocessedSubVals.push(&N->getSubVal());
+ break;
+ }
+ case Value::Kind::AtomicBool: {
+ Atomics[Var] = cast<AtomicBoolValue>(Val);
+ break;
+ }
+ default:
+ llvm_unreachable("buildBooleanFormula: unhandled value kind");
}
}
}
@@ -212,7 +237,7 @@ BooleanFormula buildBooleanFormula(const llvm::DenseSet<BoolValue *> &Vals) {
return ValIt->second;
};
- BooleanFormula Formula(NextVar - 1);
+ BooleanFormula Formula(NextVar - 1, std::move(Atomics));
std::vector<bool> ProcessedSubVals(NextVar, false);
// Add a conjunct for each variable that represents a top-level conjunction
@@ -383,7 +408,7 @@ class WatchedLiteralsSolverImpl {
// If the root level is reached, then all possible assignments lead to
// a conflict.
if (Level == 0)
- return WatchedLiteralsSolver::Result::Unsatisfiable;
+ return Solver::Result::Unsatisfiable();
// Otherwise, take the other branch at the most recent level where a
// decision was made.
@@ -440,12 +465,28 @@ class WatchedLiteralsSolverImpl {
++I;
}
}
- return WatchedLiteralsSolver::Result::Satisfiable;
+ return Solver::Result::Satisfiable(buildSolution());
}
private:
- // Reverses forced moves until the most recent level where a decision was made
- // on the assignment of a variable.
+ /// Returns a satisfying truth assignment to the atomic values in the boolean
+ /// formula.
+ llvm::DenseMap<AtomicBoolValue *, Solver::Result::Assignment>
+ buildSolution() {
+ llvm::DenseMap<AtomicBoolValue *, Solver::Result::Assignment> Solution;
+ for (auto [Var, Val] : Formula.Atomics) {
+ // A variable may have a definite true/false assignment, or it may be
+ // unassigned indicating its truth value does not affect the result of
+ // the formula. Unassigned variables are assigned to true as a default.
+ Solution[Val] = VarAssignments[Var] == Assignment::AssignedFalse
+ ? Solver::Result::Assignment::AssignedFalse
+ : Solver::Result::Assignment::AssignedTrue;
+ }
+ return Solution;
+ }
+
+ /// Reverses forced moves until the most recent level where a decision was
+ /// made on the assignment of a variable.
void reverseForcedMoves() {
for (; LevelStates[Level] == State::Forced; --Level) {
const Variable Var = LevelVars[Level];
@@ -459,7 +500,7 @@ class WatchedLiteralsSolverImpl {
}
}
- // Updates watched literals that are affected by a variable assignment.
+ /// Updates watched literals that are affected by a variable assignment.
void updateWatchedLiterals() {
const Variable Var = LevelVars[Level];
@@ -592,7 +633,7 @@ class WatchedLiteralsSolverImpl {
};
Solver::Result WatchedLiteralsSolver::solve(llvm::DenseSet<BoolValue *> Vals) {
- return Vals.empty() ? WatchedLiteralsSolver::Result::Satisfiable
+ return Vals.empty() ? Solver::Result::Satisfiable({{}})
: WatchedLiteralsSolverImpl(Vals).solve();
}
diff --git a/clang/unittests/Analysis/FlowSensitive/SolverTest.cpp b/clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
index c5116b9edbe05..cfce7a09e1220 100644
--- a/clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
@@ -20,6 +20,12 @@ namespace {
using namespace clang;
using namespace dataflow;
+using testing::_;
+using testing::AnyOf;
+using testing::Optional;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
class SolverTest : public ::testing::Test {
protected:
// Checks if the conjunction of `Vals` is satisfiable and returns the
@@ -64,6 +70,17 @@ class SolverTest : public ::testing::Test {
return conj(impl(LeftSubVal, RightSubVal), impl(RightSubVal, LeftSubVal));
}
+ void expectUnsatisfiable(Solver::Result Result) {
+ EXPECT_EQ(Result.getStatus(), Solver::Result::Status::Unsatisfiable);
+ EXPECT_FALSE(Result.getSolution().has_value());
+ }
+
+ template <typename Matcher>
+ void expectSatisfiable(Solver::Result Result, Matcher Solution) {
+ EXPECT_EQ(Result.getStatus(), Solver::Result::Status::Satisfiable);
+ EXPECT_THAT(Result.getSolution(), Optional(Solution));
+ }
+
private:
std::vector<std::unique_ptr<BoolValue>> Vals;
};
@@ -72,7 +89,9 @@ TEST_F(SolverTest, Var) {
auto X = atom();
// X
- EXPECT_EQ(solve({X}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({X}),
+ UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedTrue)));
}
TEST_F(SolverTest, NegatedVar) {
@@ -80,7 +99,9 @@ TEST_F(SolverTest, NegatedVar) {
auto NotX = neg(X);
// !X
- EXPECT_EQ(solve({NotX}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({NotX}),
+ UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedFalse)));
}
TEST_F(SolverTest, UnitConflict) {
@@ -88,7 +109,7 @@ TEST_F(SolverTest, UnitConflict) {
auto NotX = neg(X);
// X ^ !X
- EXPECT_EQ(solve({X, NotX}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({X, NotX}));
}
TEST_F(SolverTest, DistinctVars) {
@@ -97,7 +118,10 @@ TEST_F(SolverTest, DistinctVars) {
auto NotY = neg(Y);
// X ^ !Y
- EXPECT_EQ(solve({X, NotY}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({X, NotY}),
+ UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedTrue),
+ Pair(Y, Solver::Result::Assignment::AssignedFalse)));
}
TEST_F(SolverTest, DoubleNegation) {
@@ -106,7 +130,7 @@ TEST_F(SolverTest, DoubleNegation) {
auto NotNotX = neg(NotX);
// !!X ^ !X
- EXPECT_EQ(solve({NotNotX, NotX}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({NotNotX, NotX}));
}
TEST_F(SolverTest, NegatedDisjunction) {
@@ -116,7 +140,7 @@ TEST_F(SolverTest, NegatedDisjunction) {
auto NotXOrY = neg(XOrY);
// !(X v Y) ^ (X v Y)
- EXPECT_EQ(solve({NotXOrY, XOrY}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({NotXOrY, XOrY}));
}
TEST_F(SolverTest, NegatedConjunction) {
@@ -126,7 +150,7 @@ TEST_F(SolverTest, NegatedConjunction) {
auto NotXAndY = neg(XAndY);
// !(X ^ Y) ^ (X ^ Y)
- EXPECT_EQ(solve({NotXAndY, XAndY}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({NotXAndY, XAndY}));
}
TEST_F(SolverTest, DisjunctionSameVars) {
@@ -135,7 +159,7 @@ TEST_F(SolverTest, DisjunctionSameVars) {
auto XOrNotX = disj(X, NotX);
// X v !X
- EXPECT_EQ(solve({XOrNotX}), Solver::Result::Satisfiable);
+ expectSatisfiable(solve({XOrNotX}), _);
}
TEST_F(SolverTest, ConjunctionSameVarsConflict) {
@@ -144,7 +168,7 @@ TEST_F(SolverTest, ConjunctionSameVarsConflict) {
auto XAndNotX = conj(X, NotX);
// X ^ !X
- EXPECT_EQ(solve({XAndNotX}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({XAndNotX}));
}
TEST_F(SolverTest, PureVar) {
@@ -156,7 +180,10 @@ TEST_F(SolverTest, PureVar) {
auto NotXOrNotY = disj(NotX, NotY);
// (!X v Y) ^ (!X v !Y)
- EXPECT_EQ(solve({NotXOrY, NotXOrNotY}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({NotXOrY, NotXOrNotY}),
+ UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedFalse),
+ Pair(Y, _)));
}
TEST_F(SolverTest, MustAssumeVarIsFalse) {
@@ -169,7 +196,10 @@ TEST_F(SolverTest, MustAssumeVarIsFalse) {
auto NotXOrNotY = disj(NotX, NotY);
// (X v Y) ^ (!X v Y) ^ (!X v !Y)
- EXPECT_EQ(solve({XOrY, NotXOrY, NotXOrNotY}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({XOrY, NotXOrY, NotXOrNotY}),
+ UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedFalse),
+ Pair(Y, Solver::Result::Assignment::AssignedTrue)));
}
TEST_F(SolverTest, DeepConflict) {
@@ -183,8 +213,7 @@ TEST_F(SolverTest, DeepConflict) {
auto XOrNotY = disj(X, NotY);
// (X v Y) ^ (!X v Y) ^ (!X v !Y) ^ (X v !Y)
- EXPECT_EQ(solve({XOrY, NotXOrY, NotXOrNotY, XOrNotY}),
- Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({XOrY, NotXOrY, NotXOrNotY, XOrNotY}));
}
TEST_F(SolverTest, IffSameVars) {
@@ -192,7 +221,7 @@ TEST_F(SolverTest, IffSameVars) {
auto XEqX = iff(X, X);
// X <=> X
- EXPECT_EQ(solve({XEqX}), Solver::Result::Satisfiable);
+ expectSatisfiable(solve({XEqX}), _);
}
TEST_F(SolverTest, IffDistinctVars) {
@@ -201,7 +230,14 @@ TEST_F(SolverTest, IffDistinctVars) {
auto XEqY = iff(X, Y);
// X <=> Y
- EXPECT_EQ(solve({XEqY}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({XEqY}),
+ AnyOf(UnorderedElementsAre(
+ Pair(X, Solver::Result::Assignment::AssignedTrue),
+ Pair(Y, Solver::Result::Assignment::AssignedTrue)),
+ UnorderedElementsAre(
+ Pair(X, Solver::Result::Assignment::AssignedFalse),
+ Pair(Y, Solver::Result::Assignment::AssignedFalse))));
}
TEST_F(SolverTest, IffWithUnits) {
@@ -210,7 +246,10 @@ TEST_F(SolverTest, IffWithUnits) {
auto XEqY = iff(X, Y);
// (X <=> Y) ^ X ^ Y
- EXPECT_EQ(solve({XEqY, X, Y}), Solver::Result::Satisfiable);
+ expectSatisfiable(
+ solve({XEqY, X, Y}),
+ UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedTrue),
+ Pair(Y, Solver::Result::Assignment::AssignedTrue)));
}
TEST_F(SolverTest, IffWithUnitsConflict) {
@@ -220,7 +259,7 @@ TEST_F(SolverTest, IffWithUnitsConflict) {
auto NotY = neg(Y);
// (X <=> Y) ^ X !Y
- EXPECT_EQ(solve({XEqY, X, NotY}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({XEqY, X, NotY}));
}
TEST_F(SolverTest, IffTransitiveConflict) {
@@ -232,7 +271,7 @@ TEST_F(SolverTest, IffTransitiveConflict) {
auto NotX = neg(X);
// (X <=> Y) ^ (Y <=> Z) ^ Z ^ !X
- EXPECT_EQ(solve({XEqY, YEqZ, Z, NotX}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({XEqY, YEqZ, Z, NotX}));
}
TEST_F(SolverTest, DeMorgan) {
@@ -248,7 +287,7 @@ TEST_F(SolverTest, DeMorgan) {
auto B = iff(neg(conj(Z, W)), disj(neg(Z), neg(W)));
// A ^ B
- EXPECT_EQ(solve({A, B}), Solver::Result::Satisfiable);
+ expectSatisfiable(solve({A, B}), _);
}
TEST_F(SolverTest, RespectsAdditionalConstraints) {
@@ -258,7 +297,7 @@ TEST_F(SolverTest, RespectsAdditionalConstraints) {
auto NotY = neg(Y);
// (X <=> Y) ^ X ^ !Y
- EXPECT_EQ(solve({XEqY, X, NotY}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({XEqY, X, NotY}));
}
TEST_F(SolverTest, ImplicationConflict) {
@@ -268,7 +307,7 @@ TEST_F(SolverTest, ImplicationConflict) {
auto *XAndNotY = conj(X, neg(Y));
// X => Y ^ X ^ !Y
- EXPECT_EQ(solve({XImplY, XAndNotY}), Solver::Result::Unsatisfiable);
+ expectUnsatisfiable(solve({XImplY, XAndNotY}));
}
} // namespace
More information about the cfe-commits
mailing list