Author: Congcong Cai Date: 2024-06-15T06:21:41+08:00 New Revision: 3d25e5ae996d709cf1496f0c6823f6a82f51831e
URL: https://github.com/llvm/llvm-project/commit/3d25e5ae996d709cf1496f0c6823f6a82f51831e DIFF: https://github.com/llvm/llvm-project/commit/3d25e5ae996d709cf1496f0c6823f6a82f51831e.diff LOG: [clang-tidy] avoid false positive when overload for bugprone-return-const-ref-from-parameter (#95434) Fixes: #90274 Added: Modified: clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp index cacba38b4a5aa..adb26ade955c5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "ReturnConstRefFromParameterCheck.h" -#include "../utils/Matchers.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -18,20 +17,82 @@ namespace clang::tidy::bugprone { void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( returnStmt( - hasReturnValue(declRefExpr(to(parmVarDecl(hasType(hasCanonicalType( - qualType(matchers::isReferenceToConst()).bind("type"))))))), - hasAncestor(functionDecl(hasReturnTypeLoc( - loc(qualType(hasCanonicalType(equalsBoundNode("type")))))))) + hasReturnValue(declRefExpr( + to(parmVarDecl(hasType(hasCanonicalType( + qualType(lValueReferenceType(pointee( + qualType(isConstQualified())))) + .bind("type")))) + .bind("param")))), + hasAncestor( + functionDecl(hasReturnTypeLoc(loc(qualType( + hasCanonicalType(equalsBoundNode("type")))))) + .bind("func"))) .bind("ret"), this); } +static bool isSameTypeIgnoringConst(QualType A, QualType B) { + return A.getCanonicalType().withConst() == B.getCanonicalType().withConst(); +} + +static bool isSameTypeIgnoringConstRef(QualType A, QualType B) { + return isSameTypeIgnoringConst(A.getCanonicalType().getNonReferenceType(), + B.getCanonicalType().getNonReferenceType()); +} + +static bool hasSameParameterTypes(const FunctionDecl &FD, const FunctionDecl &O, + const ParmVarDecl &PD) { + if (FD.getNumParams() != O.getNumParams()) + return false; + for (unsigned I = 0, E = FD.getNumParams(); I < E; ++I) { + const ParmVarDecl *DPD = FD.getParamDecl(I); + const QualType OPT = O.getParamDecl(I)->getType(); + if (DPD == &PD) { + if (!llvm::isa<RValueReferenceType>(OPT) || + !isSameTypeIgnoringConstRef(DPD->getType(), OPT)) + return false; + } else { + if (!isSameTypeIgnoringConst(DPD->getType(), OPT)) + return false; + } + } + return true; +} + +static const Decl *findRVRefOverload(const FunctionDecl &FD, + const ParmVarDecl &PD) { + // Actually it would be better to do lookup in caller site. + // But in most of cases, overloads of LVRef and RVRef will appear together. + // FIXME: + // 1. overload in anonymous namespace + // 2. forward reference + DeclContext::lookup_result LookupResult = + FD.getParent()->lookup(FD.getNameInfo().getName()); + if (LookupResult.isSingleResult()) { + return nullptr; + } + for (const Decl *Overload : LookupResult) { + if (Overload == &FD) + continue; + if (const auto *O = dyn_cast<FunctionDecl>(Overload)) + if (hasSameParameterTypes(FD, *O, PD)) + return O; + } + return nullptr; +} + void ReturnConstRefFromParameterCheck::check( const MatchFinder::MatchResult &Result) { + const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func"); + const auto *PD = Result.Nodes.getNodeAs<ParmVarDecl>("param"); const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret"); const SourceRange Range = R->getRetValue()->getSourceRange(); if (Range.isInvalid()) return; + + if (findRVRefOverload(*FD, *PD) != nullptr) + return; + diag(Range.getBegin(), "returning a constant reference parameter may cause use-after-free " "when the parameter is constructed from a temporary") diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp index ca41bdf74a107..d13c127da7c2a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp @@ -143,3 +143,37 @@ void instantiate(const int ¶m, const float ¶mf, int &mut_param, float &m } } // namespace valid + +namespace overload { + +int const &overload_base(int const &a) { return a; } +int const &overload_base(int &&a); + +int const &overload_ret_type(int const &a) { return a; } +void overload_ret_type(int &&a); + +int const &overload_params1(int p1, int const &a) { return a; } +int const & overload_params1(int p1, int &&a); + +int const &overload_params2(int p1, int const &a, int p2) { return a; } +int const &overload_params2(int p1, int &&a, int p2); + +int const &overload_params3(T p1, int const &a, int p2) { return a; } +int const &overload_params3(int p1, int &&a, T p2); + +int const &overload_params_const(int p1, int const &a, int const p2) { return a; } +int const &overload_params_const(int const p1, int &&a, int p2); + +int const &overload_params_ diff erence1(int p1, int const &a, int p2) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter +int const &overload_params_ diff erence1(long p1, int &&a, int p2); + +int const &overload_params_ diff erence2(int p1, int const &a, int p2) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter +int const &overload_params_ diff erence2(int p1, int &&a, long p2); + +int const &overload_params_ diff erence3(int p1, int const &a, int p2) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter +int const &overload_params_ diff erence3(int p1, long &&a, int p2); + +} // namespace overload _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits