Skip to content

Commit

Permalink
[clang][dataflow] Add analysis that detects unsafe accesses to optionals
Browse files Browse the repository at this point in the history
This commit reverts e0cc28d and moves
UncheckedOptionalAccessModelTest.cpp into clang/unittests/Analysis/FlowSensitive,
to avoid build failures. The test will be moved back into a Models subdir
in a follow up patch that will address the build configuration issues.

Original description:

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
  • Loading branch information
sgatev committed Mar 10, 2022
1 parent eb4037f commit af98b0a
Show file tree
Hide file tree
Showing 6 changed files with 531 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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<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
2 changes: 2 additions & 0 deletions clang/lib/Analysis/FlowSensitive/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ add_clang_library(clangAnalysisFlowSensitive
clangAST
clangBasic
)

add_subdirectory(Models)
10 changes: 10 additions & 0 deletions clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
add_clang_library(clangAnalysisFlowSensitiveModels
UncheckedOptionalAccessModel.cpp

LINK_LIBS
clangAnalysis
clangAnalysisFlowSensitive
clangAST
clangASTMatchers
clangBasic
)
Original file line number Diff line number Diff line change
@@ -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 <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
2 changes: 2 additions & 0 deletions clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests
TransferTest.cpp
TypeErasedDataflowAnalysisTest.cpp
SolverTest.cpp
UncheckedOptionalAccessModelTest.cpp
)

clang_target_link_libraries(ClangAnalysisFlowSensitiveTests
PRIVATE
clangAnalysis
clangAnalysisFlowSensitive
clangAnalysisFlowSensitiveModels
clangAST
clangASTMatchers
clangBasic
Expand Down
Loading

0 comments on commit af98b0a

Please sign in to comment.