Skip to content

Commit da69eb7

Browse files
ilya-biryukovilya-biryukov
authored
[NFC] [ASTMatchers] Share code of forEachArgumentWithParamType with UnsafeBufferUsage (#132387)
This changes exposes a low-level helper that is used to implement `forEachArgumentWithParamType` but can also be used without matchers, e.g. if performance is a concern. Commit f5ee105 introduced a copy of the implementation of the `forEachArgumentWithParamType` matcher that was needed for optimizing performance of `-Wunsafe-buffer-usage`. This change shares the code between the two so that we do not repeat ourselves and any bugfixes or changes will be picked up by both implementations in the future.
1 parent d02786e commit da69eb7

File tree

5 files changed

+163
-156
lines changed

5 files changed

+163
-156
lines changed

‎clang/include/clang/ASTMatchers/ASTMatchers.h‎

Lines changed: 17 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
#include "clang/AST/TypeLoc.h"
7272
#include "clang/ASTMatchers/ASTMatchersInternal.h"
7373
#include "clang/ASTMatchers/ASTMatchersMacros.h"
74+
#include "clang/ASTMatchers/LowLevelHelpers.h"
7475
#include "clang/Basic/AttrKinds.h"
7576
#include "clang/Basic/ExceptionSpecificationType.h"
7677
#include "clang/Basic/FileManager.h"
@@ -5211,72 +5212,25 @@ AST_POLYMORPHIC_MATCHER_P2(forEachArgumentWithParamType,
52115212
internal::Matcher<Expr>, ArgMatcher,
52125213
internal::Matcher<QualType>, ParamMatcher) {
52135214
BoundNodesTreeBuilder Result;
5214-
// The first argument of an overloaded member operator is the implicit object
5215-
// argument of the method which should not be matched against a parameter, so
5216-
// we skip over it here.
5217-
BoundNodesTreeBuilder Matches;
5218-
unsigned ArgIndex =
5219-
cxxOperatorCallExpr(
5220-
callee(cxxMethodDecl(unless(isExplicitObjectMemberFunction()))))
5221-
.matches(Node, Finder, &Matches)
5222-
? 1
5223-
: 0;
5224-
const FunctionProtoType *FProto = nullptr;
5225-
5226-
if (const auto *Call = dyn_cast<CallExpr>(&Node)) {
5227-
if (const auto *Value =
5228-
dyn_cast_or_null<ValueDecl>(Call->getCalleeDecl())) {
5229-
QualType QT = Value->getType().getCanonicalType();
5230-
5231-
// This does not necessarily lead to a `FunctionProtoType`,
5232-
// e.g. K&R functions do not have a function prototype.
5233-
if (QT->isFunctionPointerType())
5234-
FProto = QT->getPointeeType()->getAs<FunctionProtoType>();
5235-
5236-
if (QT->isMemberFunctionPointerType()) {
5237-
const auto *MP = QT->getAs<MemberPointerType>();
5238-
assert(MP && "Must be member-pointer if its a memberfunctionpointer");
5239-
FProto = MP->getPointeeType()->getAs<FunctionProtoType>();
5240-
assert(FProto &&
5241-
"The call must have happened through a member function "
5242-
"pointer");
5243-
}
5244-
}
5245-
}
5246-
5247-
unsigned ParamIndex = 0;
52485215
bool Matched = false;
5249-
unsigned NumArgs = Node.getNumArgs();
5250-
if (FProto && FProto->isVariadic())
5251-
NumArgs = std::min(NumArgs, FProto->getNumParams());
5252-
5253-
for (; ArgIndex < NumArgs; ++ArgIndex, ++ParamIndex) {
5216+
auto ProcessParamAndArg = [&](QualType ParamType, const Expr *Arg) {
52545217
BoundNodesTreeBuilder ArgMatches(*Builder);
5255-
if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder,
5256-
&ArgMatches)) {
5257-
BoundNodesTreeBuilder ParamMatches(ArgMatches);
5218+
if (!ArgMatcher.matches(*Arg, Finder, &ArgMatches))
5219+
return;
5220+
BoundNodesTreeBuilder ParamMatches(std::move(ArgMatches));
5221+
if (!ParamMatcher.matches(ParamType, Finder, &ParamMatches))
5222+
return;
5223+
Result.addMatch(ParamMatches);
5224+
Matched = true;
5225+
return;
5226+
};
5227+
if (auto *Call = llvm::dyn_cast<CallExpr>(&Node))
5228+
matchEachArgumentWithParamType(*Call, ProcessParamAndArg);
5229+
else if (auto *Construct = llvm::dyn_cast<CXXConstructExpr>(&Node))
5230+
matchEachArgumentWithParamType(*Construct, ProcessParamAndArg);
5231+
else
5232+
llvm_unreachable("expected CallExpr or CXXConstructExpr");
52585233

5259-
// This test is cheaper compared to the big matcher in the next if.
5260-
// Therefore, please keep this order.
5261-
if (FProto && FProto->getNumParams() > ParamIndex) {
5262-
QualType ParamType = FProto->getParamType(ParamIndex);
5263-
if (ParamMatcher.matches(ParamType, Finder, &ParamMatches)) {
5264-
Result.addMatch(ParamMatches);
5265-
Matched = true;
5266-
continue;
5267-
}
5268-
}
5269-
if (expr(anyOf(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
5270-
hasParameter(ParamIndex, hasType(ParamMatcher))))),
5271-
callExpr(callee(functionDecl(
5272-
hasParameter(ParamIndex, hasType(ParamMatcher)))))))
5273-
.matches(Node, Finder, &ParamMatches)) {
5274-
Result.addMatch(ParamMatches);
5275-
Matched = true;
5276-
continue;
5277-
}
5278-
}
5279-
}
52805234
*Builder = std::move(Result);
52815235
return Matched;
52825236
}

