samestep updated this revision to Diff 439372.
samestep added a comment.
- Merge branch 'main' into diagnose-api
- Incorporate Gábor's suggestions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127898/new/
https://reviews.llvm.org/D127898
Files:
clang/docs/tools/clang-formatted-files.txt
clang/include/clang/Analysis/FlowSensitive/Diagnosis.h
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/SourceLocationsLattice.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
@@ -12,8 +12,10 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Error.h"
#include "gmock/gmock.h"
@@ -26,9 +28,6 @@
using namespace dataflow;
using namespace test;
-using ::testing::Pair;
-using ::testing::UnorderedElementsAre;
-
// FIXME: Move header definitions in separate file(s).
static constexpr char CSDtdDefHeader[] = R"(
#ifndef CSTDDEF_H
@@ -1181,10 +1180,9 @@
)";
/// Converts `L` to string.
-static std::string ConvertToString(const SourceLocationsLattice &L,
+static std::string ConvertToString(const llvm::DenseSet<SourceLocation> &L,
const ASTContext &Ctx) {
- return L.getSourceLocations().empty() ? "safe"
- : "unsafe: " + DebugString(L, Ctx);
+ return L.empty() ? "safe" : "unsafe: " + DebugString(L, Ctx);
}
/// Replaces all occurrences of `Pattern` in `S` with `Replacement`.
@@ -1245,29 +1243,30 @@
)");
const tooling::FileContentMappings FileContents(Headers.begin(),
Headers.end());
- llvm::Error Error = checkDataflow<UncheckedOptionalAccessModel>(
- SourceCode, FuncMatcher,
- [](ASTContext &Ctx, Environment &) {
- return UncheckedOptionalAccessModel(
- Ctx, UncheckedOptionalAccessModelOptions{
- /*IgnoreSmartPointerDereference=*/true});
- },
- [&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));
- }
- EXPECT_THAT(CheckToStringifiedLatticeMap, MatchesLatticeChecks);
- },
- {"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+ UncheckedOptionalAccessModelOptions Options{
+ /*IgnoreSmartPointerDereference=*/true};
+ llvm::Error Error =
+ checkDataflowDiagnosis<UncheckedOptionalAccessModel, SourceLocation>(
+ SourceCode, FuncMatcher,
+ [Options](ASTContext &Ctx, Environment &) {
+ return UncheckedOptionalAccessModel(Ctx, Options);
+ },
+ [Options](ASTContext &Ctx) {
+ UncheckedOptionalAccessDiagnosis Diagnosis(Ctx, Options);
+ return [Diagnosis = std::move(Diagnosis)](
+ const Stmt *Stmt,
+ const UncheckedOptionalAccessModel::Lattice &,
+ const Environment &Env) mutable {
+ return Diagnosis.diagnose(Stmt, Env);
+ };
+ },
+ [&MatchesLatticeChecks](llvm::DenseSet<SourceLocation> Locs,
+ ASTContext &Ctx) {
+ std::string StringifiedDiags = ConvertToString(Locs, Ctx);
+ EXPECT_THAT(StringifiedDiags, MatchesLatticeChecks);
+ },
+ {"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"},
+ FileContents);
if (Error)
FAIL() << llvm::toString(std::move(Error));
}
@@ -1289,7 +1288,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingValueNoCheck) {
@@ -1302,7 +1301,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ "unsafe: input.cc:5:7");
ExpectLatticeChecksFor(
R"(
@@ -1313,7 +1312,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ "unsafe: input.cc:5:7");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorStarNoCheck) {
@@ -1326,7 +1325,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
+ "unsafe: input.cc:5:8");
ExpectLatticeChecksFor(
R"(
@@ -1337,7 +1336,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
+ "unsafe: input.cc:5:8");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) {
@@ -1354,7 +1353,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
+ "unsafe: input.cc:9:7");
ExpectLatticeChecksFor(
R"(
@@ -1369,7 +1368,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
+ "unsafe: input.cc:9:7");
}
TEST_P(UncheckedOptionalAccessTest, HasValueCheck) {
@@ -1383,7 +1382,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, OperatorBoolCheck) {
@@ -1397,7 +1396,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapFunctionCallResultNoCheck) {
@@ -1411,7 +1410,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ "unsafe: input.cc:5:7");
ExpectLatticeChecksFor(
R"(
@@ -1422,7 +1421,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ "unsafe: input.cc:5:7");
}
TEST_P(UncheckedOptionalAccessTest, DefaultConstructor) {
@@ -1436,7 +1435,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ "unsafe: input.cc:6:7");
}
TEST_P(UncheckedOptionalAccessTest, NulloptConstructor) {
@@ -1450,7 +1449,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ "unsafe: input.cc:6:7");
}
TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
@@ -1463,7 +1462,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1476,7 +1475,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1491,7 +1490,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1506,7 +1505,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, ValueConstructor) {
@@ -1519,7 +1518,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1530,7 +1529,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1540,7 +1539,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1555,7 +1554,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1572,7 +1571,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1587,7 +1586,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
@@ -1607,7 +1606,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ "unsafe: input.cc:12:7");
ExpectLatticeChecksFor(
R"(
@@ -1625,7 +1624,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ "unsafe: input.cc:12:7");
ExpectLatticeChecksFor(
R"(
@@ -1644,7 +1643,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:13:7")));
+ "unsafe: input.cc:13:7");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1662,7 +1661,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1680,7 +1679,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, MakeOptional) {
@@ -1693,7 +1692,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1708,7 +1707,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1724,7 +1723,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, ValueOr) {
@@ -1738,7 +1737,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
@@ -1757,8 +1756,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-ptrs-1", "safe"),
- Pair("check-ptrs-2", "unsafe: input.cc:9:9")));
+ "unsafe: input.cc:9:9");
// Integers.
ExpectLatticeChecksFor(
@@ -1775,8 +1773,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-ints-1", "safe"),
- Pair("check-ints-2", "unsafe: input.cc:9:9")));
+ "unsafe: input.cc:9:9");
// Strings.
ExpectLatticeChecksFor(
@@ -1793,8 +1790,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-strings-1", "safe"),
- Pair("check-strings-2", "unsafe: input.cc:9:9")));
+ "unsafe: input.cc:9:9");
ExpectLatticeChecksFor(
R"code(
@@ -1810,9 +1806,7 @@
}
}
)code",
- UnorderedElementsAre(
- Pair("check-strings-neq-1", "safe"),
- Pair("check-strings-neq-2", "unsafe: input.cc:9:9")));
+ "unsafe: input.cc:9:9");
// Pointer-to-optional.
//
@@ -1833,8 +1827,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-pto-1", "safe"),
- Pair("check-pto-2", "unsafe: input.cc:10:9")));
+ "unsafe: input.cc:10:9");
}
TEST_P(UncheckedOptionalAccessTest, Emplace) {
@@ -1848,7 +1841,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1859,7 +1852,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
// FIXME: Add tests that call `emplace` in conditional branches:
// ExpectLatticeChecksFor(
@@ -1879,8 +1872,7 @@
// }
// }
// )",
- // UnorderedElementsAre(Pair("check-1", "safe"),
- // Pair("check-2", "unsafe: input.cc:12:9")));
+ // "unsafe: input.cc:12:9");
}
TEST_P(UncheckedOptionalAccessTest, Reset) {
@@ -1895,7 +1887,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+ "unsafe: input.cc:7:7");
ExpectLatticeChecksFor(
R"(
@@ -1909,7 +1901,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
+ "unsafe: input.cc:7:9");
// FIXME: Add tests that call `reset` in conditional branches:
// ExpectLatticeChecksFor(
@@ -1930,8 +1922,7 @@
// }
// }
// )",
- // UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:10:9"),
- // Pair("check-2", "safe")));
+ // "safe");
}
TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
@@ -1947,7 +1938,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1961,7 +1952,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1977,7 +1968,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -1992,7 +1983,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, OptionalConversionAssignment) {
@@ -2014,7 +2005,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
ExpectLatticeChecksFor(
R"(
@@ -2036,7 +2027,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:15:9")));
+ "unsafe: input.cc:15:9");
ExpectLatticeChecksFor(
R"(
@@ -2056,7 +2047,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, NulloptAssignment) {
@@ -2071,7 +2062,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+ "unsafe: input.cc:7:7");
ExpectLatticeChecksFor(
R"(
@@ -2083,7 +2074,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ "unsafe: input.cc:6:7");
}
TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
@@ -2104,8 +2095,7 @@
/*[[check-2]]*/
}
)",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:13:7")));
+ "unsafe: input.cc:13:7");
ExpectLatticeChecksFor(
R"(
@@ -2124,8 +2114,7 @@
/*[[check-4]]*/
}
)",
- UnorderedElementsAre(Pair("check-3", "safe"),
- Pair("check-4", "unsafe: input.cc:13:7")));
+ "unsafe: input.cc:13:7");
}
TEST_P(UncheckedOptionalAccessTest, StdSwap) {
@@ -2146,8 +2135,7 @@
/*[[check-2]]*/
}
)",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:13:7")));
+ "unsafe: input.cc:13:7");
ExpectLatticeChecksFor(
R"(
@@ -2166,8 +2154,7 @@
/*[[check-4]]*/
}
)",
- UnorderedElementsAre(Pair("check-3", "safe"),
- Pair("check-4", "unsafe: input.cc:13:7")));
+ "unsafe: input.cc:13:7");
}
TEST_P(UncheckedOptionalAccessTest, UniquePtrToStructWithOptionalField) {
@@ -2195,7 +2182,7 @@
/*[[check-2]]*/
}
)",
- UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
@@ -2212,7 +2199,7 @@
/*[[check-1]]*/
}
)",
- UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:9:7")));
+ "unsafe: input.cc:9:7");
ExpectLatticeChecksFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2226,7 +2213,7 @@
/*[[check-2]]*/
}
)",
- UnorderedElementsAre(Pair("check-2", "unsafe: input.cc:9:7")));
+ "unsafe: input.cc:9:7");
ExpectLatticeChecksFor(
R"(
@@ -2242,7 +2229,7 @@
/*[[check-3]]*/
}
)",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:10:7")));
+ "unsafe: input.cc:10:7");
ExpectLatticeChecksFor(
R"(
@@ -2258,7 +2245,7 @@
/*[[check-4]]*/
}
)",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
+ "unsafe: input.cc:10:7");
}
// Verifies that the model sees through aliases.
@@ -2275,7 +2262,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+ "unsafe: input.cc:8:7");
}
TEST_P(UncheckedOptionalAccessTest, OptionalValueOptional) {
@@ -2295,7 +2282,7 @@
}
}
)",
- UnorderedElementsAre(Pair("access", "safe")));
+ "safe");
// Mutation is supported for nested values.
ExpectLatticeChecksFor(
@@ -2312,7 +2299,7 @@
}
}
)",
- UnorderedElementsAre(Pair("reset", "unsafe: input.cc:9:9")));
+ "unsafe: input.cc:9:9");
}
// Tests that structs can be nested. We use an optional field because its easy
@@ -2333,7 +2320,7 @@
}
}
)",
- UnorderedElementsAre(Pair("access", "safe")));
+ "safe");
}
TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
@@ -2363,7 +2350,7 @@
/*[[merge]]*/
}
)",
- UnorderedElementsAre(Pair("merge", "unsafe: input.cc:19:7")));
+ "unsafe: input.cc:19:7");
}
TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) {
@@ -2383,7 +2370,7 @@
/*[[check]]*/
}
)",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ "unsafe: input.cc:12:7");
}
TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
@@ -2399,7 +2386,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-1", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"code(
#include "unchecked_optional_access_test.h"
@@ -2412,7 +2399,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-2", "safe")));
+ "safe");
ExpectLatticeChecksFor(
R"code(
@@ -2426,7 +2413,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+ "unsafe: input.cc:7:9");
ExpectLatticeChecksFor(R"code(
#include "unchecked_optional_access_test.h"
@@ -2440,7 +2427,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-4", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -2454,7 +2441,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check-5", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -2468,7 +2455,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check-6", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -2487,7 +2474,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check", "safe")));
+ "safe");
// FIXME: Add support for operator==.
// ExpectLatticeChecksFor(R"(
@@ -2502,7 +2489,7 @@
// }
// }
// )",
- // UnorderedElementsAre(Pair("check-7", "safe")));
+ // "safe");
}
TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
@@ -2526,8 +2513,7 @@
}
}
)code",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:15:9")));
+ "unsafe: input.cc:15:9");
ExpectLatticeChecksFor(R"code(
#include "unchecked_optional_access_test.h"
@@ -2545,7 +2531,7 @@
/*[[check-3]]*/
}
)code",
- UnorderedElementsAre(Pair("check-3", "safe")));
+ "safe");
ExpectLatticeChecksFor(
R"code(
@@ -2563,7 +2549,7 @@
/*[[check-4]]*/
}
)code",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:12:7")));
+ "unsafe: input.cc:12:7");
ExpectLatticeChecksFor(
R"code(
@@ -2580,7 +2566,7 @@
/*[[check-5]]*/
}
)code",
- UnorderedElementsAre(Pair("check-5", "safe")));
+ "safe");
ExpectLatticeChecksFor(
R"code(
@@ -2597,7 +2583,7 @@
/*[[check-6]]*/
}
)code",
- UnorderedElementsAre(Pair("check-6", "unsafe: input.cc:11:7")));
+ "unsafe: input.cc:11:7");
}
TEST_P(UncheckedOptionalAccessTest, ReassignValueInLoop) {
@@ -2612,7 +2598,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check-1", "safe")));
+ "safe");
ExpectLatticeChecksFor(R"(
#include "unchecked_optional_access_test.h"
@@ -2628,7 +2614,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check-2", "safe")));
+ "safe");
ExpectLatticeChecksFor(
R"(
@@ -2644,7 +2630,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+ "unsafe: input.cc:7:9");
ExpectLatticeChecksFor(
R"(
@@ -2661,7 +2647,7 @@
}
}
)",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:7:9")));
+ "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
@@ -23,6 +23,8 @@
#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/Diagnosis.h"
+#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
#include "clang/Basic/LLVM.h"
#include "clang/Serialization/PCHContainerOperations.h"
@@ -30,6 +32,7 @@
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
@@ -62,21 +65,33 @@
buildStatementToAnnotationMapping(const FunctionDecl *Func,
llvm::Annotations AnnotatedCode);
+struct AnalysisResults {
+ AnalysisResults(
+ ASTContext &Context, const ControlFlowContext &CFCtx,
+ const Environment &Env, TypeErasedDataflowAnalysis &Analysis,
+ llvm::DenseMap<const clang::Stmt *, std::string> &Annotations,
+ std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>> &BlockStates)
+ : Context(Context), CFCtx(CFCtx), Env(Env), Analysis(Analysis),
+ Annotations(Annotations), BlockStates(BlockStates) {}
+
+ ASTContext &Context;
+ const ControlFlowContext &CFCtx;
+ const Environment &Env;
+ TypeErasedDataflowAnalysis &Analysis;
+ llvm::DenseMap<const clang::Stmt *, std::string> &Annotations;
+ std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>> &BlockStates;
+};
+
// Runs dataflow on the body of the function that matches `func_matcher` in code
// snippet `code`. Requires: `Analysis` contains a type `Lattice`.
template <typename AnalysisT>
-llvm::Error checkDataflow(
+llvm::Error checkDataflowHelper(
llvm::StringRef Code,
ast_matchers::internal::Matcher<FunctionDecl> FuncMatcher,
std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis,
- std::function<void(
- llvm::ArrayRef<std::pair<
- std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
- ASTContext &)>
- Expectations,
+ std::function<void(AnalysisResults)> Expectations,
ArrayRef<std::string> Args,
const tooling::FileContentMappings &VirtualMappedFiles = {}) {
- using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
llvm::Annotations AnnotatedCode(Code);
auto Unit = tooling::buildASTFromCodeWithArgs(
@@ -121,35 +136,63 @@
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()].hasValue())
- 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);
+ AnalysisResults AnalysisResults(Context, *CFCtx, Env, Analysis, Annotations,
+ BlockStates);
+ Expectations(AnalysisResults);
return llvm::Error::success();
}
+template <typename AnalysisT>
+llvm::Error checkDataflow(
+ llvm::StringRef Code,
+ ast_matchers::internal::Matcher<FunctionDecl> FuncMatcher,
+ 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,
+ const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+ using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
+
+ return checkDataflowHelper(
+ Code, std::move(FuncMatcher), std::move(MakeAnalysis),
+ [&Expectations](AnalysisResults AnalysisResults) {
+ if (AnalysisResults.BlockStates.empty()) {
+ Expectations({}, AnalysisResults.Context);
+ return;
+ }
+
+ auto &Annotations = AnalysisResults.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 : AnalysisResults.CFCtx.getCFG()) {
+ // Skip blocks that were not evaluated.
+ if (!AnalysisResults.BlockStates[Block->getBlockID()].hasValue())
+ continue;
+
+ transferBlock(
+ AnalysisResults.CFCtx, AnalysisResults.BlockStates, *Block,
+ AnalysisResults.Env, AnalysisResults.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, AnalysisResults.Context);
+ },
+ Args, VirtualMappedFiles);
+}
+
// Runs dataflow on the body of the function named `target_fun` in code snippet
// `code`.
template <typename AnalysisT>
@@ -168,6 +211,29 @@
VirtualMappedFiles);
}
+template <typename AnalysisT, typename Diag>
+llvm::Error checkDataflowDiagnosis(
+ llvm::StringRef Code,
+ ast_matchers::internal::Matcher<FunctionDecl> FuncMatcher,
+ std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis,
+ std::function<Diagnosis<typename AnalysisT::Lattice, llvm::DenseSet<Diag>>(
+ ASTContext &)>
+ MakeDiagnosis,
+ std::function<void(llvm::DenseSet<Diag>, ASTContext &)> Expectations,
+ ArrayRef<std::string> Args,
+ const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+ return checkDataflowHelper(
+ Code, std::move(FuncMatcher), std::move(MakeAnalysis),
+ [&MakeDiagnosis, &Expectations](AnalysisResults AnalysisResults) {
+ auto Diagnose = MakeDiagnosis(AnalysisResults.Context);
+ auto Diags = diagnoseCFG(
+ AnalysisResults.CFCtx, AnalysisResults.BlockStates,
+ AnalysisResults.Env, AnalysisResults.Analysis, Diagnose);
+ Expectations(Diags, AnalysisResults.Context);
+ },
+ Args, VirtualMappedFiles);
+}
+
/// Returns the `ValueDecl` for the given identifier.
///
/// Requirements:
Index: clang/lib/Analysis/FlowSensitive/SourceLocationsLattice.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/SourceLocationsLattice.cpp
+++ clang/lib/Analysis/FlowSensitive/SourceLocationsLattice.cpp
@@ -13,6 +13,7 @@
#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
#include "clang/AST/ASTContext.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
@@ -30,14 +31,14 @@
: LatticeJoinEffect::Changed;
}
-std::string DebugString(const SourceLocationsLattice &Lattice,
+std::string DebugString(const llvm::DenseSet<SourceLocation> &Locs,
const ASTContext &Context) {
- if (Lattice.getSourceLocations().empty())
+ if (Locs.empty())
return "";
std::vector<std::string> Locations;
- Locations.reserve(Lattice.getSourceLocations().size());
- for (const clang::SourceLocation &Loc : Lattice.getSourceLocations()) {
+ Locations.reserve(Locs.size());
+ for (const clang::SourceLocation &Loc : Locs) {
Locations.push_back(Loc.printToString(Context.getSourceManager()));
}
std::sort(Locations.begin(), Locations.end());
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,6 +22,8 @@
#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/DenseSet.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include <cassert>
@@ -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,48 @@
.Build();
}
+void diagnoseUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
+ DiagnoseState<llvm::DenseSet<SourceLocation>> &State) {
+ if (auto *OptionalVal =
+ State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) {
+ auto *Prop = OptionalVal->getProperty("has_value");
+ if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) {
+ if (State.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.
+ State.Diags.insert(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<DiagnoseState<llvm::DenseSet<SourceLocation>>>()
+ // optional::value
+ .CaseOf<CXXMemberCallExpr>(
+ valueCall(IgnorableOptional),
+ [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
+ DiagnoseState<llvm::DenseSet<SourceLocation>> &State) {
+ diagnoseUnwrapCall(E, E->getImplicitObjectArgument(), State);
+ })
+
+ // optional::operator*, optional::operator->
+ .CaseOf<CallExpr>(
+ valueOperatorCall(IgnorableOptional),
+ [](const CallExpr *E, const MatchFinder::MatchResult &,
+ DiagnoseState<llvm::DenseSet<SourceLocation>> &State) {
+ diagnoseUnwrapCall(E, E->getArg(0), State);
+ })
+ .Build();
+}
+
} // namespace
ast_matchers::DeclarationMatcher
@@ -699,5 +752,19 @@
return true;
}
+UncheckedOptionalAccessDiagnosis::UncheckedOptionalAccessDiagnosis(
+ ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options)
+ : Context(AstContext),
+ DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {}
+
+llvm::DenseSet<SourceLocation>
+UncheckedOptionalAccessDiagnosis::diagnose(const Stmt *Stmt,
+ const Environment &Env) {
+ llvm::DenseSet<SourceLocation> Locs;
+ DiagnoseState<llvm::DenseSet<SourceLocation>> State(Locs, Env);
+ DiagnoseMatchSwitch(*Stmt, Context, State);
+ return Locs;
+}
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h
+++ clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h
@@ -55,8 +55,8 @@
llvm::DenseSet<SourceLocation> Locs;
};
-/// Returns a string that represents the source locations of the lattice.
-std::string DebugString(const SourceLocationsLattice &Lattice,
+/// Returns a string that represents the source locations.
+std::string DebugString(const llvm::DenseSet<SourceLocation> &Locs,
const ASTContext &Context);
} // namespace dataflow
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 "llvm/ADT/DenseSet.h"
namespace clang {
namespace dataflow {
@@ -71,6 +73,20 @@
MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
};
+class UncheckedOptionalAccessDiagnosis {
+public:
+ UncheckedOptionalAccessDiagnosis(
+ ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {});
+
+ llvm::DenseSet<SourceLocation> diagnose(const Stmt *Stmt,
+ const Environment &Env);
+
+private:
+ ASTContext &Context;
+ MatchSwitch<DiagnoseState<llvm::DenseSet<SourceLocation>>>
+ DiagnoseMatchSwitch;
+};
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
+++ clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
@@ -44,6 +44,16 @@
Environment &Env;
};
+/// A common form of state shared between the cases of a diagnose function.
+template <typename DiagsT> struct DiagnoseState {
+ DiagnoseState(DiagsT &Diags, const Environment &Env)
+ : Diags(Diags), Env(Env) {}
+
+ /// Current set of diagnostics.
+ DiagsT &Diags;
+ const Environment &Env;
+};
+
/// Matches against `Stmt` and, based on its structure, dispatches to an
/// appropriate handler.
template <typename State>
Index: clang/include/clang/Analysis/FlowSensitive/Diagnosis.h
===================================================================
--- /dev/null
+++ clang/include/clang/Analysis/FlowSensitive/Diagnosis.h
@@ -0,0 +1,63 @@
+//===- Diagnosis.h ----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines a type and helper function for gathering diagnostics using
+// the results of a dataflow analysis.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_DIAGNOSIS_H
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_DIAGNOSIS_H
+
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include <functional>
+#include <utility>
+#include <vector>
+
+namespace clang {
+namespace dataflow {
+
+/// Looks at a single statement using the `Lattice` and `Environment` at that
+/// program point from running a dataflow analysis, and returns any diagnostics.
+template <typename Lattice, typename Diags>
+using Diagnosis =
+ std::function<Diags(const Stmt *, const Lattice &, const Environment &)>;
+
+/// Collects diagnostics from all blocks in a CFG, given some dataflow analysis
+/// results and a `Diagnose` function which can be run on individual statements.
+template <typename Lattice, typename Diag>
+llvm::DenseSet<Diag> diagnoseCFG(
+ const ControlFlowContext &CFCtx,
+ std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>> &BlockStates,
+ const Environment &Env, TypeErasedDataflowAnalysis &Analysis,
+ Diagnosis<Lattice, llvm::DenseSet<Diag>> Diagnose) {
+ llvm::DenseSet<Diag> Diags;
+ for (const CFGBlock *Block : CFCtx.getCFG()) {
+ // Skip blocks that were not evaluated.
+ if (!BlockStates[Block->getBlockID()].hasValue())
+ continue;
+ transferBlock(
+ CFCtx, BlockStates, *Block, Env, Analysis,
+ [&Diags, &Diagnose](const clang::CFGStmt &Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ auto *L = llvm::any_cast<Lattice>(&State.Lattice.Value);
+ auto Other = Diagnose(Stmt.getStmt(), *L, State.Env);
+ Diags.insert(Other.begin(), Other.end());
+ });
+ }
+ return std::move(Diags);
+}
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_DIAGNOSIS_H
Index: clang/docs/tools/clang-formatted-files.txt
===================================================================
--- clang/docs/tools/clang-formatted-files.txt
+++ clang/docs/tools/clang-formatted-files.txt
@@ -129,6 +129,7 @@
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/DataflowLattice.h
clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h
+clang/include/clang/Analysis/FlowSensitive/Diagnosis.h
clang/include/clang/Analysis/FlowSensitive/MapLattice.h
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
clang/include/clang/Analysis/FlowSensitive/Solver.h
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
