Skip to content

Commit 54309b1

Browse files
jvoungjvoung
authored
[clang][dataflow] Add matchers for smart pointer accessors to be cached (#120102)
This is part 1 of caching for smart pointer accessors, building on top of the CachedConstAccessorsLattice, which caches "normal" accessors. Smart pointer accessors are a bit different in that they may: - have aliases to access the same underlying data (but potentially returning slightly different types like `&` vs `*`). Within a "checked" sequence users may mix uses of the different aliases and the check should apply to any of the spellings. - may have non-const overloads in addition to the const version, where the non-const doesn't actually modify the container Part 2 will follow and add transfer functions utilities. It will also add a user UncheckedOptionalAccessModel. We'd seen false positives when nesting StatusOr<optional<T>> and optional<StatusOr<T>>, etc. which this can help address.
1 parent 42873e0 commit 54309b1

File tree

5 files changed

+406
-0
lines changed

5 files changed

+406
-0
lines changed

‎clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//===-- SmartPointerAccessorCaching.h ---------------------------*- C++ -*-===//
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+
// This file defines utilities to help cache accessors for smart pointer
10+
// like objects.
11+
//
12+
// These should be combined with CachedConstAccessorsLattice.
13+
// Beyond basic const accessors, smart pointers may have the following two
14+
// additional issues:
15+
//
16+
// 1) There may be multiple accessors for the same underlying object, e.g.
17+
// `operator->`, `operator*`, and `get`. Users may use a mixture of these
18+
// accessors, so the cache should unify them.
19+
//
20+
// 2) There may be non-const overloads of accessors. They are still safe to
21+
// cache, as they don't modify the container object.
22+
//===----------------------------------------------------------------------===//
23+
24+
#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SMARTPOINTERACCESSORCACHING_H
25+
#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SMARTPOINTERACCESSORCACHING_H
26+
27+
#include <cassert>
28+
29+
#include "clang/AST/Decl.h"
30+
#include "clang/AST/Stmt.h"
31+
#include "clang/ASTMatchers/ASTMatchers.h"
32+
33+
namespace clang::dataflow {
34+
35+
/// Matchers:
36+
/// For now, these match on any class with an `operator*` or `operator->`
37+
/// where the return types have a similar shape as std::unique_ptr
38+
/// and std::optional.
39+
///
40+
/// - `*` returns a reference to a type `T`
41+
/// - `->` returns a pointer to `T`
42+
/// - `get` returns a pointer to `T`
43+
/// - `value` returns a reference `T`
44+
///
45+
/// (1) The `T` should all match across the accessors (ignoring qualifiers).
46+
///
47+
/// (2) The specific accessor used in a call isn't required to be const,
48+
/// but the class must have a const overload of each accessor.
49+
///
50+
/// For now, we don't have customization to ignore certain classes.
51+
/// For example, if writing a ClangTidy check for `std::optional`, these
52+
/// would also match `std::optional`. In order to have special handling
53+
/// for `std::optional`, we assume the (Matcher, TransferFunction) case
54+
/// with custom handling is ordered early so that these generic cases
55+
/// do not trigger.
56+
ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar();
57+
ast_matchers::StatementMatcher isSmartPointerLikeOperatorArrow();
58+
ast_matchers::StatementMatcher isSmartPointerLikeValueMethodCall();
59+
ast_matchers::StatementMatcher isSmartPointerLikeGetMethodCall();
60+
61+
} // namespace clang::dataflow
62+
63+
#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SMARTPOINTERACCESSORCACHING_H

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_clang_library(clangAnalysisFlowSensitive
1010
Logger.cpp
1111
RecordOps.cpp
1212
SimplifyConstraints.cpp
13+
SmartPointerAccessorCaching.cpp
1314
Transfer.cpp
1415
TypeErasedDataflowAnalysis.cpp
1516
Value.cpp

‎clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
#include "clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h"
2+
3+
#include "clang/AST/CanonicalType.h"
4+
#include "clang/AST/DeclCXX.h"
5+
#include "clang/ASTMatchers/ASTMatchers.h"
6+
#include "clang/ASTMatchers/ASTMatchersMacros.h"
7+
#include "clang/Basic/OperatorKinds.h"
8+
9+
namespace clang::dataflow {
10+
11+
namespace {
12+
13+
using ast_matchers::callee;
14+
using ast_matchers::cxxMemberCallExpr;
15+
using ast_matchers::cxxMethodDecl;
16+
using ast_matchers::cxxOperatorCallExpr;
17+
using ast_matchers::hasName;
18+
using ast_matchers::hasOverloadedOperatorName;
19+
using ast_matchers::ofClass;
20+
using ast_matchers::parameterCountIs;
21+
using ast_matchers::pointerType;
22+
using ast_matchers::referenceType;
23+
using ast_matchers::returns;
24+
25+
bool hasSmartPointerClassShape(const CXXRecordDecl &RD, bool &HasGet,
26+
bool &HasValue) {
27+
// We may want to cache this search, but in current profiles it hasn't shown
28+
// up as a hot spot (possibly because there aren't many hits, relatively).
29+
bool HasArrow = false;
30+
bool HasStar = false;
31+
CanQualType StarReturnType, ArrowReturnType, GetReturnType, ValueReturnType;
32+
for (const auto *MD : RD.methods()) {
33+
// We only consider methods that are const and have zero parameters.
34+
// It may be that there is a non-const overload for the method, but
35+
// there should at least be a const overload as well.
36+
if (!MD->isConst() || MD->getNumParams() != 0)
37+
continue;
38+
switch (MD->getOverloadedOperator()) {
39+
case OO_Star:
40+
if (MD->getReturnType()->isReferenceType()) {
41+
HasStar = true;
42+
StarReturnType = MD->getReturnType()
43+
.getNonReferenceType()
44+
->getCanonicalTypeUnqualified();
45+
}
46+
break;
47+
case OO_Arrow:
48+
if (MD->getReturnType()->isPointerType()) {
49+
HasArrow = true;
50+
ArrowReturnType = MD->getReturnType()
51+
->getPointeeType()
52+
->getCanonicalTypeUnqualified();
53+
}
54+
break;
55+
case OO_None: {
56+
IdentifierInfo *II = MD->getIdentifier();
57+
if (II == nullptr)
58+
continue;
59+
if (II->isStr("get")) {
60+
if (MD->getReturnType()->isPointerType()) {
61+
HasGet = true;
62+
GetReturnType = MD->getReturnType()
63+
->getPointeeType()
64+
->getCanonicalTypeUnqualified();
65+
}
66+
} else if (II->isStr("value")) {
67+
if (MD->getReturnType()->isReferenceType()) {
68+
HasValue = true;
69+
ValueReturnType = MD->getReturnType()
70+
.getNonReferenceType()
71+
->getCanonicalTypeUnqualified();
72+
}
73+
}
74+
}
75+
default:
76+
break;
77+
}
78+
}
79+
80+
if (!HasStar || !HasArrow || StarReturnType != ArrowReturnType)
81+
return false;
82+
HasGet = HasGet && (GetReturnType == StarReturnType);
83+
HasValue = HasValue && (ValueReturnType == StarReturnType);
84+
return true;
85+
}
86+
87+
} // namespace
88+
} // namespace clang::dataflow
89+
90+
// AST_MATCHER macros create an "internal" namespace, so we put it in
91+
// its own anonymous namespace instead of in clang::dataflow.
92+
namespace {
93+
94+
AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithGet) {
95+
bool HasGet = false;
96+
bool HasValue = false;
97+
bool HasStarAndArrow =
98+
clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
99+
return HasStarAndArrow && HasGet;
100+
}
101+
102+
AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithValue) {
103+
bool HasGet = false;
104+
bool HasValue = false;
105+
bool HasStarAndArrow =
106+
clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
107+
return HasStarAndArrow && HasValue;
108+
}
109+
110+
AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithGetOrValue) {
111+
bool HasGet = false;
112+
bool HasValue = false;
113+
bool HasStarAndArrow =
114+
clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
115+
return HasStarAndArrow && (HasGet || HasValue);
116+
}
117+
118+
} // namespace
119+
120+
namespace clang::dataflow {
121+
122+
ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar() {
123+
return cxxOperatorCallExpr(
124+
hasOverloadedOperatorName("*"),
125+
callee(cxxMethodDecl(parameterCountIs(0), returns(referenceType()),
126+
ofClass(smartPointerClassWithGetOrValue()))));
127+
}
128+
129+
ast_matchers::StatementMatcher isSmartPointerLikeOperatorArrow() {
130+
return cxxOperatorCallExpr(
131+
hasOverloadedOperatorName("->"),
132+
callee(cxxMethodDecl(parameterCountIs(0), returns(pointerType()),
133+
ofClass(smartPointerClassWithGetOrValue()))));
134+
}
135+
ast_matchers::StatementMatcher isSmartPointerLikeValueMethodCall() {
136+
return cxxMemberCallExpr(callee(
137+
cxxMethodDecl(parameterCountIs(0), returns(referenceType()),
138+
hasName("value"), ofClass(smartPointerClassWithValue()))));
139+
}
140+
141+
ast_matchers::StatementMatcher isSmartPointerLikeGetMethodCall() {
142+
return cxxMemberCallExpr(callee(
143+
cxxMethodDecl(parameterCountIs(0), returns(pointerType()), hasName("get"),
144+
ofClass(smartPointerClassWithGet()))));
145+
}
146+
147+
} // namespace clang::dataflow

‎clang/unittests/Analysis/FlowSensitive/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests
2121
SignAnalysisTest.cpp
2222
SimplifyConstraintsTest.cpp
2323
SingleVarConstantPropagationTest.cpp
24+
SmartPointerAccessorCachingTest.cpp
2425
TestingSupport.cpp
2526
TestingSupportTest.cpp
2627
TransferBranchTest.cpp

0 commit comments

Comments
 (0)