‎clang/include/clang/ASTMatchers/LowLevelHelpers.h‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//===- LowLevelHelpers.h - helpers with pure AST interface ---- *- 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+
// Collects a number of helpers that are used by matchers, but can be reused
9+
// outside of them, e.g. when corresponding matchers cannot be used due to
10+
// performance constraints.
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef LLVM_CLANG_ASTMATCHERS_LOWLEVELHELPERS_H
14+
#define LLVM_CLANG_ASTMATCHERS_LOWLEVELHELPERS_H
15+
16+
#include "clang/AST/Expr.h"
17+
#include "clang/AST/ExprCXX.h"
18+
#include "clang/AST/Type.h"
19+
#include "llvm/ADT/STLFunctionalExtras.h"
20+
21+
namespace clang {
22+
namespace ast_matchers {
23+
24+
void matchEachArgumentWithParamType(
25+
const CallExpr &Node,
26+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
27+
OnParamAndArg);
28+
29+
void matchEachArgumentWithParamType(
30+
const CXXConstructExpr &Node,
31+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
32+
OnParamAndArg);
33+
34+
} // namespace ast_matchers
35+
} // namespace clang
36+
37+
#endif

‎clang/lib/ASTMatchers/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ add_clang_library(clangASTMatchers
99
ASTMatchFinder.cpp
1010
ASTMatchersInternal.cpp
1111
GtestMatchers.cpp
12+
LowLevelHelpers.cpp
1213

1314
LINK_LIBS
1415
clangAST

‎clang/lib/ASTMatchers/LowLevelHelpers.cpp‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
//===- LowLevelHelpers.cpp -------------------------------------*- 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+
#include "clang/ASTMatchers/LowLevelHelpers.h"
10+
#include "clang/AST/Decl.h"
11+
#include "clang/AST/DeclCXX.h"
12+
#include "clang/AST/Expr.h"
13+
#include "clang/AST/ExprCXX.h"
14+
#include <type_traits>
15+
16+
namespace clang {
17+
namespace ast_matchers {
18+
19+
static const FunctionDecl *getCallee(const CXXConstructExpr &D) {
20+
return D.getConstructor();
21+
}
22+
static const FunctionDecl *getCallee(const CallExpr &D) {
23+
return D.getDirectCallee();
24+
}
25+
26+
template <class ExprNode>
27+
static void matchEachArgumentWithParamTypeImpl(
28+
const ExprNode &Node,
29+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
30+
OnParamAndArg) {
31+
static_assert(std::is_same_v<CallExpr, ExprNode> ||
32+
std::is_same_v<CXXConstructExpr, ExprNode>);
33+
// The first argument of an overloaded member operator is the implicit object
34+
// argument of the method which should not be matched against a parameter, so
35+
// we skip over it here.
36+
unsigned ArgIndex = 0;
37+
if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(&Node)) {
38+
const auto *MD = dyn_cast_or_null<CXXMethodDecl>(CE->getDirectCallee());
39+
if (MD && !MD->isExplicitObjectMemberFunction()) {
40+
// This is an overloaded operator call.
41+
// We need to skip the first argument, which is the implicit object
42+
// argument of the method which should not be matched against a
43+
// parameter.
44+
++ArgIndex;
45+
}
46+
}
47+
48+
const FunctionProtoType *FProto = nullptr;
49+
50+
if (const auto *Call = dyn_cast<CallExpr>(&Node)) {
51+
if (const auto *Value =
52+
dyn_cast_or_null<ValueDecl>(Call->getCalleeDecl())) {
53+
QualType QT = Value->getType().getCanonicalType();
54+
55+
// This does not necessarily lead to a `FunctionProtoType`,
56+
// e.g. K&R functions do not have a function prototype.
57+
if (QT->isFunctionPointerType())
58+
FProto = QT->getPointeeType()->getAs<FunctionProtoType>();
59+
60+
if (QT->isMemberFunctionPointerType()) {
61+
const auto *MP = QT->getAs<MemberPointerType>();
62+
assert(MP && "Must be member-pointer if its a memberfunctionpointer");
63+
FProto = MP->getPointeeType()->getAs<FunctionProtoType>();
64+
assert(FProto &&
65+
"The call must have happened through a member function "
66+
"pointer");
67+
}
68+
}
69+
}
70+
71+
unsigned ParamIndex = 0;
72+
unsigned NumArgs = Node.getNumArgs();
73+
if (FProto && FProto->isVariadic())
74+
NumArgs = std::min(NumArgs, FProto->getNumParams());
75+
76+
for (; ArgIndex < NumArgs; ++ArgIndex, ++ParamIndex) {
77+
QualType ParamType;
78+
if (FProto && FProto->getNumParams() > ParamIndex)
79+
ParamType = FProto->getParamType(ParamIndex);
80+
else if (const FunctionDecl *FD = getCallee(Node);
81+
FD && FD->getNumParams() > ParamIndex)
82+
ParamType = FD->getParamDecl(ParamIndex)->getType();
83+
else
84+
continue;
85+
86+
OnParamAndArg(ParamType, Node.getArg(ArgIndex)->IgnoreParenCasts());
87+
}
88+
}
89+
90+
void matchEachArgumentWithParamType(
91+
const CallExpr &Node,
92+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
93+
OnParamAndArg) {
94+
matchEachArgumentWithParamTypeImpl(Node, OnParamAndArg);
95+
}
96+
97+
void matchEachArgumentWithParamType(
98+
const CXXConstructExpr &Node,
99+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
100+
OnParamAndArg) {
101+
matchEachArgumentWithParamTypeImpl(Node, OnParamAndArg);
102+
}
103+
104+
} // namespace ast_matchers
105+
106+
} // namespace clang

