Author: Ilya Biryukov Date: 2025-04-04T14:35:15+02:00 New Revision: da69eb75cbc634a56886e94de3e546c63c17567e
URL: https://github.com/llvm/llvm-project/commit/da69eb75cbc634a56886e94de3e546c63c17567e DIFF: https://github.com/llvm/llvm-project/commit/da69eb75cbc634a56886e94de3e546c63c17567e.diff LOG: [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 f5ee10538b68835112323c241ca7db67ca78bf62 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. Added: clang/include/clang/ASTMatchers/LowLevelHelpers.h clang/lib/ASTMatchers/LowLevelHelpers.cpp Modified: clang/include/clang/ASTMatchers/ASTMatchers.h clang/lib/ASTMatchers/CMakeLists.txt clang/lib/Analysis/UnsafeBufferUsage.cpp Removed: ################################################################################ diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h index 738617759eb29..e6b684b24b080 100644 --- a/clang/include/clang/ASTMatchers/ASTMatchers.h +++ b/clang/include/clang/ASTMatchers/ASTMatchers.h @@ -71,6 +71,7 @@ #include "clang/AST/TypeLoc.h" #include "clang/ASTMatchers/ASTMatchersInternal.h" #include "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/ASTMatchers/LowLevelHelpers.h" #include "clang/Basic/AttrKinds.h" #include "clang/Basic/ExceptionSpecificationType.h" #include "clang/Basic/FileManager.h" @@ -5211,72 +5212,25 @@ AST_POLYMORPHIC_MATCHER_P2(forEachArgumentWithParamType, internal::Matcher<Expr>, ArgMatcher, internal::Matcher<QualType>, ParamMatcher) { BoundNodesTreeBuilder Result; - // The first argument of an overloaded member operator is the implicit object - // argument of the method which should not be matched against a parameter, so - // we skip over it here. - BoundNodesTreeBuilder Matches; - unsigned ArgIndex = - cxxOperatorCallExpr( - callee(cxxMethodDecl(unless(isExplicitObjectMemberFunction())))) - .matches(Node, Finder, &Matches) - ? 1 - : 0; - const FunctionProtoType *FProto = nullptr; - - if (const auto *Call = dyn_cast<CallExpr>(&Node)) { - if (const auto *Value = - dyn_cast_or_null<ValueDecl>(Call->getCalleeDecl())) { - QualType QT = Value->getType().getCanonicalType(); - - // This does not necessarily lead to a `FunctionProtoType`, - // e.g. K&R functions do not have a function prototype. - if (QT->isFunctionPointerType()) - FProto = QT->getPointeeType()->getAs<FunctionProtoType>(); - - if (QT->isMemberFunctionPointerType()) { - const auto *MP = QT->getAs<MemberPointerType>(); - assert(MP && "Must be member-pointer if its a memberfunctionpointer"); - FProto = MP->getPointeeType()->getAs<FunctionProtoType>(); - assert(FProto && - "The call must have happened through a member function " - "pointer"); - } - } - } - - unsigned ParamIndex = 0; bool Matched = false; - unsigned NumArgs = Node.getNumArgs(); - if (FProto && FProto->isVariadic()) - NumArgs = std::min(NumArgs, FProto->getNumParams()); - - for (; ArgIndex < NumArgs; ++ArgIndex, ++ParamIndex) { + auto ProcessParamAndArg = [&](QualType ParamType, const Expr *Arg) { BoundNodesTreeBuilder ArgMatches(*Builder); - if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder, - &ArgMatches)) { - BoundNodesTreeBuilder ParamMatches(ArgMatches); + if (!ArgMatcher.matches(*Arg, Finder, &ArgMatches)) + return; + BoundNodesTreeBuilder ParamMatches(std::move(ArgMatches)); + if (!ParamMatcher.matches(ParamType, Finder, &ParamMatches)) + return; + Result.addMatch(ParamMatches); + Matched = true; + return; + }; + if (auto *Call = llvm::dyn_cast<CallExpr>(&Node)) + matchEachArgumentWithParamType(*Call, ProcessParamAndArg); + else if (auto *Construct = llvm::dyn_cast<CXXConstructExpr>(&Node)) + matchEachArgumentWithParamType(*Construct, ProcessParamAndArg); + else + llvm_unreachable("expected CallExpr or CXXConstructExpr"); - // This test is cheaper compared to the big matcher in the next if. - // Therefore, please keep this order. - if (FProto && FProto->getNumParams() > ParamIndex) { - QualType ParamType = FProto->getParamType(ParamIndex); - if (ParamMatcher.matches(ParamType, Finder, &ParamMatches)) { - Result.addMatch(ParamMatches); - Matched = true; - continue; - } - } - if (expr(anyOf(cxxConstructExpr(hasDeclaration(cxxConstructorDecl( - hasParameter(ParamIndex, hasType(ParamMatcher))))), - callExpr(callee(functionDecl( - hasParameter(ParamIndex, hasType(ParamMatcher))))))) - .matches(Node, Finder, &ParamMatches)) { - Result.addMatch(ParamMatches); - Matched = true; - continue; - } - } - } *Builder = std::move(Result); return Matched; } diff --git a/clang/include/clang/ASTMatchers/LowLevelHelpers.h b/clang/include/clang/ASTMatchers/LowLevelHelpers.h new file mode 100644 index 0000000000000..ad1fffb5e5e01 --- /dev/null +++ b/clang/include/clang/ASTMatchers/LowLevelHelpers.h @@ -0,0 +1,37 @@ +//===- LowLevelHelpers.h - helpers with pure AST interface ---- *- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// Collects a number of helpers that are used by matchers, but can be reused +// outside of them, e.g. when corresponding matchers cannot be used due to +// performance constraints. +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ASTMATCHERS_LOWLEVELHELPERS_H +#define LLVM_CLANG_ASTMATCHERS_LOWLEVELHELPERS_H + +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Type.h" +#include "llvm/ADT/STLFunctionalExtras.h" + +namespace clang { +namespace ast_matchers { + +void matchEachArgumentWithParamType( + const CallExpr &Node, + llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)> + OnParamAndArg); + +void matchEachArgumentWithParamType( + const CXXConstructExpr &Node, + llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)> + OnParamAndArg); + +} // namespace ast_matchers +} // namespace clang + +#endif diff --git a/clang/lib/ASTMatchers/CMakeLists.txt b/clang/lib/ASTMatchers/CMakeLists.txt index 30303c1e39a00..7769fd656ac06 100644 --- a/clang/lib/ASTMatchers/CMakeLists.txt +++ b/clang/lib/ASTMatchers/CMakeLists.txt @@ -9,6 +9,7 @@ add_clang_library(clangASTMatchers ASTMatchFinder.cpp ASTMatchersInternal.cpp GtestMatchers.cpp + LowLevelHelpers.cpp LINK_LIBS clangAST diff --git a/clang/lib/ASTMatchers/LowLevelHelpers.cpp b/clang/lib/ASTMatchers/LowLevelHelpers.cpp new file mode 100644 index 0000000000000..eb2604c6252dd --- /dev/null +++ b/clang/lib/ASTMatchers/LowLevelHelpers.cpp @@ -0,0 +1,106 @@ +//===- LowLevelHelpers.cpp -------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/ASTMatchers/LowLevelHelpers.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include <type_traits> + +namespace clang { +namespace ast_matchers { + +static const FunctionDecl *getCallee(const CXXConstructExpr &D) { + return D.getConstructor(); +} +static const FunctionDecl *getCallee(const CallExpr &D) { + return D.getDirectCallee(); +} + +template <class ExprNode> +static void matchEachArgumentWithParamTypeImpl( + const ExprNode &Node, + llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)> + OnParamAndArg) { + static_assert(std::is_same_v<CallExpr, ExprNode> || + std::is_same_v<CXXConstructExpr, ExprNode>); + // The first argument of an overloaded member operator is the implicit object + // argument of the method which should not be matched against a parameter, so + // we skip over it here. + unsigned ArgIndex = 0; + if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(&Node)) { + const auto *MD = dyn_cast_or_null<CXXMethodDecl>(CE->getDirectCallee()); + if (MD && !MD->isExplicitObjectMemberFunction()) { + // This is an overloaded operator call. + // We need to skip the first argument, which is the implicit object + // argument of the method which should not be matched against a + // parameter. + ++ArgIndex; + } + } + + const FunctionProtoType *FProto = nullptr; + + if (const auto *Call = dyn_cast<CallExpr>(&Node)) { + if (const auto *Value = + dyn_cast_or_null<ValueDecl>(Call->getCalleeDecl())) { + QualType QT = Value->getType().getCanonicalType(); + + // This does not necessarily lead to a `FunctionProtoType`, + // e.g. K&R functions do not have a function prototype. + if (QT->isFunctionPointerType()) + FProto = QT->getPointeeType()->getAs<FunctionProtoType>(); + + if (QT->isMemberFunctionPointerType()) { + const auto *MP = QT->getAs<MemberPointerType>(); + assert(MP && "Must be member-pointer if its a memberfunctionpointer"); + FProto = MP->getPointeeType()->getAs<FunctionProtoType>(); + assert(FProto && + "The call must have happened through a member function " + "pointer"); + } + } + } + + unsigned ParamIndex = 0; + unsigned NumArgs = Node.getNumArgs(); + if (FProto && FProto->isVariadic()) + NumArgs = std::min(NumArgs, FProto->getNumParams()); + + for (; ArgIndex < NumArgs; ++ArgIndex, ++ParamIndex) { + QualType ParamType; + if (FProto && FProto->getNumParams() > ParamIndex) + ParamType = FProto->getParamType(ParamIndex); + else if (const FunctionDecl *FD = getCallee(Node); + FD && FD->getNumParams() > ParamIndex) + ParamType = FD->getParamDecl(ParamIndex)->getType(); + else + continue; + + OnParamAndArg(ParamType, Node.getArg(ArgIndex)->IgnoreParenCasts()); + } +} + +void matchEachArgumentWithParamType( + const CallExpr &Node, + llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)> + OnParamAndArg) { + matchEachArgumentWithParamTypeImpl(Node, OnParamAndArg); +} + +void matchEachArgumentWithParamType( + const CXXConstructExpr &Node, + llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)> + OnParamAndArg) { + matchEachArgumentWithParamTypeImpl(Node, OnParamAndArg); +} + +} // namespace ast_matchers + +} // namespace clang diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 776cbf6196b60..fbe753de9ef1f 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -20,6 +20,7 @@ #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" #include "clang/AST/Type.h" +#include "clang/ASTMatchers/LowLevelHelpers.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Preprocessor.h" @@ -300,98 +301,6 @@ static void findStmtsInUnspecifiedLvalueContext( OnResult(BO->getLHS()); } -/// Note: Copied and modified from ASTMatchers. -/// Matches all arguments and their respective types for a \c CallExpr. -/// It is very similar to \c forEachArgumentWithParam but -/// it works on calls through function pointers as well. -/// -/// The diff erence is, that function pointers do not provide access to a -/// \c ParmVarDecl, but only the \c QualType for each argument. -/// -/// Given -/// \code -/// void f(int i); -/// int y; -/// f(y); -/// void (*f_ptr)(int) = f; -/// f_ptr(y); -/// \endcode -/// callExpr( -/// forEachArgumentWithParamType( -/// declRefExpr(to(varDecl(hasName("y")))), -/// qualType(isInteger()).bind("type) -/// )) -/// matches f(y) and f_ptr(y) -/// with declRefExpr(...) -/// matching int y -/// and qualType(...) -/// matching int -static void forEachArgumentWithParamType( - const CallExpr &Node, - const llvm::function_ref<void(QualType /*Param*/, const Expr * /*Arg*/)> - OnParamAndArg) { - // The first argument of an overloaded member operator is the implicit object - // argument of the method which should not be matched against a parameter, so - // we skip over it here. - unsigned ArgIndex = 0; - if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(&Node)) { - const auto *MD = dyn_cast_or_null<CXXMethodDecl>(CE->getDirectCallee()); - if (MD && !MD->isExplicitObjectMemberFunction()) { - // This is an overloaded operator call. - // We need to skip the first argument, which is the implicit object - // argument of the method which should not be matched against a - // parameter. - ++ArgIndex; - } - } - - const FunctionProtoType *FProto = nullptr; - - if (const auto *Call = dyn_cast<CallExpr>(&Node)) { - if (const auto *Value = - dyn_cast_or_null<ValueDecl>(Call->getCalleeDecl())) { - QualType QT = Value->getType().getCanonicalType(); - - // This does not necessarily lead to a `FunctionProtoType`, - // e.g. K&R functions do not have a function prototype. - if (QT->isFunctionPointerType()) - FProto = QT->getPointeeType()->getAs<FunctionProtoType>(); - - if (QT->isMemberFunctionPointerType()) { - const auto *MP = QT->getAs<MemberPointerType>(); - assert(MP && "Must be member-pointer if its a memberfunctionpointer"); - FProto = MP->getPointeeType()->getAs<FunctionProtoType>(); - assert(FProto && - "The call must have happened through a member function " - "pointer"); - } - } - } - - unsigned ParamIndex = 0; - unsigned NumArgs = Node.getNumArgs(); - if (FProto && FProto->isVariadic()) - NumArgs = std::min(NumArgs, FProto->getNumParams()); - - const auto GetParamType = - [&FProto, &Node](unsigned int ParamIndex) -> std::optional<QualType> { - if (FProto && FProto->getNumParams() > ParamIndex) { - return FProto->getParamType(ParamIndex); - } - const auto *FD = Node.getDirectCallee(); - if (FD && FD->getNumParams() > ParamIndex) { - return FD->getParamDecl(ParamIndex)->getType(); - } - return std::nullopt; - }; - - for (; ArgIndex < NumArgs; ++ArgIndex, ++ParamIndex) { - auto ParamType = GetParamType(ParamIndex); - if (ParamType) - OnParamAndArg(*ParamType, Node.getArg(ArgIndex)->IgnoreParenCasts()); - } -} - // Finds any expression `e` such that `InnerMatcher` matches `e` and // `e` is in an Unspecified Pointer Context (UPC). static void findStmtsInUnspecifiedPointerContext( @@ -408,7 +317,7 @@ static void findStmtsInUnspecifiedPointerContext( if (const auto *FnDecl = CE->getDirectCallee(); FnDecl && FnDecl->hasAttr<UnsafeBufferUsageAttr>()) return; - forEachArgumentWithParamType( + ast_matchers::matchEachArgumentWithParamType( *CE, [&InnerMatcher](QualType Type, const Expr *Arg) { if (Type->isAnyPointerType()) InnerMatcher(Arg); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
