Revert "[clang][dataflow] Add analysis that detects unsafe accesses to optionals"

This reverts commit ce205cffdfa0f16ce9441ba46fa43e23cecf8be7.
This commit is contained in:
Stanislav Gatev 2022-03-09 09:51:03 +00:00
parent e3d87fd6e5
commit e0cc28dfdc
16 changed files with 12 additions and 562 deletions

View File

@ -1,40 +0,0 @@
#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<UncheckedOptionalAccessModel,
SourceLocationsLattice> {
public:
explicit UncheckedOptionalAccessModel(ASTContext &AstContext);
static SourceLocationsLattice initialElement() {
return SourceLocationsLattice();
}
void transfer(const Stmt *Stmt, SourceLocationsLattice &State,
Environment &Env);
private:
MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
};
} // namespace dataflow
} // namespace clang
#endif // CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H

View File

@ -3,7 +3,6 @@ add_clang_library(clangAnalysisFlowSensitive
DataflowAnalysisContext.cpp
DataflowEnvironment.cpp
SourceLocationsLattice.cpp
TestingSupport.cpp
Transfer.cpp
TypeErasedDataflowAnalysis.cpp
WatchedLiteralsSolver.cpp
@ -13,5 +12,3 @@ add_clang_library(clangAnalysisFlowSensitive
clangAST
clangBasic
)
add_subdirectory(Models)

View File

@ -1,10 +0,0 @@
add_clang_library(clangAnalysisFlowSensitiveModels
UncheckedOptionalAccessModel.cpp
LINK_LIBS
clangAnalysis
clangAnalysisFlowSensitive
clangAST
clangASTMatchers
clangBasic
)

View File

@ -1,136 +0,0 @@
#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 <cassert>
namespace clang {
namespace dataflow {
namespace {
using namespace ::clang::ast_matchers;
using LatticeTransferState = TransferState<SourceLocationsLattice>;
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<StructValue>(Val)) {
return cast<BoolValue>(OptionalVal->getProperty("has_value"));
}
return nullptr;
}
static void initializeOptionalReference(const Expr *OptionalExpr,
LatticeTransferState &State) {
if (auto *OptionalVal = cast_or_null<StructValue>(
State.Env.getValue(*OptionalExpr, SkipPast::Reference))) {
if (OptionalVal->getProperty("has_value") == nullptr) {
OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue());
}
}
}
static void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
LatticeTransferState &State) {
if (auto *OptionalVal = cast_or_null<StructValue>(
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<StructValue>(
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<LatticeTransferState>()
// 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<UncheckedOptionalAccessModel, SourceLocationsLattice>(
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

View File

@ -11,6 +11,7 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests
MultiVarConstantPropagationTest.cpp
SingleVarConstantPropagationTest.cpp
SourceLocationsLatticeTest.cpp
TestingSupport.cpp
TestingSupportTest.cpp
TransferTest.cpp
TypeErasedDataflowAnalysisTest.cpp
@ -35,5 +36,3 @@ target_link_libraries(ClangAnalysisFlowSensitiveTests
PRIVATE
LLVMTestingSupport
)
add_subdirectory(Models)

View File

@ -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"

View File

@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
@ -23,7 +24,6 @@
#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"

View File

@ -1,28 +0,0 @@
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
)

View File

@ -1,332 +0,0 @@
#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 <string>
#include <utility>
#include <vector>
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<T&> {typedef T type;};
template< class T > struct remove_reference<T&&> {typedef T type;};
template <class T>
using remove_reference_t = typename remove_reference<T>::type;
} // namespace std
)";
static constexpr char StdUtilityHeader[] = R"(
#include "std_type_traits.h"
namespace std {
template <typename T>
constexpr std::remove_reference_t<T>&& move(T&& x);
} // namespace std
)";
static constexpr char StdOptionalHeader[] = R"(
namespace std {
template <typename T>
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 <typename T>
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 <typename T>
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<OptionalTypeIdentifier> {
protected:
template <typename LatticeChecksMatcher>
void ExpectLatticeChecksFor(std::string SourceCode,
LatticeChecksMatcher MatchesLatticeChecks) {
ExpectLatticeChecksFor(SourceCode, ast_matchers::hasName("target"),
MatchesLatticeChecks);
}
private:
template <typename FuncDeclMatcher, typename LatticeChecksMatcher>
void ExpectLatticeChecksFor(std::string SourceCode,
FuncDeclMatcher FuncMatcher,
LatticeChecksMatcher MatchesLatticeChecks) {
ReplaceAllOccurrences(SourceCode, "$ns", GetParam().NamespaceName);
ReplaceAllOccurrences(SourceCode, "$optional", GetParam().TypeName);
std::vector<std::pair<std::string, std::string>> 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<UncheckedOptionalAccessModel>(
SourceCode, FuncMatcher,
[](ASTContext &Ctx, Environment &) {
return UncheckedOptionalAccessModel(Ctx);
},
[&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);
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<OptionalTypeIdentifier> &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<int> opt) {
opt.value();
/*[[check]]*/
}
)",
UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
ExpectLatticeChecksFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> 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<int> opt) {
*opt;
/*[[check]]*/
}
)",
UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
ExpectLatticeChecksFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> 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<Foo> 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<Foo> 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<int> 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)

View File

@ -12,6 +12,7 @@
//
//===----------------------------------------------------------------------===//
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
@ -22,7 +23,6 @@
#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"

View File

@ -12,6 +12,7 @@
//
//===----------------------------------------------------------------------===//
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
@ -21,7 +22,6 @@
#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"

View File

@ -1,4 +1,4 @@
#include "clang/Analysis/FlowSensitive/TestingSupport.h"
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Stmt.h"

View File

@ -10,8 +10,8 @@
//
//===----------------------------------------------------------------------===//
#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_TESTING_SUPPORT_H
#define CLANG_ANALYSIS_FLOWSENSITIVE_TESTING_SUPPORT_H
#ifndef LLVM_CLANG_ANALYSIS_FLOW_SENSITIVE_TESTING_SUPPORT_H_
#define LLVM_CLANG_ANALYSIS_FLOW_SENSITIVE_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 // CLANG_ANALYSIS_FLOWSENSITIVE_TESTING_SUPPORT_H
#endif // LLVM_CLANG_ANALYSIS_FLOW_SENSITIVE_TESTING_SUPPORT_H_

View File

@ -1,4 +1,4 @@
#include "clang/Analysis/FlowSensitive/TestingSupport.h"
#include "TestingSupport.h"
#include "NoopAnalysis.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"

View File

@ -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"

View File

@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "NoopAnalysis.h"
#include "TestingSupport.h"
#include "clang/AST/Decl.h"
#include "clang/AST/ExprCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@ -16,7 +17,6 @@
#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"