sgatev updated this revision to Diff 436710.
sgatev marked 4 inline comments as done.
sgatev added a comment.
Address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125931/new/
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
@@ -1861,7 +1861,26 @@
)",
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 @@
)",
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,265 @@
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")));
+
+ // 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)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -169,11 +169,17 @@
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,17 @@
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());
}
}
}
+bool isEngagedOptionalValue(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 +663,32 @@
TransferMatchSwitch(*S, getASTContext(), State);
}
+bool UncheckedOptionalAccessModel::compareEquivalent(QualType Type,
+ const Value &Val1,
+ const Environment &Env1,
+ const Value &Val2,
+ const Environment &Env2) {
+ return isEngagedOptionalValue(Val1, Env1) ==
+ isEngagedOptionalValue(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 (isEngagedOptionalValue(Val1, Env1) &&
+ isEngagedOptionalValue(Val2, Env2)) {
+ MergedEnv.addToFlowCondition(HasValueVal);
+ }
+ setHasValue(MergedVal, HasValueVal);
+ 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,14 @@
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;
};
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
