[clang] 8fcdd62 - [clang][dataflow] Add support for correlated branches to optional model
Stanislav Gatev via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 15 03:04:27 PDT 2022
Author: Stanislav Gatev
Date: 2022-06-15T10:00:44Z
New Revision: 8fcdd625856b2e7df2fdb3a4c57efedb35e4d7c1
URL: https://github.com/llvm/llvm-project/commit/8fcdd625856b2e7df2fdb3a4c57efedb35e4d7c1
DIFF: https://github.com/llvm/llvm-project/commit/8fcdd625856b2e7df2fdb3a4c57efedb35e4d7c1.diff
LOG: [clang][dataflow] Add support for correlated branches to optional model
Add support for correlated branches to the std::optional dataflow model.
Differential Revision: https://reviews.llvm.org/D125931
Reviewed-by: ymandel, xazax.hun
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 8cc3fa73de9d..2102ed56907b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -59,6 +59,14 @@ class UncheckedOptionalAccessModel
void transfer(const Stmt *Stmt, SourceLocationsLattice &State,
Environment &Env);
+ bool compareEquivalent(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) override;
+
+ bool merge(QualType Type, const Value &Val1, const Environment &Env1,
+ const Value &Val2, const Environment &Env2, Value &MergedVal,
+ Environment &MergedEnv) override;
+
private:
MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
};
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 23a9d964b5b1..502974186621 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -169,11 +169,17 @@ auto isCallReturningOptional() {
optionalOrAliasType(), referenceType(pointee(optionalOrAliasType()))))));
}
+/// Sets `HasValueVal` as the symbolic value that represents the "has_value"
+/// property of the optional value `OptionalVal`.
+void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {
+ OptionalVal.setProperty("has_value", HasValueVal);
+}
+
/// Creates a symbolic value for an `optional` value using `HasValueVal` as the
/// symbolic value of its "has_value" property.
StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
auto OptionalVal = std::make_unique<StructValue>();
- OptionalVal->setProperty("has_value", HasValueVal);
+ setHasValue(*OptionalVal, HasValueVal);
return Env.takeOwnership(std::move(OptionalVal));
}
@@ -274,11 +280,28 @@ void initializeOptionalReference(const Expr *OptionalExpr,
if (auto *OptionalVal =
State.Env.getValue(*OptionalExpr, SkipPast::Reference)) {
if (OptionalVal->getProperty("has_value") == nullptr) {
- OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue());
+ setHasValue(*OptionalVal, State.Env.makeAtomicBoolValue());
}
}
}
+/// Returns true if and only if `OptionalVal` is initialized and known to be
+/// empty in `Env.
+bool isEmptyOptional(const Value &OptionalVal, const Environment &Env) {
+ auto *HasValueVal =
+ cast_or_null<BoolValue>(OptionalVal.getProperty("has_value"));
+ return HasValueVal != nullptr &&
+ Env.flowConditionImplies(Env.makeNot(*HasValueVal));
+}
+
+/// Returns true if and only if `OptionalVal` is initialized and known to be
+/// non-empty in `Env.
+bool isNonEmptyOptional(const Value &OptionalVal, const Environment &Env) {
+ auto *HasValueVal =
+ cast_or_null<BoolValue>(OptionalVal.getProperty("has_value"));
+ return HasValueVal != nullptr && Env.flowConditionImplies(*HasValueVal);
+}
+
void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
LatticeTransferState &State) {
if (auto *OptionalVal =
@@ -651,5 +674,31 @@ void UncheckedOptionalAccessModel::transfer(const Stmt *S,
TransferMatchSwitch(*S, getASTContext(), State);
}
+bool UncheckedOptionalAccessModel::compareEquivalent(QualType Type,
+ const Value &Val1,
+ const Environment &Env1,
+ const Value &Val2,
+ const Environment &Env2) {
+ return isNonEmptyOptional(Val1, Env1) == isNonEmptyOptional(Val2, Env2);
+}
+
+bool UncheckedOptionalAccessModel::merge(QualType Type, const Value &Val1,
+ const Environment &Env1,
+ const Value &Val2,
+ const Environment &Env2,
+ Value &MergedVal,
+ Environment &MergedEnv) {
+ if (!IsOptionalType(Type))
+ return true;
+
+ auto &HasValueVal = MergedEnv.makeAtomicBoolValue();
+ if (isNonEmptyOptional(Val1, Env1) && isNonEmptyOptional(Val2, Env2))
+ MergedEnv.addToFlowCondition(HasValueVal);
+ else if (isEmptyOptional(Val1, Env1) && isEmptyOptional(Val2, Env2))
+ MergedEnv.addToFlowCondition(MergedEnv.makeNot(HasValueVal));
+ setHasValue(MergedVal, HasValueVal);
+ return true;
+}
+
} // namespace dataflow
} // namespace clang
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 3c37bec82139..912754ebe8b6 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1861,7 +1861,26 @@ TEST_P(UncheckedOptionalAccessTest, Emplace) {
)",
UnorderedElementsAre(Pair("check", "safe")));
- // FIXME: Add tests that call `emplace` in conditional branches.
+ // FIXME: Add tests that call `emplace` in conditional branches:
+ // ExpectLatticeChecksFor(
+ // R"(
+ // #include "unchecked_optional_access_test.h"
+ //
+ // void target($ns::$optional<int> opt, bool b) {
+ // if (b) {
+ // opt.emplace(0);
+ // }
+ // if (b) {
+ // opt.value();
+ // /*[[check-1]]*/
+ // } else {
+ // opt.value();
+ // /*[[check-2]]*/
+ // }
+ // }
+ // )",
+ // UnorderedElementsAre(Pair("check-1", "safe"),
+ // Pair("check-2", "unsafe: input.cc:12:9")));
}
TEST_P(UncheckedOptionalAccessTest, Reset) {
@@ -1892,7 +1911,27 @@ TEST_P(UncheckedOptionalAccessTest, Reset) {
)",
UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
- // FIXME: Add tests that call `reset` in conditional branches.
+ // FIXME: Add tests that call `reset` in conditional branches:
+ // ExpectLatticeChecksFor(
+ // R"(
+ // #include "unchecked_optional_access_test.h"
+ //
+ // void target(bool b) {
+ // $ns::$optional<int> opt = $ns::make_optional(0);
+ // if (b) {
+ // opt.reset();
+ // }
+ // if (b) {
+ // opt.value();
+ // /*[[check-1]]*/
+ // } else {
+ // opt.value();
+ // /*[[check-2]]*/
+ // }
+ // }
+ // )",
+ // UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:10:9"),
+ // Pair("check-2", "safe")));
}
TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
@@ -2347,6 +2386,284 @@ TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) {
UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
}
+TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
+ ExpectLatticeChecksFor(R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b, $ns::$optional<int> opt) {
+ if (b || opt.has_value()) {
+ if (!b) {
+ opt.value();
+ /*[[check-1]]*/
+ }
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-1", "safe")));
+
+ ExpectLatticeChecksFor(R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b, $ns::$optional<int> opt) {
+ if (b && !opt.has_value()) return;
+ if (b) {
+ opt.value();
+ /*[[check-2]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-2", "safe")));
+
+ ExpectLatticeChecksFor(
+ R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b, $ns::$optional<int> opt) {
+ if (opt.has_value()) b = true;
+ if (b) {
+ opt.value();
+ /*[[check-3]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+
+ ExpectLatticeChecksFor(R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b, $ns::$optional<int> opt) {
+ if (b) return;
+ if (opt.has_value()) b = true;
+ if (b) {
+ opt.value();
+ /*[[check-4]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-4", "safe")));
+
+ ExpectLatticeChecksFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b, $ns::$optional<int> opt) {
+ if (opt.has_value() == b) {
+ if (b) {
+ opt.value();
+ /*[[check-5]]*/
+ }
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check-5", "safe")));
+
+ ExpectLatticeChecksFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b, $ns::$optional<int> opt) {
+ if (opt.has_value() != b) {
+ if (!b) {
+ opt.value();
+ /*[[check-6]]*/
+ }
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check-6", "safe")));
+
+ ExpectLatticeChecksFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b) {
+ $ns::$optional<int> opt1 = $ns::nullopt;
+ $ns::$optional<int> opt2;
+ if (b) {
+ opt2 = $ns::nullopt;
+ } else {
+ opt2 = $ns::nullopt;
+ }
+ if (opt2.has_value()) {
+ opt1.value();
+ /*[[check]]*/
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check", "safe")));
+
+ // FIXME: Add support for operator==.
+ // ExpectLatticeChecksFor(R"(
+ // #include "unchecked_optional_access_test.h"
+ //
+ // void target($ns::$optional<int> opt1, $ns::$optional<int> opt2) {
+ // if (opt1 == opt2) {
+ // if (opt1.has_value()) {
+ // opt2.value();
+ // /*[[check-7]]*/
+ // }
+ // }
+ // }
+ // )",
+ // UnorderedElementsAre(Pair("check-7", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
+ ExpectLatticeChecksFor(
+ R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b) {
+ $ns::$optional<int> opt;
+ if (b) {
+ opt = Make<$ns::$optional<int>>();
+ } else {
+ opt = Make<$ns::$optional<int>>();
+ }
+ if (opt.has_value()) {
+ opt.value();
+ /*[[check-1]]*/
+ } else {
+ opt.value();
+ /*[[check-2]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-1", "safe"),
+ Pair("check-2", "unsafe: input.cc:15:9")));
+
+ ExpectLatticeChecksFor(R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b) {
+ $ns::$optional<int> opt;
+ if (b) {
+ opt = Make<$ns::$optional<int>>();
+ if (!opt.has_value()) return;
+ } else {
+ opt = Make<$ns::$optional<int>>();
+ if (!opt.has_value()) return;
+ }
+ opt.value();
+ /*[[check-3]]*/
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-3", "safe")));
+
+ ExpectLatticeChecksFor(
+ R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b) {
+ $ns::$optional<int> opt;
+ if (b) {
+ opt = Make<$ns::$optional<int>>();
+ if (!opt.has_value()) return;
+ } else {
+ opt = Make<$ns::$optional<int>>();
+ }
+ opt.value();
+ /*[[check-4]]*/
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:12:7")));
+
+ ExpectLatticeChecksFor(
+ R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b) {
+ $ns::$optional<int> opt;
+ if (b) {
+ opt = 1;
+ } else {
+ opt = 2;
+ }
+ opt.value();
+ /*[[check-5]]*/
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-5", "safe")));
+
+ ExpectLatticeChecksFor(
+ R"code(
+ #include "unchecked_optional_access_test.h"
+
+ void target(bool b) {
+ $ns::$optional<int> opt;
+ if (b) {
+ opt = 1;
+ } else {
+ opt = Make<$ns::$optional<int>>();
+ }
+ opt.value();
+ /*[[check-6]]*/
+ }
+ )code",
+ UnorderedElementsAre(Pair("check-6", "unsafe: input.cc:11:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, ReassignValueInLoop) {
+ ExpectLatticeChecksFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target() {
+ $ns::$optional<int> opt = 3;
+ while (Make<bool>()) {
+ opt.value();
+ /*[[check-1]]*/
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check-1", "safe")));
+
+ ExpectLatticeChecksFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target() {
+ $ns::$optional<int> opt = 3;
+ while (Make<bool>()) {
+ opt.value();
+ /*[[check-2]]*/
+
+ opt = Make<$ns::$optional<int>>();
+ if (!opt.has_value()) return;
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check-2", "safe")));
+
+ ExpectLatticeChecksFor(
+ R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target() {
+ $ns::$optional<int> opt = 3;
+ while (Make<bool>()) {
+ opt.value();
+ /*[[check-3]]*/
+
+ opt = Make<$ns::$optional<int>>();
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+
+ ExpectLatticeChecksFor(
+ R"(
+ #include "unchecked_optional_access_test.h"
+
+ void target() {
+ $ns::$optional<int> opt = 3;
+ while (Make<bool>()) {
+ opt.value();
+ /*[[check-4]]*/
+
+ opt = Make<$ns::$optional<int>>();
+ if (!opt.has_value()) continue;
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:7:9")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
More information about the cfe-commits
mailing list