sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
sgatev requested review of this revision.
Herald added a project: clang.
Add support for correlated branches to the std::optional dataflow model.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125931
Files:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1852,7 +1852,25 @@
)",
UnorderedElementsAre(Pair("check", "safe")));
- // 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) {
@@ -1883,7 +1901,26 @@
)",
UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
- // 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) {
@@ -2150,6 +2187,106 @@
UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
}
+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]]*/
+ }
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check", "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]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check", "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]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check", "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]]*/
+ }
+ }
+ )code",
+ UnorderedElementsAre(Pair("check", "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]]*/
+ }
+ }
+ }
+ )",
+ UnorderedElementsAre(Pair("check", "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]]*/
+ }
+ }
+ }
+ )",
+ 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]]*/
+ // }
+ // }
+ // }
+ // )",
+ // UnorderedElementsAre(Pair("check", "safe")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -158,18 +158,24 @@
ComparesToSame(integerLiteral(equals(0)))));
}
+/// Sets `HasValueVal` as the symbolic value that represents the "has_value"
+/// property of the optional value `OptionalVal`.
+void setHasValue(StructValue &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));
}
/// Returns the symbolic value that represents the "has_value" property of the
/// optional value `Val`. Returns null if `Val` is null.
-BoolValue *getHasValue(Value *Val) {
- if (auto *OptionalVal = cast_or_null<StructValue>(Val)) {
+BoolValue *getHasValue(const Value *Val) {
+ if (const auto *OptionalVal = cast_or_null<StructValue>(Val)) {
return cast<BoolValue>(OptionalVal->getProperty("has_value"));
}
return nullptr;
@@ -213,7 +219,7 @@
if (auto *OptionalVal = cast_or_null<StructValue>(
State.Env.getValue(*OptionalExpr, SkipPast::Reference))) {
if (OptionalVal->getProperty("has_value") == nullptr) {
- OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue());
+ setHasValue(*OptionalVal, State.Env.makeAtomicBoolValue());
}
}
}
@@ -570,5 +576,28 @@
TransferMatchSwitch(*S, getASTContext(), State);
}
+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 *HasValueVal1 = getHasValue(&Val1);
+ assert(HasValueVal1 != nullptr);
+
+ auto *HasValueVal2 = getHasValue(&Val2);
+ assert(HasValueVal2 != nullptr);
+
+ setHasValue(
+ *cast<StructValue>(&MergedVal),
+ MergedEnv.makeOr(
+ MergedEnv.makeAnd(Env1.getFlowConditionToken(), *HasValueVal1),
+ MergedEnv.makeAnd(Env2.getFlowConditionToken(), *HasValueVal2)));
+ return true;
+}
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -59,6 +59,10 @@
void transfer(const Stmt *Stmt, SourceLocationsLattice &State,
Environment &Env);
+ 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;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
