[clang] [Clang] Optimize -Wunsafe-buffer-usage. (PR #125492)
Ivana Ivanovska via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 7 04:53:07 PST 2025
================
@@ -186,106 +212,208 @@ class MatchDescendantVisitor : public DynamicRecursiveASTVisitor {
return DynamicRecursiveASTVisitor::TraverseStmt(Node);
}
+ void setASTContext(ASTContext &Context) { ActiveASTContext = &Context; }
+
+ void setHandler(const UnsafeBufferUsageHandler &NewHandler) {
+ Handler = &NewHandler;
+ }
+
private:
// Sets 'Matched' to true if 'Matcher' matches 'Node'
//
// Returns 'true' if traversal should continue after this function
// returns, i.e. if no match is found or 'Bind' is 'BK_All'.
template <typename T> bool match(const T &Node) {
- internal::BoundNodesTreeBuilder RecursiveBuilder(*Builder);
-
- if (Matcher->matches(DynTypedNode::create(Node), Finder,
- &RecursiveBuilder)) {
- ResultBindings.addMatch(RecursiveBuilder);
+ if (Matcher->matches(DynTypedNode::create(Node), *ActiveASTContext,
+ *Handler)) {
Matches = true;
- if (Bind != internal::ASTMatchFinder::BK_All)
+ if (!FindAll)
return false; // Abort as soon as a match is found.
}
return true;
}
- const internal::DynTypedMatcher *const Matcher;
- internal::ASTMatchFinder *const Finder;
- internal::BoundNodesTreeBuilder *const Builder;
- internal::BoundNodesTreeBuilder ResultBindings;
- const internal::ASTMatchFinder::BindKind Bind;
+ FastMatcher *const Matcher;
+ // When true, finds all matches. When false, finds the first match and stops.
+ const bool FindAll;
bool Matches;
bool ignoreUnevaluatedContext;
+ ASTContext *ActiveASTContext;
+ const UnsafeBufferUsageHandler *Handler;
};
// Because we're dealing with raw pointers, let's define what we mean by that.
-static auto hasPointerType() {
- return hasType(hasCanonicalType(pointerType()));
+static bool hasPointerType(const Expr &E) {
+ return isa<PointerType>(E.getType().getCanonicalType());
}
-static auto hasArrayType() { return hasType(hasCanonicalType(arrayType())); }
-
-AST_MATCHER_P(Stmt, forEachDescendantEvaluatedStmt, internal::Matcher<Stmt>,
- innerMatcher) {
- const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
-
- MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All,
- true);
- return Visitor.findMatch(DynTypedNode::create(Node));
+static bool hasArrayType(const Expr &E) {
+ return isa<ArrayType>(E.getType().getCanonicalType());
}
-AST_MATCHER_P(Stmt, forEachDescendantStmt, internal::Matcher<Stmt>,
- innerMatcher) {
- const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
+static void
+forEachDescendantEvaluatedStmt(const Stmt *S, ASTContext &Ctx,
+ const UnsafeBufferUsageHandler &Handler,
+ FastMatcher &Matcher) {
+ MatchDescendantVisitor Visitor(Matcher, /*FindAll=*/true,
+ /*ignoreUnevaluatedContext=*/true);
+ Visitor.setASTContext(Ctx);
+ Visitor.setHandler(Handler);
+ Visitor.findMatch(DynTypedNode::create(*S));
+}
- MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All,
- false);
- return Visitor.findMatch(DynTypedNode::create(Node));
+static void forEachDescendantStmt(const Stmt *S, ASTContext &Ctx,
+ const UnsafeBufferUsageHandler &Handler,
+ FastMatcher &Matcher) {
+ MatchDescendantVisitor Visitor(Matcher, /*FindAll=*/true,
+ /*ignoreUnevaluatedContext=*/false);
+ Visitor.setASTContext(Ctx);
+ Visitor.setHandler(Handler);
+ Visitor.findMatch(DynTypedNode::create(*S));
}
// Matches a `Stmt` node iff the node is in a safe-buffer opt-out region
-AST_MATCHER_P(Stmt, notInSafeBufferOptOut, const UnsafeBufferUsageHandler *,
- Handler) {
+static bool notInSafeBufferOptOut(const Stmt &Node,
+ const UnsafeBufferUsageHandler *Handler) {
return !Handler->isSafeBufferOptOut(Node.getBeginLoc());
}
-AST_MATCHER_P(Stmt, ignoreUnsafeBufferInContainer,
- const UnsafeBufferUsageHandler *, Handler) {
+static bool
+ignoreUnsafeBufferInContainer(const Stmt &Node,
+ const UnsafeBufferUsageHandler *Handler) {
return Handler->ignoreUnsafeBufferInContainer(Node.getBeginLoc());
}
-AST_MATCHER_P(Stmt, ignoreUnsafeLibcCall, const UnsafeBufferUsageHandler *,
- Handler) {
- if (Finder->getASTContext().getLangOpts().CPlusPlus)
+static bool ignoreUnsafeLibcCall(const ASTContext &Ctx, const Stmt &Node,
+ const UnsafeBufferUsageHandler *Handler) {
+ if (Ctx.getLangOpts().CPlusPlus)
return Handler->ignoreUnsafeBufferInLibcCall(Node.getBeginLoc());
return true; /* Only warn about libc calls for C++ */
}
-AST_MATCHER_P(CastExpr, castSubExpr, internal::Matcher<Expr>, innerMatcher) {
- return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
+// Finds any expression 'e' such that `OnResult`
+// matches 'e' and 'e' is in an Unspecified Lvalue Context.
+static void findStmtsInUnspecifiedLvalueContext(
+ const Stmt *S, const llvm::function_ref<void(const Expr *)> OnResult) {
+ if (const auto *CE = dyn_cast<ImplicitCastExpr>(S);
+ CE && CE->getCastKind() == CastKind::CK_LValueToRValue)
+ OnResult(CE->getSubExpr());
+ if (const auto *BO = dyn_cast<BinaryOperator>(S);
+ BO && BO->getOpcode() == BO_Assign)
+ OnResult(BO->getLHS());
}
-// Matches a `UnaryOperator` whose operator is pre-increment:
-AST_MATCHER(UnaryOperator, isPreInc) {
- return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+/// Note: Copied and modified from ASTMatchers.
+/// Matches all arguments and their respective types for a \c CallExpr or
+/// \c CXXConstructExpr. It is very similar to \c forEachArgumentWithParam but
+/// it works on calls through function pointers as well.
+///
+/// The difference 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 *FD = CE->getDirectCallee();
+ if (FD) {
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(FD);
+ 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> {
+ // This test is cheaper compared to the big matcher in the next if.
+ // Therefore, please keep this order.
+ if (FProto && FProto->getNumParams() > ParamIndex) {
+ return FProto->getParamType(ParamIndex);
+ }
+ if (const auto *E = dyn_cast<Expr>(&Node)) {
----------------
ivanaivanovska wrote:
I removed the `CXXConstructExpr` path as the code could not be reached.
https://github.com/llvm/llvm-project/pull/125492
More information about the cfe-commits
mailing list