Skip to content

Commit ef63166

Browse files
ilya-biryukovilya-biryukov
committed
[NFC] [ASTMatchers] Share code of forEachArgumentWithParamType with UnsafeBufferUsage
This changes exposes a low-level helper that is also used to implement `forEachArgumentWithParamType`, but can 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 will ensure the code is shared so that we do not repeat ourselves and any bugfixes or changes will be picked up by both implementations in the future.
1 parent 387f3e8 commit ef63166

File tree

5 files changed

+162
-151
lines changed

5 files changed

+162
-151
lines changed

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

Lines changed: 17 additions & 58 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"
@@ -5215,68 +5216,26 @@ AST_POLYMORPHIC_MATCHER_P2(forEachArgumentWithParamType,
52155216
// argument of the method which should not be matched against a parameter, so
52165217
// we skip over it here.
52175218
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-
}
52465219

5247-
unsigned ParamIndex = 0;
52485220
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) {
5221+
auto ProcessParamAndArg = [&](QualType ParamType, const Expr *Arg) {
52545222
BoundNodesTreeBuilder ArgMatches(*Builder);
5255-
if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder,
5256-
&ArgMatches)) {
5257-
BoundNodesTreeBuilder ParamMatches(ArgMatches);
5223+
if (!ArgMatcher.matches(*Arg, Finder, &ArgMatches))
5224+
return;
5225+
BoundNodesTreeBuilder ParamMatches(std::move(ArgMatches));
5226+
if (!ParamMatcher.matches(ParamType, Finder, &ParamMatches))
5227+
return;
5228+
Result.addMatch(ParamMatches);
5229+
Matched = true;
5230+
return;
5231+
};
5232+
if (auto *Call = llvm::dyn_cast<CallExpr>(&Node))
5233+
matchEachArgumentWithParamType(*Call, ProcessParamAndArg);
5234+
else if (auto *Construct = llvm::dyn_cast<CXXConstructExpr>(&Node))
5235+
matchEachArgumentWithParamType(*Construct, ProcessParamAndArg);
5236+
else
5237+
return false;
52585238

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-
}
52805239
*Builder = std::move(Result);
52815240
return Matched;
52825241
}

‎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: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
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); FD && FD->getNumParams() > ParamIndex)
81+
ParamType = FD->getParamDecl(ParamIndex)->getType();
82+
else
83+
continue;
84+
85+
OnParamAndArg(ParamType, Node.getArg(ArgIndex)->IgnoreParenCasts());
86+
}
87+
}
88+
89+
void matchEachArgumentWithParamType(
90+
const CallExpr &Node,
91+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
92+
OnParamAndArg) {
93+
matchEachArgumentWithParamTypeImpl(Node, OnParamAndArg);
94+
}
95+
96+
void matchEachArgumentWithParamType(
97+
const CXXConstructExpr &Node,
98+
llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)>
99+
OnParamAndArg) {
100+
matchEachArgumentWithParamTypeImpl(Node, OnParamAndArg);
101+
}
102+
103+
} // namespace ast_matchers
104+
105+
} // 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)