Skip to content

Commit fe02993

Browse files
authored
[FlowSensitive] [StatusOr] [2/N] Add minimal model (#162932)
This model implements a dataflow analysis for reporting instances of unchecked use of absl::StatusOr values. It makes sure that every use the value of a StatusOr object is dominated by a check that the StatusOr object is ok. This is an example of code that will be flagged by the analysis: ```cpp int f(absl::StatusOr<int> SOR) { return SOR.value(); } ``` This is an example of code that will not be flagged by the analysis: ```cpp int f(absl::StatusOr<int> SOR) { if (SOR.ok()) return SOR.value(); return 0; } ``` This model has successfully been used by Google for some time now. This is the initial commit that adds the simplest possible model, that only models calls to `ok()` and checks for unsafe accesses. I will add more fidelity to the model in follow up changes. The test setup is notable in that it has an extra indirection. This is because we have an internal model that extends the model we intend to upstream, in order to model special constructs only found in our code base. The parametrized test allows us (and anyone who chooses to do this) to make sure our extensions do not break the base functionality. RFC: https://discourse.llvm.org/t/rfc-abseil-unchecked-statusor-use-check/87998
1 parent 1508a8e commit fe02993

File tree

9 files changed

+3122
-0
lines changed

9 files changed

+3122
-0
lines changed
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
//===- UncheckedStatusOrAccessModel.h -------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDSTATUSORACCESSMODEL_H
10+
#define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDSTATUSORACCESSMODEL_H
11+
12+
#include "clang/AST/Type.h"
13+
#include "clang/ASTMatchers/ASTMatchers.h"
14+
#include "clang/Analysis/CFG.h"
15+
#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
16+
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
17+
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
18+
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
19+
#include "clang/Analysis/FlowSensitive/NoopLattice.h"
20+
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
21+
#include "clang/Analysis/FlowSensitive/Value.h"
22+
#include "clang/Basic/SourceLocation.h"
23+
#include "llvm/ADT/SmallVector.h"
24+
#include "llvm/ADT/StringMap.h"
25+
#include "llvm/ADT/StringRef.h"
26+
27+
namespace clang::dataflow::statusor_model {
28+
29+
// The helper functions exported here are for use of downstream vendor
30+
// extensions of this model.
31+
32+
// Match declaration of `absl::StatusOr<T>` and bind `T` to "T".
33+
clang::ast_matchers::DeclarationMatcher statusOrClass();
34+
// Match declaration of `absl::Status`.
35+
clang::ast_matchers::DeclarationMatcher statusClass();
36+
// Match declaration of `absl::internal_statusor::OperatorBase`.
37+
clang::ast_matchers::DeclarationMatcher statusOrOperatorBaseClass();
38+
clang::ast_matchers::TypeMatcher statusOrType();
39+
40+
// Get RecordStorageLocation for the `Status` contained in the `StatusOr`
41+
RecordStorageLocation &locForStatus(RecordStorageLocation &StatusOrLoc);
42+
// Get the StorageLocation for the OK boolean in the `Status`
43+
StorageLocation &locForOk(RecordStorageLocation &StatusLoc);
44+
// Get the OK boolean in the `Status`, and initialize it if necessary.
45+
BoolValue &valForOk(RecordStorageLocation &StatusLoc, Environment &Env);
46+
// Get synthetic fields for the types modelled by
47+
// `UncheckedStatusOrAccessModel`.
48+
llvm::StringMap<QualType> getSyntheticFields(QualType Ty, QualType StatusType,
49+
const CXXRecordDecl &RD);
50+
51+
// Initialize the synthetic fields of the `StatusOr`.
52+
// N.B. if it is already initialized, the value gets reset.
53+
BoolValue &initializeStatusOr(RecordStorageLocation &StatusOrLoc,
54+
Environment &Env);
55+
// Initialize the synthetic fields of the `Status`.
56+
// N.B. if it is already initialized, the value gets reset.
57+
BoolValue &initializeStatus(RecordStorageLocation &StatusLoc, Environment &Env);
58+
59+
bool isRecordTypeWithName(QualType Type, llvm::StringRef TypeName);
60+
// Return true if `Type` is instantiation of `absl::StatusOr<T>`
61+
bool isStatusOrType(QualType Type);
62+
// Return true if `Type` is `absl::Status`
63+
bool isStatusType(QualType Type);
64+
65+
// Get `QualType` for `absl::Status`, or default-constructed
66+
// QualType if it does not exist.
67+
QualType findStatusType(const ASTContext &Ctx);
68+
69+
struct UncheckedStatusOrAccessModelOptions {};
70+
71+
// Dataflow analysis that discovers unsafe uses of StatusOr values.
72+
class UncheckedStatusOrAccessModel
73+
: public DataflowAnalysis<UncheckedStatusOrAccessModel, NoopLattice> {
74+
public:
75+
explicit UncheckedStatusOrAccessModel(ASTContext &Ctx, Environment &Env);
76+
77+
static Lattice initialElement() { return {}; }
78+
79+
void transfer(const CFGElement &Elt, Lattice &L, Environment &Env);
80+
81+
private:
82+
CFGMatchSwitch<TransferState<Lattice>> TransferMatchSwitch;
83+
};
84+
85+
using LatticeTransferState =
86+
TransferState<UncheckedStatusOrAccessModel::Lattice>;
87+
88+
// Extend the Builder with the transfer functions for
89+
// `UncheckedStatusOrAccessModel`. This is useful to write downstream models
90+
// that extend the model.
91+
CFGMatchSwitch<LatticeTransferState>
92+
buildTransferMatchSwitch(ASTContext &Ctx,
93+
CFGMatchSwitchBuilder<LatticeTransferState> Builder);
94+
95+
class UncheckedStatusOrAccessDiagnoser {
96+
public:
97+
explicit UncheckedStatusOrAccessDiagnoser(
98+
UncheckedStatusOrAccessModelOptions Options = {});
99+
100+
llvm::SmallVector<SourceLocation> operator()(
101+
const CFGElement &Elt, ASTContext &Ctx,
102+
const TransferStateForDiagnostics<UncheckedStatusOrAccessModel::Lattice>
103+
&State);
104+
105+
private:
106+
CFGMatchSwitch<const Environment, llvm::SmallVector<SourceLocation>>
107+
DiagnoseMatchSwitch;
108+
};
109+
110+
} // namespace clang::dataflow::statusor_model
111+
112+
#endif // CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDSTATUSORACCESSMODEL_H

clang/lib/Analysis/FlowSensitive/Models/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
add_clang_library(clangAnalysisFlowSensitiveModels
22
ChromiumCheckModel.cpp
33
UncheckedOptionalAccessModel.cpp
4+
UncheckedStatusOrAccessModel.cpp
45

56
LINK_LIBS
67
clangAnalysis

0 commit comments

Comments
 (0)