‎clang/lib/Analysis/UnsafeBufferUsage.cpp‎

Lines changed: 2 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "clang/AST/Stmt.h"
2121
#include "clang/AST/StmtVisitor.h"
2222
#include "clang/AST/Type.h"
23+
#include "clang/ASTMatchers/LowLevelHelpers.h"
2324
#include "clang/Basic/SourceLocation.h"
2425
#include "clang/Lex/Lexer.h"
2526
#include "clang/Lex/Preprocessor.h"
@@ -300,98 +301,6 @@ static void findStmtsInUnspecifiedLvalueContext(
300301
OnResult(BO->getLHS());
301302
}
302303

303-
/// Note: Copied and modified from ASTMatchers.
304-
/// Matches all arguments and their respective types for a \c CallExpr.
305-
/// It is very similar to \c forEachArgumentWithParam but
306-
/// it works on calls through function pointers as well.
307-
///
308-
/// The difference is, that function pointers do not provide access to a
309-
/// \c ParmVarDecl, but only the \c QualType for each argument.
310-
///
311-
/// Given
312-
/// \code
313-
/// void f(int i);
314-
/// int y;
315-
/// f(y);
316-
/// void (*f_ptr)(int) = f;
317-
/// f_ptr(y);
318-
/// \endcode
319-
/// callExpr(
320-
/// forEachArgumentWithParamType(
321-
/// declRefExpr(to(varDecl(hasName("y")))),
322-
/// qualType(isInteger()).bind("type)
323-
/// ))
324-
/// matches f(y) and f_ptr(y)
325-
/// with declRefExpr(...)
326-
/// matching int y
327-
/// and qualType(...)
328-
/// matching int
329-
static void forEachArgumentWithParamType(
330-
const CallExpr &Node,
331-
const llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
332-
OnParamAndArg) {
333-
// The first argument of an overloaded member operator is the implicit object
334-
// argument of the method which should not be matched against a parameter, so
335-
// we skip over it here.
336-
unsigned ArgIndex = 0;
337-
if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(&Node)) {
338-
const auto *MD = dyn_cast_or_null<CXXMethodDecl>(CE->getDirectCallee());
339-
if (MD && !MD->isExplicitObjectMemberFunction()) {
340-
// This is an overloaded operator call.
341-
// We need to skip the first argument, which is the implicit object
342-
// argument of the method which should not be matched against a
343-
// parameter.
344-
++ArgIndex;
345-
}
346-
}
347-
348-
const FunctionProtoType *FProto = nullptr;
349-
350-
if (const auto *Call = dyn_cast<CallExpr>(&Node)) {
351-
if (const auto *Value =
352-
dyn_cast_or_null<ValueDecl>(Call->getCalleeDecl())) {
353-
QualType QT = Value->getType().getCanonicalType();
354-
355-
// This does not necessarily lead to a `FunctionProtoType`,
356-
// e.g. K&R functions do not have a function prototype.
357-
if (QT->isFunctionPointerType())
358-
FProto = QT->getPointeeType()->getAs<FunctionProtoType>();
359-
360-
if (QT->isMemberFunctionPointerType()) {
361-
const auto *MP = QT->getAs<MemberPointerType>();
362-
assert(MP && "Must be member-pointer if its a memberfunctionpointer");
363-
FProto = MP->getPointeeType()->getAs<FunctionProtoType>();
364-
assert(FProto &&
365-
"The call must have happened through a member function "
366-
"pointer");
367-
}
368-
}
369-
}
370-
371-
unsigned ParamIndex = 0;
372-
unsigned NumArgs = Node.getNumArgs();
373-
if (FProto && FProto->isVariadic())
374-
NumArgs = std::min(NumArgs, FProto->getNumParams());
375-
376-
const auto GetParamType =
377-
[&FProto, &Node](unsigned int ParamIndex) -> std::optional<QualType> {
378-
if (FProto && FProto->getNumParams() > ParamIndex) {
379-
return FProto->getParamType(ParamIndex);
380-
}
381-
const auto *FD = Node.getDirectCallee();
382-
if (FD && FD->getNumParams() > ParamIndex) {
383-
return FD->getParamDecl(ParamIndex)->getType();
384-
}
385-
return std::nullopt;
386-
};
387-
388-
for (; ArgIndex < NumArgs; ++ArgIndex, ++ParamIndex) {
389-
auto ParamType = GetParamType(ParamIndex);
390-
if (ParamType)
391-
OnParamAndArg(*ParamType, Node.getArg(ArgIndex)->IgnoreParenCasts());
392-
}
393-
}
394-
395304
// Finds any expression `e` such that `InnerMatcher` matches `e` and
396305
// `e` is in an Unspecified Pointer Context (UPC).
397306
static void findStmtsInUnspecifiedPointerContext(
@@ -408,7 +317,7 @@ static void findStmtsInUnspecifiedPointerContext(
408317
if (const auto *FnDecl = CE->getDirectCallee();
409318
FnDecl && FnDecl->hasAttr<UnsafeBufferUsageAttr>())
410319
return;
411-
forEachArgumentWithParamType(
320+
ast_matchers::matchEachArgumentWithParamType(
412321
*CE, [&InnerMatcher](QualType Type, const Expr *Arg) {
413322
if (Type->isAnyPointerType())
414323
InnerMatcher(Arg);

0 commit comments

Comments
 (0)