samestep updated this revision to Diff 441112.
samestep added a comment.
- Tidy AnalysisData struct and PostVisitStmt param
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127898/new/
https://reviews.llvm.org/D127898
Files:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
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
@@ -11,9 +11,12 @@
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Error.h"
#include "gmock/gmock.h"
@@ -26,8 +29,7 @@
using namespace dataflow;
using namespace test;
-using ::testing::Pair;
-using ::testing::UnorderedElementsAre;
+using ::testing::ContainerEq;
// FIXME: Move header definitions in separate file(s).
static constexpr char CSDtdDefHeader[] = R"(
@@ -1180,13 +1182,6 @@
} // namespace base
)";
-/// Converts `L` to string.
-static std::string ConvertToString(const SourceLocationsLattice &L,
- const ASTContext &Ctx) {
- return L.getSourceLocations().empty() ? "safe"
- : "unsafe: " + DebugString(L, Ctx);
-}
-
/// Replaces all occurrences of `Pattern` in `S` with `Replacement`.
static void ReplaceAllOccurrences(std::string &S, const std::string &Pattern,
const std::string &Replacement) {
@@ -1207,18 +1202,14 @@
class UncheckedOptionalAccessTest
: public ::testing::TestWithParam<OptionalTypeIdentifier> {
protected:
- template <typename LatticeChecksMatcher>
- void ExpectLatticeChecksFor(std::string SourceCode,
- LatticeChecksMatcher MatchesLatticeChecks) {
- ExpectLatticeChecksFor(SourceCode, ast_matchers::hasName("target"),
- MatchesLatticeChecks);
+ void ExpectDiagnosticsFor(std::string SourceCode) {
+ ExpectDiagnosticsFor(SourceCode, ast_matchers::hasName("target"));
}
private:
- template <typename FuncDeclMatcher, typename LatticeChecksMatcher>
- void ExpectLatticeChecksFor(std::string SourceCode,
- FuncDeclMatcher FuncMatcher,
- LatticeChecksMatcher MatchesLatticeChecks) {
+ template <typename FuncDeclMatcher>
+ void ExpectDiagnosticsFor(std::string SourceCode,
+ FuncDeclMatcher FuncMatcher) {
ReplaceAllOccurrences(SourceCode, "$ns", GetParam().NamespaceName);
ReplaceAllOccurrences(SourceCode, "$optional", GetParam().TypeName);
@@ -1245,27 +1236,44 @@
)");
const tooling::FileContentMappings FileContents(Headers.begin(),
Headers.end());
+ UncheckedOptionalAccessModelOptions Options{
+ /*IgnoreSmartPointerDereference=*/true};
+ std::vector<SourceLocation> Diagnostics;
llvm::Error Error = checkDataflow<UncheckedOptionalAccessModel>(
SourceCode, FuncMatcher,
- [](ASTContext &Ctx, Environment &) {
- return UncheckedOptionalAccessModel(
- Ctx, UncheckedOptionalAccessModelOptions{
- /*IgnoreSmartPointerDereference=*/true});
+ [Options](ASTContext &Ctx, Environment &) {
+ return UncheckedOptionalAccessModel(Ctx, Options);
+ },
+ [&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ ASTContext &Ctx, const Stmt *Stmt,
+ const TypeErasedDataflowAnalysisState &State) mutable {
+ auto StmtDiagnostics = Diagnoser.diagnose(Ctx, Stmt, State.Env);
+ llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
},
- [&MatchesLatticeChecks](
- llvm::ArrayRef<std::pair<
- std::string, DataflowAnalysisState<SourceLocationsLattice>>>
- CheckToLatticeMap,
- ASTContext &Ctx) {
- // FIXME: Consider using a matcher instead of translating
- // `CheckToLatticeMap` to `CheckToStringifiedLatticeMap`.
- std::vector<std::pair<std::string, std::string>>
- CheckToStringifiedLatticeMap;
- for (const auto &E : CheckToLatticeMap) {
- CheckToStringifiedLatticeMap.emplace_back(
- E.first, ConvertToString(E.second.Lattice, Ctx));
+ [&Diagnostics](AnalysisData AnalysisData) {
+ auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
+
+ llvm::DenseSet<unsigned> AnnotationLines;
+ for (const auto &Pair : AnalysisData.Annotations) {
+ auto *Stmt = Pair.getFirst();
+ AnnotationLines.insert(
+ SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
+ // We add both the begin and end locations, so that if the
+ // statement spans multiple lines then the test will fail.
+ //
+ // FIXME: Going forward, we should change this to instead just
+ // get the single line number from the annotation itself, rather
+ // than looking at the statement it's attached to.
+ AnnotationLines.insert(
+ SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+ }
+
+ llvm::DenseSet<unsigned> DiagnosticLines;
+ for (SourceLocation &Loc : Diagnostics) {
+ DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
}
- EXPECT_THAT(CheckToStringifiedLatticeMap, MatchesLatticeChecks);
+
+ EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
},
{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
if (Error)
@@ -1283,65 +1291,55 @@
});
TEST_P(UncheckedOptionalAccessTest, EmptyFunctionBody) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
void target() {
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingValueNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- std::move(opt).value();
- /*[[check]]*/
+ std::move(opt).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorStarNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- *opt;
- /*[[check]]*/
+ *opt; // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- *std::move(opt);
- /*[[check]]*/
+ *std::move(opt); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1350,13 +1348,11 @@
};
void target($ns::$optional<Foo> opt) {
- opt->foo();
- /*[[check]]*/
+ opt->foo(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1365,107 +1361,91 @@
};
void target($ns::$optional<Foo> opt) {
- std::move(opt)->foo();
- /*[[check]]*/
+ std::move(opt)->foo(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, HasValueCheck) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
if (opt.has_value()) {
opt.value();
- /*[[check]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OperatorBoolCheck) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
if (opt) {
opt.value();
- /*[[check]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapFunctionCallResultNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
- Make<$ns::$optional<int>>().value();
+ Make<$ns::$optional<int>>().value(); // [[unsafe]]
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- std::move(opt).value();
- /*[[check]]*/
+ std::move(opt).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, DefaultConstructor) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt;
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, NulloptConstructor) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt($ns::nullopt);
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt($ns::in_place, 3);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1473,12 +1453,10 @@
void target() {
$ns::$optional<Foo> opt($ns::in_place);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1488,12 +1466,10 @@
void target() {
$ns::$optional<Foo> opt($ns::in_place, 3, false);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1503,46 +1479,38 @@
void target() {
$ns::$optional<Foo> opt($ns::in_place, {3});
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ValueConstructor) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt(21);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = $ns::$optional<int>(21);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
- ExpectLatticeChecksFor(R"(
+ )");
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<$ns::$optional<int>> opt(Make<$ns::$optional<int>>());
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct MyString {
@@ -1552,12 +1520,10 @@
void target() {
$ns::$optional<MyString> opt("foo");
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1569,12 +1535,10 @@
void target() {
$ns::$optional<Bar> opt(Make<Foo>());
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1584,14 +1548,12 @@
void target() {
$ns::$optional<Foo> opt(3);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1603,13 +1565,11 @@
void target() {
$ns::$optional<Bar> opt(Make<$ns::$optional<Foo>>());
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1621,13 +1581,11 @@
void target() {
$ns::$optional<Bar> opt(Make<$ns::$optional<Foo>>());
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1640,13 +1598,11 @@
void target() {
$ns::$optional<Foo> opt1 = $ns::nullopt;
$ns::$optional<Bar> opt2(opt1);
- opt2.value();
- /*[[check]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:13:7")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1659,12 +1615,10 @@
$ns::$optional<Foo> opt1(Make<Foo>());
$ns::$optional<Bar> opt2(opt1);
opt2.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1677,25 +1631,21 @@
$ns::$optional<Foo> opt1(Make<Foo>());
$ns::$optional<Bar> opt2(opt1);
opt2.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, MakeOptional) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = $ns::make_optional(0);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1705,12 +1655,10 @@
void target() {
$ns::$optional<Foo> opt = $ns::make_optional<Foo>(21, 22);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1721,104 +1669,83 @@
char a = 'a';
$ns::$optional<Foo> opt = $ns::make_optional<Foo>({a});
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ValueOr) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt;
opt.value_or(0);
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
// Pointers.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int*> opt) {
if (opt.value_or(nullptr) != nullptr) {
opt.value();
- /*[[check-ptrs-1]]*/
} else {
- opt.value();
- /*[[check-ptrs-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-ptrs-1", "safe"),
- Pair("check-ptrs-2", "unsafe: input.cc:9:9")));
+ )code");
// Integers.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
if (opt.value_or(0) != 0) {
opt.value();
- /*[[check-ints-1]]*/
} else {
- opt.value();
- /*[[check-ints-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-ints-1", "safe"),
- Pair("check-ints-2", "unsafe: input.cc:9:9")));
+ )code");
// Strings.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<std::string> opt) {
if (!opt.value_or("").empty()) {
opt.value();
- /*[[check-strings-1]]*/
} else {
- opt.value();
- /*[[check-strings-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-strings-1", "safe"),
- Pair("check-strings-2", "unsafe: input.cc:9:9")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<std::string> opt) {
if (opt.value_or("") != "") {
opt.value();
- /*[[check-strings-neq-1]]*/
} else {
- opt.value();
- /*[[check-strings-neq-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(
- Pair("check-strings-neq-1", "safe"),
- Pair("check-strings-neq-2", "unsafe: input.cc:9:9")));
+ )code");
// Pointer-to-optional.
//
// FIXME: make `opt` a parameter directly, once we ensure that all `optional`
// values have a `has_value` property.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -1826,43 +1753,35 @@
$ns::$optional<int> *opt = &p;
if (opt->value_or(0) != 0) {
opt->value();
- /*[[check-pto-1]]*/
} else {
- opt->value();
- /*[[check-pto-2]]*/
+ opt->value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-pto-1", "safe"),
- Pair("check-pto-2", "unsafe: input.cc:10:9")));
+ )code");
}
TEST_P(UncheckedOptionalAccessTest, Emplace) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt;
opt.emplace(0);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> *opt) {
opt->emplace(0);
opt->value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
// FIXME: Add tests that call `emplace` in conditional branches:
- // ExpectLatticeChecksFor(
+ // ExpectDiagnosticsFor(
// R"(
// #include "unchecked_optional_access_test.h"
//
@@ -1872,47 +1791,39 @@
// }
// if (b) {
// opt.value();
- // /*[[check-1]]*/
// } else {
- // opt.value();
- // /*[[check-2]]*/
+ // opt.value(); // [[unsafe]]
// }
// }
- // )",
- // UnorderedElementsAre(Pair("check-1", "safe"),
- // Pair("check-2", "unsafe: input.cc:12:9")));
+ // )");
}
TEST_P(UncheckedOptionalAccessTest, Reset) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = $ns::make_optional(0);
opt.reset();
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> &opt) {
if (opt.has_value()) {
opt.reset();
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
+ )");
// FIXME: Add tests that call `reset` in conditional branches:
- // ExpectLatticeChecksFor(
+ // ExpectDiagnosticsFor(
// R"(
// #include "unchecked_optional_access_test.h"
//
@@ -1922,20 +1833,16 @@
// opt.reset();
// }
// if (b) {
- // opt.value();
- // /*[[check-1]]*/
+ // opt.value(); // [[unsafe]]
// } else {
// opt.value();
- // /*[[check-2]]*/
// }
// }
- // )",
- // UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:10:9"),
- // Pair("check-2", "safe")));
+ // )");
}
TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1944,12 +1851,10 @@
$ns::$optional<Foo> opt;
opt = Foo();
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1958,12 +1863,10 @@
$ns::$optional<Foo> opt;
(opt = Foo()).value();
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct MyString {
@@ -1974,12 +1877,10 @@
$ns::$optional<MyString> opt;
opt = "foo";
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct MyString {
@@ -1989,14 +1890,12 @@
void target() {
$ns::$optional<MyString> opt;
(opt = "foo").value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalConversionAssignment) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2011,12 +1910,10 @@
$ns::$optional<Bar> opt2;
opt2 = opt1;
opt2.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2031,14 +1928,12 @@
$ns::$optional<Bar> opt2;
if (opt2.has_value()) {
opt2 = opt1;
- opt2.value();
- /*[[check]]*/
+ opt2.value(); // [[unsafe]]
}
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:15:9")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2053,41 +1948,35 @@
$ns::$optional<Bar> opt2;
(opt2 = opt1).value();
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, NulloptAssignment) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
opt = $ns::nullopt;
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
- (opt = $ns::nullopt).value();
- /*[[check]]*/
+ (opt = $ns::nullopt).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2098,16 +1987,12 @@
opt1.swap(opt2);
opt1.value();
- /*[[check-1]]*/
- opt2.value();
- /*[[check-2]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:13:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2118,18 +2003,14 @@
opt2.swap(opt1);
opt1.value();
- /*[[check-3]]*/
- opt2.value();
- /*[[check-4]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-3", "safe"),
- Pair("check-4", "unsafe: input.cc:13:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, StdSwap) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2140,16 +2021,12 @@
std::swap(opt1, opt2);
opt1.value();
- /*[[check-1]]*/
- opt2.value();
- /*[[check-2]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:13:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2160,20 +2037,16 @@
std::swap(opt2, opt1);
opt1.value();
- /*[[check-3]]*/
- opt2.value();
- /*[[check-4]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-3", "safe"),
- Pair("check-4", "unsafe: input.cc:13:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UniquePtrToStructWithOptionalField) {
// We suppress diagnostics for values reachable from smart pointers (other
// than `optional` itself).
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2190,16 +2063,13 @@
void target() {
smart_ptr<Foo> foo;
*foo->opt;
- /*[[check-1]]*/
*(*foo).opt;
- /*[[check-2]]*/
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2208,12 +2078,10 @@
void target() {
$ns::$optional<int> opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-1]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:9:7")));
- ExpectLatticeChecksFor(
+ )");
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2222,13 +2090,11 @@
void target() {
$ns::$optional<int> opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-2]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-2", "unsafe: input.cc:9:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2238,13 +2104,11 @@
void target() {
IntOpt opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-3]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:10:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2254,16 +2118,14 @@
void target() {
IntOpt opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-4]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
+ )");
}
// Verifies that the model sees through aliases.
TEST_P(UncheckedOptionalAccessTest, WithAlias) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2271,18 +2133,16 @@
using MyOptional = $ns::$optional<T>;
void target(MyOptional<int> opt) {
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalValueOptional) {
// Basic test that nested values are populated. We nest an optional because
// its easy to use in a test, but the type of the nested value shouldn't
// matter.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2291,14 +2151,12 @@
void target($ns::$optional<Foo> foo) {
if (foo && *foo) {
foo->value();
- /*[[access]]*/
}
}
- )",
- UnorderedElementsAre(Pair("access", "safe")));
+ )");
// Mutation is supported for nested values.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2307,18 +2165,16 @@
void target($ns::$optional<Foo> foo) {
if (foo && *foo) {
foo->reset();
- foo->value();
- /*[[reset]]*/
+ foo->value(); // [[unsafe]]
}
}
- )",
- UnorderedElementsAre(Pair("reset", "unsafe: input.cc:9:9")));
+ )");
}
// Tests that structs can be nested. We use an optional field because its easy
// to use in a test, but the type of the field shouldn't matter.
TEST_P(UncheckedOptionalAccessTest, OptionalValueStruct) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2329,18 +2185,16 @@
void target($ns::$optional<Foo> foo) {
if (foo && foo->opt) {
foo->opt.value();
- /*[[access]]*/
}
}
- )",
- UnorderedElementsAre(Pair("access", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
// FIXME: Fix when to initialize `value`. All unwrapping should be safe in
// this example, but `value` initialization is done multiple times during the
// fixpoint iterations and joining the environment won't correctly merge them.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2359,15 +2213,13 @@
}
// Now we merge the two values. UncheckedOptionalAccessModel::merge() will
// throw away the "value" property.
- foo->value();
- /*[[merge]]*/
+ foo->value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("merge", "unsafe: input.cc:19:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2379,56 +2231,48 @@
void target() {
smart_ptr<$ns::$optional<float>> x;
*x = $ns::nullopt;
- (*x).value();
- /*[[check]]*/
+ (*x).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(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")));
+ )code");
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(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")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
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]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+ )code");
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(R"code(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
@@ -2436,41 +2280,35 @@
if (opt.has_value()) b = true;
if (b) {
opt.value();
- /*[[check-4]]*/
}
}
- )code",
- UnorderedElementsAre(Pair("check-4", "safe")));
+ )code");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(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"(
+ ExpectDiagnosticsFor(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"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target(bool b) {
@@ -2483,30 +2321,26 @@
}
if (opt2.has_value()) {
opt1.value();
- /*[[check]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
// FIXME: Add support for operator==.
- // ExpectLatticeChecksFor(R"(
+ // ExpectDiagnosticsFor(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(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2519,17 +2353,13 @@
}
if (opt.has_value()) {
opt.value();
- /*[[check-1]]*/
} else {
- opt.value();
- /*[[check-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:15:9")));
+ )code");
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(R"code(
#include "unchecked_optional_access_test.h"
void target(bool b) {
@@ -2542,12 +2372,10 @@
if (!opt.has_value()) return;
}
opt.value();
- /*[[check-3]]*/
}
- )code",
- UnorderedElementsAre(Pair("check-3", "safe")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2559,13 +2387,11 @@
} else {
opt = Make<$ns::$optional<int>>();
}
- opt.value();
- /*[[check-4]]*/
+ opt.value(); // [[unsafe]]
}
- )code",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:12:7")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2577,12 +2403,10 @@
opt = 2;
}
opt.value();
- /*[[check-5]]*/
}
- )code",
- UnorderedElementsAre(Pair("check-5", "safe")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2593,75 +2417,65 @@
} else {
opt = Make<$ns::$optional<int>>();
}
- opt.value();
- /*[[check-6]]*/
+ opt.value(); // [[unsafe]]
}
- )code",
- UnorderedElementsAre(Pair("check-6", "unsafe: input.cc:11:7")));
+ )code");
}
TEST_P(UncheckedOptionalAccessTest, ReassignValueInLoop) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(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"(
+ ExpectDiagnosticsFor(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(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
while (Make<bool>()) {
- opt.value();
- /*[[check-3]]*/
+ opt.value(); // [[unsafe]]
opt = Make<$ns::$optional<int>>();
}
}
- )",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
while (Make<bool>()) {
- opt.value();
- /*[[check-4]]*/
+ opt.value(); // [[unsafe]]
opt = Make<$ns::$optional<int>>();
if (!opt.has_value()) continue;
}
}
- )",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:7:9")));
+ )");
}
// FIXME: Add support for:
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -62,22 +62,25 @@
buildStatementToAnnotationMapping(const FunctionDecl *Func,
llvm::Annotations AnnotatedCode);
-// Runs dataflow on the body of the function that matches `func_matcher` in code
-// snippet `code`. Requires: `Analysis` contains a type `Lattice`.
+struct AnalysisData {
+ ASTContext &ASTCtx;
+ const ControlFlowContext &CFCtx;
+ const Environment &Env;
+ TypeErasedDataflowAnalysis &Analysis;
+ llvm::DenseMap<const clang::Stmt *, std::string> &Annotations;
+ std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>> &BlockStates;
+};
+
template <typename AnalysisT>
llvm::Error checkDataflow(
llvm::StringRef Code,
- ast_matchers::internal::Matcher<FunctionDecl> FuncMatcher,
+ ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher,
std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis,
- std::function<void(
- llvm::ArrayRef<std::pair<
- std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
- ASTContext &)>
- Expectations,
- ArrayRef<std::string> Args,
+ std::function<void(ASTContext &, const Stmt *,
+ const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmt,
+ std::function<void(AnalysisData)> VerifyResults, ArrayRef<std::string> Args,
const tooling::FileContentMappings &VirtualMappedFiles = {}) {
- using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
-
llvm::Annotations AnnotatedCode(Code);
auto Unit = tooling::buildASTFromCodeWithArgs(
AnnotatedCode.code(), Args, "input.cc", "clang-dataflow-test",
@@ -93,10 +96,10 @@
const FunctionDecl *F = ast_matchers::selectFirst<FunctionDecl>(
"target",
- ast_matchers::match(
- ast_matchers::functionDecl(ast_matchers::isDefinition(), FuncMatcher)
- .bind("target"),
- Context));
+ ast_matchers::match(ast_matchers::functionDecl(
+ ast_matchers::isDefinition(), TargetFuncMatcher)
+ .bind("target"),
+ Context));
if (F == nullptr)
return llvm::make_error<llvm::StringError>(
llvm::errc::invalid_argument, "Could not find target function.");
@@ -109,6 +112,16 @@
Environment Env(DACtx, *F);
auto Analysis = MakeAnalysis(Context, Env);
+ std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmtClosure = nullptr;
+ if (PostVisitStmt != nullptr) {
+ PostVisitStmtClosure = [&PostVisitStmt, &Context](
+ const Stmt *Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ PostVisitStmt(Context, Stmt, State);
+ };
+ }
+
llvm::Expected<llvm::DenseMap<const clang::Stmt *, std::string>>
StmtToAnnotations = buildStatementToAnnotationMapping(F, AnnotatedCode);
if (!StmtToAnnotations)
@@ -116,40 +129,72 @@
auto &Annotations = *StmtToAnnotations;
llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
- MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env);
+ MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
+ PostVisitStmtClosure);
if (!MaybeBlockStates)
return MaybeBlockStates.takeError();
auto &BlockStates = *MaybeBlockStates;
- if (BlockStates.empty()) {
- Expectations({}, Context);
- return llvm::Error::success();
- }
-
- // Compute a map from statement annotations to the state computed for
- // the program point immediately after the annotated statement.
- std::vector<std::pair<std::string, StateT>> Results;
- for (const CFGBlock *Block : CFCtx->getCFG()) {
- // Skip blocks that were not evaluated.
- if (!BlockStates[Block->getBlockID()])
- continue;
-
- transferBlock(
- *CFCtx, BlockStates, *Block, Env, Analysis,
- [&Results, &Annotations](const clang::CFGStmt &Stmt,
- const TypeErasedDataflowAnalysisState &State) {
- auto It = Annotations.find(Stmt.getStmt());
- if (It == Annotations.end())
- return;
- auto *Lattice =
- llvm::any_cast<typename AnalysisT::Lattice>(&State.Lattice.Value);
- Results.emplace_back(It->second, StateT{*Lattice, State.Env});
- });
- }
- Expectations(Results, Context);
+ AnalysisData AnalysisData{Context, *CFCtx, Env,
+ Analysis, Annotations, BlockStates};
+ VerifyResults(AnalysisData);
return llvm::Error::success();
}
+// Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
+// code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
+template <typename AnalysisT>
+llvm::Error checkDataflow(
+ llvm::StringRef Code,
+ ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher,
+ std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis,
+ std::function<void(
+ llvm::ArrayRef<std::pair<
+ std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
+ ASTContext &)>
+ VerifyResults,
+ ArrayRef<std::string> Args,
+ const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+ using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
+
+ return checkDataflow(
+ Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
+ /*PostVisitStmt=*/nullptr,
+ [&VerifyResults](AnalysisData AnalysisData) {
+ if (AnalysisData.BlockStates.empty()) {
+ VerifyResults({}, AnalysisData.ASTCtx);
+ return;
+ }
+
+ auto &Annotations = AnalysisData.Annotations;
+
+ // Compute a map from statement annotations to the state computed for
+ // the program point immediately after the annotated statement.
+ std::vector<std::pair<std::string, StateT>> Results;
+ for (const CFGBlock *Block : AnalysisData.CFCtx.getCFG()) {
+ // Skip blocks that were not evaluated.
+ if (!AnalysisData.BlockStates[Block->getBlockID()])
+ continue;
+
+ transferBlock(
+ AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
+ AnalysisData.Env, AnalysisData.Analysis,
+ [&Results,
+ &Annotations](const clang::CFGStmt &Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ auto It = Annotations.find(Stmt.getStmt());
+ if (It == Annotations.end())
+ return;
+ auto *Lattice = llvm::any_cast<typename AnalysisT::Lattice>(
+ &State.Lattice.Value);
+ Results.emplace_back(It->second, StateT{*Lattice, State.Env});
+ });
+ }
+ VerifyResults(Results, AnalysisData.ASTCtx);
+ },
+ Args, VirtualMappedFiles);
+}
+
// Runs dataflow on the body of the function named `target_fun` in code snippet
// `code`.
template <typename AnalysisT>
@@ -160,11 +205,11 @@
llvm::ArrayRef<std::pair<
std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
ASTContext &)>
- Expectations,
+ VerifyResults,
ArrayRef<std::string> Args,
const tooling::FileContentMappings &VirtualMappedFiles = {}) {
return checkDataflow(Code, ast_matchers::hasName(TargetFun),
- std::move(MakeAnalysis), std::move(Expectations), Args,
+ std::move(MakeAnalysis), std::move(VerifyResults), Args,
VirtualMappedFiles);
}
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -326,9 +326,11 @@
}
llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
-runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
- TypeErasedDataflowAnalysis &Analysis,
- const Environment &InitEnv) {
+runTypeErasedDataflowAnalysis(
+ const ControlFlowContext &CFCtx, TypeErasedDataflowAnalysis &Analysis,
+ const Environment &InitEnv,
+ std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmt) {
PostOrderCFGView POV(&CFCtx.getCFG());
ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
@@ -387,6 +389,20 @@
// FIXME: Consider evaluating unreachable basic blocks (those that have a
// state set to `llvm::None` at this point) to also analyze dead code.
+ if (PostVisitStmt) {
+ for (const CFGBlock *Block : CFCtx.getCFG()) {
+ // Skip blocks that were not evaluated.
+ if (!BlockStates[Block->getBlockID()])
+ continue;
+ transferBlock(
+ CFCtx, BlockStates, *Block, InitEnv, Analysis,
+ [&PostVisitStmt](const clang::CFGStmt &Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ PostVisitStmt(Stmt.getStmt(), State);
+ });
+ }
+ }
+
return BlockStates;
}
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,11 +22,13 @@
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
#include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include <cassert>
#include <memory>
#include <utility>
+#include <vector>
namespace clang {
namespace dataflow {
@@ -551,6 +553,17 @@
return llvm::None;
}
+StatementMatcher
+valueCall(llvm::Optional<StatementMatcher> &IgnorableOptional) {
+ return isOptionalMemberCallWithName("value", IgnorableOptional);
+}
+
+StatementMatcher
+valueOperatorCall(llvm::Optional<StatementMatcher> &IgnorableOptional) {
+ return expr(anyOf(isOptionalOperatorCallWithName("*", IgnorableOptional),
+ isOptionalOperatorCallWithName("->", IgnorableOptional)));
+}
+
auto buildTransferMatchSwitch(
const UncheckedOptionalAccessModelOptions &Options) {
// FIXME: Evaluate the efficiency of matchers. If using matchers results in a
@@ -592,20 +605,18 @@
// optional::value
.CaseOf<CXXMemberCallExpr>(
- isOptionalMemberCallWithName("value", IgnorableOptional),
+ valueCall(IgnorableOptional),
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
LatticeTransferState &State) {
transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
})
// optional::operator*, optional::operator->
- .CaseOf<CallExpr>(
- expr(anyOf(isOptionalOperatorCallWithName("*", IgnorableOptional),
- isOptionalOperatorCallWithName("->", IgnorableOptional))),
- [](const CallExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
- transferUnwrapCall(E, E->getArg(0), State);
- })
+ .CaseOf<CallExpr>(valueOperatorCall(IgnorableOptional),
+ [](const CallExpr *E, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ transferUnwrapCall(E, E->getArg(0), State);
+ })
// optional::has_value
.CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("has_value"),
@@ -653,6 +664,49 @@
.Build();
}
+std::vector<SourceLocation> diagnoseUnwrapCall(const Expr *UnwrapExpr,
+ const Expr *ObjectExpr,
+ const Environment &Env) {
+ if (auto *OptionalVal =
+ Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) {
+ auto *Prop = OptionalVal->getProperty("has_value");
+ if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) {
+ if (Env.flowConditionImplies(*HasValueVal))
+ return {};
+ }
+ }
+
+ // Record that this unwrap is *not* provably safe.
+ // FIXME: include either the name of the optional (if applicable) or a source
+ // range of the access for easier interpretation of the result.
+ return {ObjectExpr->getBeginLoc()};
+}
+
+auto buildDiagnoseMatchSwitch(
+ const UncheckedOptionalAccessModelOptions &Options) {
+ // FIXME: Evaluate the efficiency of matchers. If using matchers results in a
+ // lot of duplicated work (e.g. string comparisons), consider providing APIs
+ // that avoid it through memoization.
+ auto IgnorableOptional = ignorableOptional(Options);
+ return MatchSwitchBuilder<const Environment, std::vector<SourceLocation>>()
+ // optional::value
+ .CaseOf<CXXMemberCallExpr>(
+ valueCall(IgnorableOptional),
+ [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
+ const Environment &Env) {
+ return diagnoseUnwrapCall(E, E->getImplicitObjectArgument(), Env);
+ })
+
+ // optional::operator*, optional::operator->
+ .CaseOf<CallExpr>(
+ valueOperatorCall(IgnorableOptional),
+ [](const CallExpr *E, const MatchFinder::MatchResult &,
+ const Environment &Env) {
+ return diagnoseUnwrapCall(E, E->getArg(0), Env);
+ })
+ .Build();
+}
+
} // namespace
ast_matchers::DeclarationMatcher
@@ -699,5 +753,14 @@
return true;
}
+UncheckedOptionalAccessDiagnoser::UncheckedOptionalAccessDiagnoser(
+ UncheckedOptionalAccessModelOptions Options)
+ : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {}
+
+std::vector<SourceLocation> UncheckedOptionalAccessDiagnoser::diagnose(
+ ASTContext &Context, const Stmt *Stmt, const Environment &Env) {
+ return DiagnoseMatchSwitch(*Stmt, Context, Env);
+}
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -115,13 +115,17 @@
HandleTransferredStmt = nullptr);
/// Performs dataflow analysis and returns a mapping from basic block IDs to
-/// dataflow analysis states that model the respective basic blocks. Indices
-/// of the returned vector correspond to basic block IDs. Returns an error if
-/// the dataflow analysis cannot be performed successfully.
+/// dataflow analysis states that model the respective basic blocks. Indices of
+/// the returned vector correspond to basic block IDs. Returns an error if the
+/// dataflow analysis cannot be performed successfully. Otherwise, calls
+/// `PostVisitStmt` on each statement with the final analysis results at that
+/// program point.
llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
-runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
- TypeErasedDataflowAnalysis &Analysis,
- const Environment &InitEnv);
+runTypeErasedDataflowAnalysis(
+ const ControlFlowContext &CFCtx, TypeErasedDataflowAnalysis &Analysis,
+ const Environment &InitEnv,
+ std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmt = nullptr);
} // 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
@@ -20,6 +20,8 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Basic/SourceLocation.h"
+#include <vector>
namespace clang {
namespace dataflow {
@@ -71,6 +73,19 @@
MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
};
+class UncheckedOptionalAccessDiagnoser {
+public:
+ UncheckedOptionalAccessDiagnoser(
+ UncheckedOptionalAccessModelOptions Options = {});
+
+ std::vector<SourceLocation> diagnose(ASTContext &Context, const Stmt *Stmt,
+ const Environment &Env);
+
+private:
+ MatchSwitch<const Environment, std::vector<SourceLocation>>
+ DiagnoseMatchSwitch;
+};
+
} // namespace dataflow
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
