From ce205cffdfa0f16ce9441ba46fa43e23cecf8be7 Mon Sep 17 00:00:00 2001 From: Stanislav Gatev Date: Tue, 8 Mar 2022 09:55:00 +0000 Subject: [PATCH] [clang][dataflow] Add analysis that detects unsafe accesses to optionals Adds a dataflow analysis that detects unsafe accesses to values of type `std::optional`, `absl::optional`, or `base::Optional`. Reviewed-by: ymandel, xazax.hun Differential Revision: https://reviews.llvm.org/D121197 --- .../Models/UncheckedOptionalAccessModel.h | 40 +++ .../clang}/Analysis/FlowSensitive/TestingSupport.h | 6 +- clang/lib/Analysis/FlowSensitive/CMakeLists.txt | 3 + .../Analysis/FlowSensitive/Models/CMakeLists.txt | 10 + .../Models/UncheckedOptionalAccessModel.cpp | 136 +++++++++ .../Analysis/FlowSensitive/TestingSupport.cpp | 2 +- .../Analysis/FlowSensitive/CMakeLists.txt | 3 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 2 +- .../Analysis/FlowSensitive/MatchSwitchTest.cpp | 2 +- .../Analysis/FlowSensitive/Models/CMakeLists.txt | 28 ++ .../Models/UncheckedOptionalAccessModelTest.cpp | 332 +++++++++++++++++++++ .../MultiVarConstantPropagationTest.cpp | 2 +- .../SingleVarConstantPropagationTest.cpp | 2 +- .../Analysis/FlowSensitive/TestingSupportTest.cpp | 2 +- .../Analysis/FlowSensitive/TransferTest.cpp | 2 +- .../TypeErasedDataflowAnalysisTest.cpp | 2 +- 16 files changed, 562 insertions(+), 12 deletions(-) create mode 100644 clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h rename clang/{unittests => include/clang}/Analysis/FlowSensitive/TestingSupport.h (97%) create mode 100644 clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt create mode 100644 clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp rename clang/{unittests => lib}/Analysis/FlowSensitive/TestingSupport.cpp (98%) create mode 100644 clang/unittests/Analysis/FlowSensitive/Models/CMakeLists.txt create mode 100644 clang/unittests/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModelTest.cpp diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h new file mode 100644 index 0000000..e5ad0ca --- /dev/null +++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h @@ -0,0 +1,40 @@ +#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H +#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H + +#include "clang/AST/ASTContext.h" +#include "clang/AST/Stmt.h" +#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/MatchSwitch.h" +#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" + +namespace clang { +namespace dataflow { + +/// Dataflow analysis that discovers unsafe accesses of optional values and +/// adds the respective source locations to the lattice. +/// +/// Models the `std::optional`, `absl::optional`, and `base::Optional` types. +/// +/// FIXME: Consider separating the models from the unchecked access analysis. +class UncheckedOptionalAccessModel + : public DataflowAnalysis { +public: + explicit UncheckedOptionalAccessModel(ASTContext &AstContext); + + static SourceLocationsLattice initialElement() { + return SourceLocationsLattice(); + } + + void transfer(const Stmt *Stmt, SourceLocationsLattice &State, + Environment &Env); + +private: + MatchSwitch> TransferMatchSwitch; +}; + +} // namespace dataflow +} // namespace clang + +#endif // CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/include/clang/Analysis/FlowSensitive/TestingSupport.h similarity index 97% rename from clang/unittests/Analysis/FlowSensitive/TestingSupport.h rename to clang/include/clang/Analysis/FlowSensitive/TestingSupport.h index 957d73f..9878081 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h +++ b/clang/include/clang/Analysis/FlowSensitive/TestingSupport.h @@ -10,8 +10,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_ANALYSIS_FLOW_SENSITIVE_TESTING_SUPPORT_H_ -#define LLVM_CLANG_ANALYSIS_FLOW_SENSITIVE_TESTING_SUPPORT_H_ +#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_TESTING_SUPPORT_H +#define CLANG_ANALYSIS_FLOWSENSITIVE_TESTING_SUPPORT_H #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" @@ -179,4 +179,4 @@ const ValueDecl *findValueDecl(ASTContext &ASTCtx, llvm::StringRef Name); } // namespace dataflow } // namespace clang -#endif // LLVM_CLANG_ANALYSIS_FLOW_SENSITIVE_TESTING_SUPPORT_H_ +#endif // CLANG_ANALYSIS_FLOWSENSITIVE_TESTING_SUPPORT_H diff --git a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt index cfe3c8e..df7a09f 100644 --- a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt +++ b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangAnalysisFlowSensitive DataflowAnalysisContext.cpp DataflowEnvironment.cpp SourceLocationsLattice.cpp + TestingSupport.cpp Transfer.cpp TypeErasedDataflowAnalysis.cpp WatchedLiteralsSolver.cpp @@ -12,3 +13,5 @@ add_clang_library(clangAnalysisFlowSensitive clangAST clangBasic ) + +add_subdirectory(Models) diff --git a/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt new file mode 100644 index 0000000..5bed00c --- /dev/null +++ b/clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt @@ -0,0 +1,10 @@ +add_clang_library(clangAnalysisFlowSensitiveModels + UncheckedOptionalAccessModel.cpp + + LINK_LIBS + clangAnalysis + clangAnalysisFlowSensitive + clangAST + clangASTMatchers + clangBasic + ) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp new file mode 100644 index 0000000..bb6aa5f --- /dev/null +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -0,0 +1,136 @@ +#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Stmt.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/MatchSwitch.h" +#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" +#include "clang/Analysis/FlowSensitive/Value.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include + +namespace clang { +namespace dataflow { +namespace { + +using namespace ::clang::ast_matchers; + +using LatticeTransferState = TransferState; + +static auto optionalClass() { + return classTemplateSpecializationDecl( + anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"), + hasName("__optional_destruct_base"), hasName("absl::optional"), + hasName("base::Optional")), + hasTemplateArgument(0, refersToType(type().bind("T")))); +} + +static auto hasOptionalType() { return hasType(optionalClass()); } + +static auto isOptionalMemberCallWithName(llvm::StringRef MemberName) { + return cxxMemberCallExpr( + on(expr(unless(cxxThisExpr()))), + callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass())))); +} + +static auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) { + return cxxOperatorCallExpr(hasOverloadedOperatorName(OperatorName), + callee(cxxMethodDecl(ofClass(optionalClass())))); +} + +/// Returns the symbolic value that represents the "has_value" property of the +/// optional value `Val`. Returns null if `Val` is null. +static BoolValue *getHasValue(Value *Val) { + if (auto *OptionalVal = cast_or_null(Val)) { + return cast(OptionalVal->getProperty("has_value")); + } + return nullptr; +} + +static void initializeOptionalReference(const Expr *OptionalExpr, + LatticeTransferState &State) { + if (auto *OptionalVal = cast_or_null( + State.Env.getValue(*OptionalExpr, SkipPast::Reference))) { + if (OptionalVal->getProperty("has_value") == nullptr) { + OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue()); + } + } +} + +static void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, + LatticeTransferState &State) { + if (auto *OptionalVal = cast_or_null( + State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer))) { + auto *HasValueVal = getHasValue(OptionalVal); + assert(HasValueVal != nullptr); + + if (State.Env.flowConditionImplies(*HasValueVal)) + return; + } + + // Record that this unwrap is *not* provably safe. + State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc()); +} + +static void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr, + LatticeTransferState &State) { + if (auto *OptionalVal = cast_or_null( + State.Env.getValue(*CallExpr->getImplicitObjectArgument(), + SkipPast::ReferenceThenPointer))) { + auto *HasValueVal = getHasValue(OptionalVal); + assert(HasValueVal != nullptr); + + auto &CallExprLoc = State.Env.createStorageLocation(*CallExpr); + State.Env.setValue(CallExprLoc, *HasValueVal); + State.Env.setStorageLocation(*CallExpr, CallExprLoc); + } +} + +static auto buildTransferMatchSwitch() { + return MatchSwitchBuilder() + // Attach a symbolic "has_value" state to optional values that we see for + // the first time. + .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()), + initializeOptionalReference) + + // optional::value + .CaseOf( + isOptionalMemberCallWithName("value"), + +[](const CXXMemberCallExpr *E, LatticeTransferState &State) { + transferUnwrapCall(E, E->getImplicitObjectArgument(), State); + }) + + // optional::operator*, optional::operator-> + .CaseOf( + expr(anyOf(isOptionalOperatorCallWithName("*"), + isOptionalOperatorCallWithName("->"))), + +[](const CallExpr *E, LatticeTransferState &State) { + transferUnwrapCall(E, E->getArg(0), State); + }) + + // optional::has_value + .CaseOf(isOptionalMemberCallWithName("has_value"), + transferOptionalHasValueCall) + + .Build(); +} + +} // namespace + +UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(ASTContext &Ctx) + : DataflowAnalysis( + Ctx), + TransferMatchSwitch(buildTransferMatchSwitch()) {} + +void UncheckedOptionalAccessModel::transfer(const Stmt *S, + SourceLocationsLattice &L, + Environment &Env) { + LatticeTransferState State(L, Env); + TransferMatchSwitch(*S, getASTContext(), State); +} + +} // namespace dataflow +} // namespace clang diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp b/clang/lib/Analysis/FlowSensitive/TestingSupport.cpp similarity index 98% rename from clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp rename to clang/lib/Analysis/FlowSensitive/TestingSupport.cpp index 4c5efa7..17c0189 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp +++ b/clang/lib/Analysis/FlowSensitive/TestingSupport.cpp @@ -1,4 +1,4 @@ -#include "TestingSupport.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/Stmt.h" diff --git a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt index d260850..131b62c 100644 --- a/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt +++ b/clang/unittests/Analysis/FlowSensitive/CMakeLists.txt @@ -11,7 +11,6 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests MultiVarConstantPropagationTest.cpp SingleVarConstantPropagationTest.cpp SourceLocationsLatticeTest.cpp - TestingSupport.cpp TestingSupportTest.cpp TransferTest.cpp TypeErasedDataflowAnalysisTest.cpp @@ -36,3 +35,5 @@ target_link_libraries(ClangAnalysisFlowSensitiveTests PRIVATE LLVMTestingSupport ) + +add_subdirectory(Models) diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 51b40f2..ef0049f 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -8,8 +8,8 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "NoopAnalysis.h" -#include "TestingSupport.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h" #include "gmock/gmock.h" diff --git a/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp b/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp index b99e5c6..9b59ee2 100644 --- a/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp @@ -13,7 +13,6 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/FlowSensitive/MatchSwitch.h" -#include "TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" @@ -24,6 +23,7 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/MapLattice.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" diff --git a/clang/unittests/Analysis/FlowSensitive/Models/CMakeLists.txt b/clang/unittests/Analysis/FlowSensitive/Models/CMakeLists.txt new file mode 100644 index 0000000..df378c4 --- /dev/null +++ b/clang/unittests/Analysis/FlowSensitive/Models/CMakeLists.txt @@ -0,0 +1,28 @@ +set(LLVM_LINK_COMPONENTS + FrontendOpenMP + Support + ) + +add_clang_unittest(ClangAnalysisFlowSensitiveModelsTests + UncheckedOptionalAccessModelTest.cpp + ) + +clang_target_link_libraries(ClangAnalysisFlowSensitiveModelsTests + PRIVATE + clangAnalysis + clangAnalysisFlowSensitive + clangAnalysisFlowSensitiveModels + clangAST + clangASTMatchers + clangBasic + clangFrontend + clangLex + clangSerialization + clangTesting + clangTooling + ) + +target_link_libraries(ClangAnalysisFlowSensitiveModelsTests + PRIVATE + LLVMTestingSupport + ) diff --git a/clang/unittests/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModelTest.cpp new file mode 100644 index 0000000..5d99d79 --- /dev/null +++ b/clang/unittests/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModelTest.cpp @@ -0,0 +1,332 @@ +#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Error.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include +#include +#include + +using namespace clang; +using namespace dataflow; +using namespace test; + +using ::testing::Pair; +using ::testing::UnorderedElementsAre; + +static constexpr char StdTypeTraitsHeader[] = R"( +namespace std { + +template< class T > struct remove_reference {typedef T type;}; +template< class T > struct remove_reference {typedef T type;}; +template< class T > struct remove_reference {typedef T type;}; + +template + using remove_reference_t = typename remove_reference::type; + +} // namespace std +)"; + +static constexpr char StdUtilityHeader[] = R"( +#include "std_type_traits.h" + +namespace std { + +template +constexpr std::remove_reference_t&& move(T&& x); + +} // namespace std +)"; + +static constexpr char StdOptionalHeader[] = R"( +namespace std { + +template +class optional { + public: + constexpr optional() noexcept; + + const T& operator*() const&; + T& operator*() &; + const T&& operator*() const&&; + T&& operator*() &&; + + const T* operator->() const; + T* operator->(); + + const T& value() const&; + T& value() &; + const T&& value() const&&; + T&& value() &&; + + constexpr bool has_value() const noexcept; +}; + +} // namespace std +)"; + +static constexpr char AbslOptionalHeader[] = R"( +namespace absl { + +template +class optional { + public: + constexpr optional() noexcept; + + const T& operator*() const&; + T& operator*() &; + const T&& operator*() const&&; + T&& operator*() &&; + + const T* operator->() const; + T* operator->(); + + const T& value() const&; + T& value() &; + const T&& value() const&&; + T&& value() &&; + + constexpr bool has_value() const noexcept; +}; + +} // namespace absl +)"; + +static constexpr char BaseOptionalHeader[] = R"( +namespace base { + +template +class Optional { + public: + constexpr Optional() noexcept; + + const T& operator*() const&; + T& operator*() &; + const T&& operator*() const&&; + T&& operator*() &&; + + const T* operator->() const; + T* operator->(); + + const T& value() const&; + T& value() &; + const T&& value() const&&; + T&& value() &&; + + constexpr bool has_value() const noexcept; +}; + +} // 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) { + size_t Pos = 0; + while (true) { + Pos = S.find(Pattern, Pos); + if (Pos == std::string::npos) + break; + S.replace(Pos, Pattern.size(), Replacement); + } +} + +struct OptionalTypeIdentifier { + std::string NamespaceName; + std::string TypeName; +}; + +class UncheckedOptionalAccessTest + : public ::testing::TestWithParam { +protected: + template + void ExpectLatticeChecksFor(std::string SourceCode, + LatticeChecksMatcher MatchesLatticeChecks) { + ExpectLatticeChecksFor(SourceCode, ast_matchers::hasName("target"), + MatchesLatticeChecks); + } + +private: + template + void ExpectLatticeChecksFor(std::string SourceCode, + FuncDeclMatcher FuncMatcher, + LatticeChecksMatcher MatchesLatticeChecks) { + ReplaceAllOccurrences(SourceCode, "$ns", GetParam().NamespaceName); + ReplaceAllOccurrences(SourceCode, "$optional", GetParam().TypeName); + + std::vector> Headers; + Headers.emplace_back("std_type_traits.h", StdTypeTraitsHeader); + Headers.emplace_back("std_utility.h", StdUtilityHeader); + Headers.emplace_back("std_optional.h", StdOptionalHeader); + Headers.emplace_back("absl_optional.h", AbslOptionalHeader); + Headers.emplace_back("base_optional.h", BaseOptionalHeader); + Headers.emplace_back("unchecked_optional_access_test.h", R"( + #include "absl_optional.h" + #include "base_optional.h" + #include "std_optional.h" + #include "std_utility.h" + )"); + const tooling::FileContentMappings FileContents(Headers.begin(), + Headers.end()); + llvm::Error Error = checkDataflow( + SourceCode, FuncMatcher, + [](ASTContext &Ctx, Environment &) { + return UncheckedOptionalAccessModel(Ctx); + }, + [&MatchesLatticeChecks]( + llvm::ArrayRef>> + CheckToLatticeMap, + ASTContext &Ctx) { + // FIXME: Consider using a matcher instead of translating + // `CheckToLatticeMap` to `CheckToStringifiedLatticeMap`. + std::vector> + 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); + if (Error) + FAIL() << llvm::toString(std::move(Error)); + } +}; + +INSTANTIATE_TEST_SUITE_P( + UncheckedOptionalUseTestInst, UncheckedOptionalAccessTest, + ::testing::Values(OptionalTypeIdentifier{"std", "optional"}, + OptionalTypeIdentifier{"absl", "optional"}, + OptionalTypeIdentifier{"base", "Optional"}), + [](const ::testing::TestParamInfo &Info) { + return Info.param.NamespaceName; + }); + +TEST_P(UncheckedOptionalAccessTest, EmptyFunctionBody) { + ExpectLatticeChecksFor(R"( + void target() { + (void)0; + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); +} + +TEST_P(UncheckedOptionalAccessTest, UnwrapUsingValueNoCheck) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + std::move(opt).value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7"))); +} + +TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorStarNoCheck) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + *opt; + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + *std::move(opt); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8"))); +} + +TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + struct Foo { + void foo(); + }; + + void target($ns::$optional opt) { + opt->foo(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + struct Foo { + void foo(); + }; + + void target($ns::$optional opt) { + std::move(opt)->foo(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7"))); +} + +TEST_P(UncheckedOptionalAccessTest, UnwrapWithCheck) { + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + if (opt.has_value()) { + opt.value(); + /*[[check]]*/ + } + } + )", + UnorderedElementsAre(Pair("check", "safe"))); +} + +// FIXME: Add support for: +// - constructors (default, copy, move, non-standard) +// - assignment operators (default, copy, move, non-standard) +// - operator bool +// - emplace +// - reset +// - value_or +// - swap +// - make_optional +// - invalidation (passing optional by non-const reference/pointer) diff --git a/clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp b/clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp index 9535f99..e246da7 100644 --- a/clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp @@ -12,7 +12,6 @@ // //===----------------------------------------------------------------------===// -#include "TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" @@ -23,6 +22,7 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/MapLattice.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" diff --git a/clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp b/clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp index e147249..6d35f05 100644 --- a/clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp @@ -12,7 +12,6 @@ // //===----------------------------------------------------------------------===// -#include "TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" @@ -22,6 +21,7 @@ #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp b/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp index 9608790..8703273 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp @@ -1,4 +1,4 @@ -#include "TestingSupport.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "NoopAnalysis.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 49aaca9..66ca0d0 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -7,13 +7,13 @@ //===----------------------------------------------------------------------===// #include "NoopAnalysis.h" -#include "TestingSupport.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/StorageLocation.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/LangStandard.h" #include "llvm/ADT/ArrayRef.h" diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp index f93b3fc..06d8891 100644 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "NoopAnalysis.h" -#include "TestingSupport.h" #include "clang/AST/Decl.h" #include "clang/AST/ExprCXX.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -17,6 +16,7 @@ #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" +#include "clang/Analysis/FlowSensitive/TestingSupport.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h" #include "clang/Tooling/Tooling.h" -- 2.7.4