ymandel created this revision.
ymandel added reviewers: sgatev, xazax.hun.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.
This patch provides the user with the ability to disable all checked of accesses
to optionals that are the pointees of smart pointers. Since smart pointers are
not modeled (yet), the system cannot distinguish safe from unsafe accesses to
optionals through smart pointers. This results in false positives whenever
optionals are used through smart pointers. The patch gives the user the choice
of ignoring all positivess in these cases.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122143
Files:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
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
@@ -1210,7 +1210,9 @@
llvm::Error Error = checkDataflow<UncheckedOptionalAccessModel>(
SourceCode, FuncMatcher,
[](ASTContext &Ctx, Environment &) {
- return UncheckedOptionalAccessModel(Ctx);
+ return UncheckedOptionalAccessModel(
+ Ctx, UncheckedOptionalAccessModelOptions{
+ /*IgnoreSmartPointerPointees=*/true});
},
[&MatchesLatticeChecks](
llvm::ArrayRef<std::pair<
@@ -1911,6 +1913,34 @@
UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
}
+TEST_P(UncheckedOptionalAccessTest, UniquePtrToStructWithOptionalField) {
+ // We suppress diagnostics for values reachable from smart pointers (other
+ // than `optional` itself).
+ ExpectLatticeChecksFor(
+ R"(
+ #include "unchecked_optional_access_test.h"
+
+ template <typename T>
+ struct unique_ptr {
+ T& operator*() &;
+ T* operator->();
+ };
+
+ struct Member {
+ $ns::$optional<int> opt;
+ };
+
+ void target() {
+ unique_ptr<Member> foo;
+ *foo->opt;
+ /*[[check-1]]*/
+ *(*foo).opt;
+ /*[[check-2]]*/
+ }
+ )",
+ UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -45,14 +45,24 @@
auto hasOptionalType() { return hasType(optionalClass()); }
-auto isOptionalMemberCallWithName(llvm::StringRef MemberName) {
+auto isOptionalMemberCallWithName(
+ llvm::StringRef MemberName,
+ llvm::Optional<StatementMatcher> Ignorable = llvm::None) {
+ auto Exception = unless(Ignorable ? expr(anyOf(*Ignorable, cxxThisExpr()))
+ : cxxThisExpr());
return cxxMemberCallExpr(
- on(expr(unless(cxxThisExpr()))),
+ on(expr(Exception)),
callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass()))));
}
-auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) {
- return cxxOperatorCallExpr(hasOverloadedOperatorName(OperatorName),
+auto isOptionalOperatorCallWithName(
+ llvm::StringRef operator_name,
+ llvm::Optional<StatementMatcher> Ignorable = llvm::None) {
+ if (Ignorable)
+ return cxxOperatorCallExpr(hasOverloadedOperatorName(operator_name),
+ callee(cxxMethodDecl(ofClass(optionalClass()))),
+ unless(hasArgument(0, *Ignorable)));
+ return cxxOperatorCallExpr(hasOverloadedOperatorName(operator_name),
callee(cxxMethodDecl(ofClass(optionalClass()))));
}
@@ -283,10 +293,22 @@
transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
}
-auto buildTransferMatchSwitch() {
+llvm::Optional<StatementMatcher>
+ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) {
+ if (Options.IgnoreSmartPointerPointees)
+ return memberExpr(hasObjectExpression(ignoringParenImpCasts(
+ cxxOperatorCallExpr(anyOf(hasOverloadedOperatorName("->"),
+ hasOverloadedOperatorName("*")),
+ unless(hasArgument(0, expr(hasOptionalType())))))));
+ return llvm::None;
+}
+
+auto buildTransferMatchSwitch(
+ 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<LatticeTransferState>()
// Attach a symbolic "has_value" state to optional values that we see for
// the first time.
@@ -321,19 +343,20 @@
// optional::value
.CaseOf<CXXMemberCallExpr>(
- isOptionalMemberCallWithName("value"),
+ isOptionalMemberCallWithName("value", IgnorableOptional),
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
LatticeTransferState &State) {
transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
})
// optional::operator*, optional::operator->
- .CaseOf<CallExpr>(expr(anyOf(isOptionalOperatorCallWithName("*"),
- isOptionalOperatorCallWithName("->"))),
- [](const CallExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
- transferUnwrapCall(E, E->getArg(0), State);
- })
+ .CaseOf<CallExpr>(
+ expr(anyOf(isOptionalOperatorCallWithName("*", IgnorableOptional),
+ isOptionalOperatorCallWithName("->", IgnorableOptional))),
+ [](const CallExpr *E, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ transferUnwrapCall(E, E->getArg(0), State);
+ })
// optional::has_value
.CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("has_value"),
@@ -366,10 +389,11 @@
} // namespace
-UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(ASTContext &Ctx)
+UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(
+ ASTContext &Ctx, UncheckedOptionalAccessModelOptions Options)
: DataflowAnalysis<UncheckedOptionalAccessModel, SourceLocationsLattice>(
Ctx),
- TransferMatchSwitch(buildTransferMatchSwitch()) {}
+ TransferMatchSwitch(buildTransferMatchSwitch(Options)) {}
void UncheckedOptionalAccessModel::transfer(const Stmt *S,
SourceLocationsLattice &L,
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
@@ -24,6 +24,14 @@
namespace clang {
namespace dataflow {
+struct UncheckedOptionalAccessModelOptions {
+ /// Ignore optionals reachable from smart pointers (other than optional
+ /// itself). The analysis does not track smart-pointer pointees (yet), so it
+ /// can't identify when optionals reachable through such pointers are used
+ /// safely, resulting in false positives in all such cases.
+ bool IgnoreSmartPointerPointees = false;
+};
+
/// Dataflow analysis that discovers unsafe accesses of optional values and
/// adds the respective source locations to the lattice.
///
@@ -34,7 +42,8 @@
: public DataflowAnalysis<UncheckedOptionalAccessModel,
SourceLocationsLattice> {
public:
- explicit UncheckedOptionalAccessModel(ASTContext &AstContext);
+ UncheckedOptionalAccessModel(
+ ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {});
static SourceLocationsLattice initialElement() {
return SourceLocationsLattice();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
