[clang] a184a0d - [clang][dataflow] Add support for disabling warnings on smart pointers.
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 25 09:45:00 PDT 2022
Author: Yitzhak Mandelbaum
Date: 2022-03-25T16:44:34Z
New Revision: a184a0d8aae6efc1d7f19a900155b8694178d617
URL: https://github.com/llvm/llvm-project/commit/a184a0d8aae6efc1d7f19a900155b8694178d617
DIFF: https://github.com/llvm/llvm-project/commit/a184a0d8aae6efc1d7f19a900155b8694178d617.diff
LOG: [clang][dataflow] Add support for disabling warnings on smart pointers.
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.
Differential Revision: https://reviews.llvm.org/D122143
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 05a9165e59dde..235121b2e5759 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -24,6 +24,18 @@
namespace clang {
namespace dataflow {
+// FIXME: Explore using an allowlist-approach, where constructs supported by the
+// analysis are always enabled and additional constructs are enabled through the
+// `Options`.
+struct UncheckedOptionalAccessModelOptions {
+ /// Ignore optionals reachable through overloaded `operator*` or `operator->`
+ /// (other than those of the optional type itself). The analysis does not
+ /// equate the results of such calls, so it can't identify when their results
+ /// are used safely (across calls), resulting in false positives in all such
+ /// cases. Note: this option does not cover access through `operator[]`.
+ bool IgnoreSmartPointerDereference = false;
+};
+
/// Dataflow analysis that discovers unsafe accesses of optional values and
/// adds the respective source locations to the lattice.
///
@@ -34,7 +46,8 @@ class UncheckedOptionalAccessModel
: public DataflowAnalysis<UncheckedOptionalAccessModel,
SourceLocationsLattice> {
public:
- explicit UncheckedOptionalAccessModel(ASTContext &AstContext);
+ UncheckedOptionalAccessModel(
+ ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {});
static SourceLocationsLattice initialElement() {
return SourceLocationsLattice();
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index bf325b04967c2..b775698dafb5d 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -45,15 +45,23 @@ auto optionalClass() {
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),
- callee(cxxMethodDecl(ofClass(optionalClass()))));
+auto isOptionalOperatorCallWithName(
+ llvm::StringRef operator_name,
+ llvm::Optional<StatementMatcher> Ignorable = llvm::None) {
+ return cxxOperatorCallExpr(
+ hasOverloadedOperatorName(operator_name),
+ callee(cxxMethodDecl(ofClass(optionalClass()))),
+ Ignorable ? callExpr(unless(hasArgument(0, *Ignorable))) : callExpr());
}
auto isMakeOptionalCall() {
@@ -333,10 +341,22 @@ void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,
transferSwap(*OptionalLoc1, *OptionalLoc2, State);
}
-auto buildTransferMatchSwitch() {
+llvm::Optional<StatementMatcher>
+ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) {
+ if (Options.IgnoreSmartPointerDereference)
+ 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.
@@ -371,19 +391,20 @@ auto buildTransferMatchSwitch() {
// 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"),
@@ -423,10 +444,11 @@ auto buildTransferMatchSwitch() {
} // 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,
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 76340aad5fd4d..6ef4b2a9c3336 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1219,7 +1219,9 @@ class UncheckedOptionalAccessTest
llvm::Error Error = checkDataflow<UncheckedOptionalAccessModel>(
SourceCode, FuncMatcher,
[](ASTContext &Ctx, Environment &) {
- return UncheckedOptionalAccessModel(Ctx);
+ return UncheckedOptionalAccessModel(
+ Ctx, UncheckedOptionalAccessModelOptions{
+ /*IgnoreSmartPointerDereference=*/true});
},
[&MatchesLatticeChecks](
llvm::ArrayRef<std::pair<
@@ -2004,6 +2006,34 @@ TEST_P(UncheckedOptionalAccessTest, StdSwap) {
Pair("check-4", "unsafe: input.cc:13: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 smart_ptr {
+ T& operator*() &;
+ T* operator->();
+ };
+
+ struct Foo {
+ $ns::$optional<int> opt;
+ };
+
+ void target() {
+ smart_ptr<Foo> 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)
More information about the cfe-commits
mailing list