diff --git a/.github/workflows/commit-access-review.py b/.github/workflows/commit-access-review.py index 97589c138c0b70..8ea9b1fcc2fb08 100644 --- a/.github/workflows/commit-access-review.py +++ b/.github/workflows/commit-access-review.py @@ -358,11 +358,10 @@ def main(): gh = github.Github(login_or_token=token) org = gh.get_organization("llvm") repo = org.get_repo("llvm-project") - team = org.get_team_by_slug("llvm-committers") one_year_ago = datetime.datetime.now() - datetime.timedelta(days=365) triage_list = {} - for member in team.get_members(): - triage_list[member.login] = User(member.login, triage_list) + for collaborator in repo.get_collaborators(permission="push"): + triage_list[collaborator.login] = User(collaborator.login, triage_list) print("Start:", len(triage_list), "triagers") # Step 0 Check if users have requested commit access in the last year. diff --git a/bolt/test/lit.local.cfg b/bolt/test/lit.local.cfg index 8aa5f15d5ccfb4..e2fa0a4a2210f6 100644 --- a/bolt/test/lit.local.cfg +++ b/bolt/test/lit.local.cfg @@ -1,6 +1,6 @@ host_linux_triple = config.target_triple.split("-")[0] + "-unknown-linux-gnu" -common_linker_flags = "-fuse-ld=lld -Wl,--unresolved-symbols=ignore-all" -flags = f"--target={host_linux_triple} {common_linker_flags}" +common_linker_flags = "-fuse-ld=lld -Wl,--unresolved-symbols=ignore-all -pie" +flags = f"--target={host_linux_triple} -fPIE {common_linker_flags}" config.substitutions.insert(0, ("%cflags", f"%cflags {flags}")) config.substitutions.insert(0, ("%cxxflags", f"%cxxflags {flags}")) diff --git a/bolt/test/perf2bolt/lit.local.cfg b/bolt/test/perf2bolt/lit.local.cfg index 4ee9ad08cc78a0..0fecf913aa98b8 100644 --- a/bolt/test/perf2bolt/lit.local.cfg +++ b/bolt/test/perf2bolt/lit.local.cfg @@ -1,4 +1,5 @@ import shutil +import subprocess -if shutil.which("perf") is not None: - config.available_features.add("perf") \ No newline at end of file +if shutil.which("perf") is not None and subprocess.run(["perf", "record", "-e", "cycles:u", "-o", "/dev/null", "--", "perf", "--version"], capture_output=True).returncode == 0: + config.available_features.add("perf") diff --git a/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp index c87b3ea7e26163..00e8f7e514368b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp @@ -9,7 +9,6 @@ #include "ForwardingReferenceOverloadCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include using namespace clang::ast_matchers; @@ -19,14 +18,14 @@ namespace { // Check if the given type is related to std::enable_if. AST_MATCHER(QualType, isEnableIf) { auto CheckTemplate = [](const TemplateSpecializationType *Spec) { - if (!Spec || !Spec->getTemplateName().getAsTemplateDecl()) { + if (!Spec) return false; - } - const NamedDecl *TypeDecl = - Spec->getTemplateName().getAsTemplateDecl()->getTemplatedDecl(); - return TypeDecl->isInStdNamespace() && - (TypeDecl->getName() == "enable_if" || - TypeDecl->getName() == "enable_if_t"); + + const TemplateDecl *TDecl = Spec->getTemplateName().getAsTemplateDecl(); + + return TDecl && TDecl->isInStdNamespace() && + (TDecl->getName() == "enable_if" || + TDecl->getName() == "enable_if_t"); }; const Type *BaseType = Node.getTypePtr(); // Case: pointer or reference to enable_if. diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp index d517e8473d94ae..a30e63f9b0fd6a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp @@ -48,6 +48,8 @@ AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth, return false; } +AST_MATCHER(Expr, offsetOfExpr) { return isa(Node); } + CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) { if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() || isa(Ty) || !Ty->isConstantSizeType()) @@ -221,17 +223,15 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { const auto ElemType = arrayType(hasElementType(recordType().bind("elem-type"))); const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type"))); + const auto SizeofDivideExpr = binaryOperator( + hasOperatorName("/"), + hasLHS( + ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(hasCanonicalType( + type(anyOf(ElemType, ElemPtrType, type())).bind("num-type")))))), + hasRHS(ignoringParenImpCasts(sizeOfExpr( + hasArgumentOfType(hasCanonicalType(type().bind("denom-type"))))))); - Finder->addMatcher( - binaryOperator( - hasOperatorName("/"), - hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType( - hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type())) - .bind("num-type")))))), - hasRHS(ignoringParenImpCasts(sizeOfExpr( - hasArgumentOfType(hasCanonicalType(type().bind("denom-type"))))))) - .bind("sizeof-divide-expr"), - this); + Finder->addMatcher(SizeofDivideExpr.bind("sizeof-divide-expr"), this); // Detect expression like: sizeof(...) * sizeof(...)); most likely an error. Finder->addMatcher(binaryOperator(hasOperatorName("*"), @@ -257,8 +257,9 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { .bind("sizeof-sizeof-expr"), this); - // Detect sizeof in pointer arithmetic like: N * sizeof(S) == P1 - P2 or - // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S. + // Detect sizeof usage in comparisons involving pointer arithmetics, such as + // N * sizeof(T) == P1 - P2 or (P1 - P2) / sizeof(T), where P1 and P2 are + // pointers to a type T. const auto PtrDiffExpr = binaryOperator( hasOperatorName("-"), hasLHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee( @@ -285,6 +286,47 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { hasRHS(ignoringParenImpCasts(SizeOfExpr.bind("sizeof-ptr-div-expr")))) .bind("sizeof-in-ptr-arithmetic-div"), this); + + // SEI CERT ARR39-C. Do not add or subtract a scaled integer to a pointer. + // Detect sizeof, alignof and offsetof usage in pointer arithmetics where + // they are used to scale the numeric distance, which is scaled again by + // the pointer arithmetic operator. This can result in forming invalid + // offsets. + // + // Examples, where P is a pointer, N is some integer (both compile-time and + // run-time): P + sizeof(T), P + sizeof(*P), P + N * sizeof(*P). + // + // This check does not warn on cases where the pointee type is "1 byte", + // as those cases can often come from generics and also do not constitute a + // problem because the size does not affect the scale used. + const auto InterestingPtrTyForPtrArithmetic = + pointerType(pointee(qualType().bind("pointee-type"))); + const auto SizeofLikeScaleExpr = + expr(anyOf(unaryExprOrTypeTraitExpr(ofKind(UETT_SizeOf)), + unaryExprOrTypeTraitExpr(ofKind(UETT_AlignOf)), + offsetOfExpr())) + .bind("sizeof-in-ptr-arithmetic-scale-expr"); + const auto PtrArithmeticIntegerScaleExpr = binaryOperator( + hasAnyOperatorName("*", "/"), + // sizeof(...) * sizeof(...) and sizeof(...) / sizeof(...) is handled + // by this check on another path. + hasOperands(expr(hasType(isInteger()), unless(SizeofLikeScaleExpr)), + SizeofLikeScaleExpr)); + const auto PtrArithmeticScaledIntegerExpr = + expr(anyOf(SizeofLikeScaleExpr, PtrArithmeticIntegerScaleExpr), + unless(SizeofDivideExpr)); + + Finder->addMatcher( + expr(anyOf( + binaryOperator(hasAnyOperatorName("+", "-"), + hasOperands(hasType(InterestingPtrTyForPtrArithmetic), + PtrArithmeticScaledIntegerExpr)) + .bind("sizeof-in-ptr-arithmetic-plusminus"), + binaryOperator(hasAnyOperatorName("+=", "-="), + hasLHS(hasType(InterestingPtrTyForPtrArithmetic)), + hasRHS(PtrArithmeticScaledIntegerExpr)) + .bind("sizeof-in-ptr-arithmetic-plusminus"))), + this); } void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) { @@ -409,6 +451,43 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) { << SizeOfExpr->getSourceRange() << E->getOperatorLoc() << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); } + } else if (const auto *E = Result.Nodes.getNodeAs( + "sizeof-in-ptr-arithmetic-plusminus")) { + const auto *PointeeTy = Result.Nodes.getNodeAs("pointee-type"); + const auto *ScaleExpr = + Result.Nodes.getNodeAs("sizeof-in-ptr-arithmetic-scale-expr"); + const CharUnits PointeeSize = getSizeOfType(Ctx, PointeeTy->getTypePtr()); + const int ScaleKind = [ScaleExpr]() { + if (const auto *UTTE = dyn_cast(ScaleExpr)) + switch (UTTE->getKind()) { + case UETT_SizeOf: + return 0; + case UETT_AlignOf: + return 1; + default: + return -1; + } + + if (isa(ScaleExpr)) + return 2; + + return -1; + }(); + + if (ScaleKind != -1 && PointeeSize > CharUnits::One()) { + diag(E->getExprLoc(), + "suspicious usage of '%select{sizeof|alignof|offsetof}0(...)' in " + "pointer arithmetic; this scaled value will be scaled again by the " + "'%1' operator") + << ScaleKind << E->getOpcodeStr() << ScaleExpr->getSourceRange(); + diag(E->getExprLoc(), + "'%0' in pointer arithmetic internally scales with 'sizeof(%1)' == " + "%2", + DiagnosticIDs::Note) + << E->getOpcodeStr() + << PointeeTy->getAsString(Ctx.getPrintingPolicy()) + << PointeeSize.getQuantity(); + } } } diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h index 9ca17bc9e6f124..66d7c34cc9e940 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h @@ -13,7 +13,7 @@ namespace clang::tidy::bugprone { -/// Find suspicious usages of sizeof expression. +/// Find suspicious usages of sizeof expressions. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index 8b5be9cd95f767..26befe0de59ae4 100644 --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -14,6 +14,7 @@ #include "../bugprone/ReservedIdentifierCheck.h" #include "../bugprone/SignalHandlerCheck.h" #include "../bugprone/SignedCharMisuseCheck.h" +#include "../bugprone/SizeofExpressionCheck.h" #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" #include "../bugprone/SuspiciousMemoryComparisonCheck.h" #include "../bugprone/UnhandledSelfAssignmentCheck.h" @@ -281,6 +282,9 @@ class CERTModule : public ClangTidyModule { "cert-oop58-cpp"); // C checkers + // ARR + CheckFactories.registerCheck( + "cert-arr39-c"); // CON CheckFactories.registerCheck( "cert-con36-c"); @@ -332,6 +336,12 @@ class CERTModule : public ClangTidyModule { ClangTidyOptions getModuleOptions() override { ClangTidyOptions Options; ClangTidyOptions::OptionMap &Opts = Options.CheckOptions; + Opts["cert-arr39-c.WarnOnSizeOfConstant"] = "false"; + Opts["cert-arr39-c.WarnOnSizeOfIntegerExpression"] = "false"; + Opts["cert-arr39-c.WarnOnSizeOfThis"] = "false"; + Opts["cert-arr39-c.WarnOnSizeOfCompareToConstant"] = "false"; + Opts["cert-arr39-c.WarnOnSizeOfPointer"] = "false"; + Opts["cert-arr39-c.WarnOnSizeOfPointerToAggregate"] = "false"; Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU"; Opts["cert-err33-c.CheckedFunctions"] = CertErr33CCheckedFunctions; Opts["cert-err33-c.AllowCastToVoid"] = "true"; diff --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp index 21008bc144b91a..ee869256898989 100644 --- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp @@ -102,7 +102,7 @@ void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) { // inline is not allowed for main function. if (FD->isMain()) return; - diag(FD->getLocation(), /*Description=*/"make as 'inline'", + diag(FD->getLocation(), "mark the definition as 'inline'", DiagnosticIDs::Note) << FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline "); } else if (const auto *VD = dyn_cast(ND)) { diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp index 89790ea70cf229..98778192dbd3ae 100644 --- a/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp @@ -9,6 +9,7 @@ #include "AvoidCArraysCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" using namespace clang::ast_matchers; @@ -60,6 +61,7 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( typeLoc(hasValidBeginLoc(), hasType(arrayType()), + optionally(hasParent(parmVarDecl().bind("param_decl"))), unless(anyOf(hasParent(parmVarDecl(isArgvOfMain())), hasParent(varDecl(isExternC())), hasParent(fieldDecl( @@ -72,11 +74,28 @@ void AvoidCArraysCheck::registerMatchers(MatchFinder *Finder) { void AvoidCArraysCheck::check(const MatchFinder::MatchResult &Result) { const auto *ArrayType = Result.Nodes.getNodeAs("typeloc"); - + const bool IsInParam = + Result.Nodes.getNodeAs("param_decl") != nullptr; + const bool IsVLA = ArrayType->getTypePtr()->isVariableArrayType(); + enum class RecommendType { Array, Vector, Span }; + llvm::SmallVector RecommendTypes{}; + if (IsVLA) { + RecommendTypes.push_back("std::vector<>"); + } else if (ArrayType->getTypePtr()->isIncompleteArrayType() && IsInParam) { + // in function parameter, we also don't know the size of + // IncompleteArrayType. + if (Result.Context->getLangOpts().CPlusPlus20) + RecommendTypes.push_back("std::span<>"); + else { + RecommendTypes.push_back("std::array<>"); + RecommendTypes.push_back("std::vector<>"); + } + } else { + RecommendTypes.push_back("std::array<>"); + } diag(ArrayType->getBeginLoc(), - "do not declare %select{C-style|C VLA}0 arrays, use " - "%select{std::array<>|std::vector<>}0 instead") - << ArrayType->getTypePtr()->isVariableArrayType(); + "do not declare %select{C-style|C VLA}0 arrays, use %1 instead") + << IsVLA << llvm::join(RecommendTypes, " or "); } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp index 418699ffbc4d1a..9861f4681db1b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp @@ -72,7 +72,11 @@ static FindArgsResult findArgs(const CallExpr *Call) { return Result; } -static SmallVector +// Returns `true` as `first` only if a nested call to `std::min` or +// `std::max` was found. Checking if `FixItHint`s were generated is not enough, +// as the explicit casts that the check introduces may be generated without a +// nested `std::min` or `std::max` call. +static std::pair> generateReplacements(const MatchFinder::MatchResult &Match, const CallExpr *TopCall, const FindArgsResult &Result, const bool IgnoreNonTrivialTypes, @@ -91,13 +95,15 @@ generateReplacements(const MatchFinder::MatchResult &Match, const bool IsResultTypeTrivial = ResultType.isTrivialType(*Match.Context); if ((!IsResultTypeTrivial && IgnoreNonTrivialTypes)) - return FixItHints; + return {false, FixItHints}; if (IsResultTypeTrivial && static_cast( Match.Context->getTypeSizeInChars(ResultType).getQuantity()) > IgnoreTrivialTypesOfSizeAbove) - return FixItHints; + return {false, FixItHints}; + + bool FoundNestedCall = false; for (const Expr *Arg : Result.Args) { const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts()); @@ -146,6 +152,9 @@ generateReplacements(const MatchFinder::MatchResult &Match, *Match.Context)) continue; + // We have found a nested call + FoundNestedCall = true; + // remove the function call FixItHints.push_back( FixItHint::CreateRemoval(InnerCall->getCallee()->getSourceRange())); @@ -168,7 +177,7 @@ generateReplacements(const MatchFinder::MatchResult &Match, CharSourceRange::getTokenRange(InnerResult.First->getEndLoc()))); } - const SmallVector InnerReplacements = generateReplacements( + const auto [_, InnerReplacements] = generateReplacements( Match, InnerCall, InnerResult, IgnoreNonTrivialTypes, IgnoreTrivialTypesOfSizeAbove); @@ -189,7 +198,7 @@ generateReplacements(const MatchFinder::MatchResult &Match, } } - return FixItHints; + return {FoundNestedCall, FixItHints}; } MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck( @@ -238,11 +247,11 @@ void MinMaxUseInitializerListCheck::check( const auto *TopCall = Match.Nodes.getNodeAs("topCall"); const FindArgsResult Result = findArgs(TopCall); - const SmallVector Replacements = + const auto [FoundNestedCall, Replacements] = generateReplacements(Match, TopCall, Result, IgnoreNonTrivialTypes, IgnoreTrivialTypesOfSizeAbove); - if (Replacements.empty()) + if (!FoundNestedCall) return; const DiagnosticBuilder Diagnostic = diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp index 8f2841c32259a2..1cb95c2b2347b7 100644 --- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -141,16 +141,18 @@ void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } void EnumInitialValueCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - enumDecl(unless(isMacro()), unless(hasConsistentInitialValues())) - .bind("inconsistent"), - this); + Finder->addMatcher(enumDecl(isDefinition(), unless(isMacro()), + unless(hasConsistentInitialValues())) + .bind("inconsistent"), + this); if (!AllowExplicitZeroFirstInitialValue) Finder->addMatcher( - enumDecl(hasZeroInitialValueForFirstEnumerator()).bind("zero_first"), + enumDecl(isDefinition(), hasZeroInitialValueForFirstEnumerator()) + .bind("zero_first"), this); if (!AllowExplicitSequentialInitialValues) - Finder->addMatcher(enumDecl(unless(isMacro()), hasSequentialInitialValues()) + Finder->addMatcher(enumDecl(isDefinition(), unless(isMacro()), + hasSequentialInitialValues()) .bind("sequential"), this); } @@ -159,7 +161,7 @@ void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Enum = Result.Nodes.getNodeAs("inconsistent")) { DiagnosticBuilder Diag = diag(Enum->getBeginLoc(), - "inital values in enum %0 are not consistent, consider explicit " + "initial values in enum %0 are not consistent, consider explicit " "initialization of all, none or only the first enumerator") << Enum; for (const EnumConstantDecl *ECD : Enum->enumerators()) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8d0c093b312dd5..2370b594d22269 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,10 @@ New checks New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cert-arr39-c ` to + :doc:`bugprone-sizeof-expression + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -111,14 +115,32 @@ Changes in existing checks ` check to suggest replacing the offending code with ``reinterpret_cast``, to more clearly express intent. -- Improved :doc:`cert-flp30-c` check to +- Improved :doc:`bugprone-forwarding-reference-overload + ` check by fixing + a crash when determining if an ``enable_if[_t]`` was found. + +- Improved :doc:`bugprone-sizeof-expression + ` check to find suspicious + usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or + subtracting from a pointer. + +- Improved :doc:`cert-flp30-c ` check to fix false positive that floating point variable is only used in increment expression. - Improved :doc:`cppcoreguidelines-prefer-member-initializer - ` check to avoid - false positive when member initialization depends on a structured binging - variable. + ` check to + avoid false positive when member initialization depends on a structured + binding variable. + +- Improved :doc:`misc-definitions-in-headers + ` check by rewording the + diagnostic note that suggests adding ``inline``. + +- Improved :doc:`modernize-avoid-c-arrays + ` check to suggest using ``std::span`` + as a replacement for parameters of incomplete C array type in C++20 and + ``std::array`` or ``std::vector`` before C++20. - Improved :doc:`modernize-use-std-format ` check to support replacing @@ -128,10 +150,20 @@ Changes in existing checks ` check to avoid false positive for C++23 deducing this. +- Improved :doc:`modernize-min-max-use-initializer-list + ` check by fixing + a false positive when only an implicit conversion happened inside an + initializer list. + - Improved :doc:`modernize-use-std-print ` check to support replacing member function calls too. +- Improved :doc:`readability-enum-initial-value + ` check by only issuing + diagnostics for the definition of an ``enum``, and by fixing a typo in the + diagnostic. + - Improved :doc:`performance-avoid-endl ` check to use ``std::endl`` as placeholder when lexer cannot get source text. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst index ed5bb4fbb89baf..89c88d2740dba2 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst @@ -164,6 +164,103 @@ hidden through macros. memcpy(dst, buf, sizeof(INT_SZ)); // sizeof(sizeof(int)) is suspicious. } +Suspicious usages of 'sizeof(...)' in pointer arithmetic +-------------------------------------------------------- + +Arithmetic operators on pointers automatically scale the result with the size +of the pointed typed. +Further use of ``sizeof`` around pointer arithmetic will typically result in an +unintended result. + +Scaling the result of pointer difference +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Subtracting two pointers results in an integer expression (of type +``ptrdiff_t``) which expresses the distance between the two pointed objects in +"number of objects between". +A common mistake is to think that the result is "number of bytes between", and +scale the difference with ``sizeof``, such as ``P1 - P2 == N * sizeof(T)`` +(instead of ``P1 - P2 == N``) or ``(P1 - P2) / sizeof(T)`` instead of +``P1 - P2``. + +.. code-block:: c++ + + void splitFour(const Obj* Objs, size_t N, Obj Delimiter) { + const Obj *P = Objs; + while (P < Objs + N) { + if (*P == Delimiter) { + break; + } + } + + if (P - Objs != 4 * sizeof(Obj)) { // Expecting a distance multiplied by sizeof is suspicious. + error(); + } + } + +.. code-block:: c++ + + void iterateIfEvenLength(int *Begin, int *End) { + auto N = (Begin - End) / sizeof(int); // Dividing by sizeof() is suspicious. + if (N % 2) + return; + + // ... + } + +Stepping a pointer with a scaled integer +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Conversely, when performing pointer arithmetics to add or subtract from a +pointer, the arithmetic operator implicitly scales the value actually added to +the pointer with the size of the pointee, as ``Ptr + N`` expects ``N`` to be +"number of objects to step", and not "number of bytes to step". + +Seeing the calculation of a pointer where ``sizeof`` appears is suspicious, +and the result is typically unintended, often out of bounds. +``Ptr + sizeof(T)`` will offset the pointer by ``sizeof(T)`` elements, +effectively exponentiating the scaling factor to the power of 2. + +This case also checks suspicious ``alignof`` and ``offsetof`` usages in +pointer arithmetic, as both return the "size" in bytes and not elements, +potentially resulting in doubly-scaled offsets. + +.. code-block:: c++ + + void printEveryEvenIndexElement(int *Array, size_t N) { + int *P = Array; + while (P <= Array + N * sizeof(int)) { // Suspicious pointer arithmetic using sizeof()! + printf("%d ", *P); + + P += 2 * sizeof(int); // Suspicious pointer arithmetic using sizeof()! + } + } + +.. code-block:: c++ + + struct Message { /* ... */; char Flags[8]; }; + void clearFlags(Message *Array, size_t N) { + const Message *End = Array + N; + while (Array < End) { + memset(Array + offsetof(Message, Flags), // Suspicious pointer arithmetic using offsetof()! + 0, sizeof(Message::Flags)); + ++Array; + } + } + +For this checked bogus pattern, `cert-arr39-c` redirects here as an alias of +this check. + +This check corresponds to the CERT C Coding Standard rule +`ARR39-C. Do not add or subtract a scaled integer to a pointer +`_. + +Limitations +""""""""""" + +Cases where the pointee type has a size of `1` byte (such as, and most +importantly, ``char``) are excluded. + Options ------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst new file mode 100644 index 00000000000000..21353c61eede86 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-arr39-c +.. meta:: + :http-equiv=refresh: 5;URL=../bugprone/sizeof-expression.html + +cert-arr39-c +============ + +The `cert-arr39-c` check is an alias, please see +:doc:`bugprone-sizeof-expression <../bugprone/sizeof-expression>` +for more information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index a931ebf025a10e..1909d7b8d8e246 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -407,6 +407,7 @@ Check aliases :header: "Name", "Redirect", "Offers fixes" :doc:`bugprone-narrowing-conversions `, :doc:`cppcoreguidelines-narrowing-conversions `, + :doc:`cert-arr39-c `, :doc:`bugprone-sizeof-expression `, :doc:`cert-con36-c `, :doc:`bugprone-spuriously-wake-up-functions `, :doc:`cert-con54-cpp `, :doc:`bugprone-spuriously-wake-up-functions `, :doc:`cert-ctr56-cpp `, :doc:`bugprone-pointer-arithmetic-on-polymorphic-object `, diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst index 8f13ca4466a310..2d72352989ab94 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-c-arrays.rst @@ -10,6 +10,9 @@ modernize-avoid-c-arrays Finds C-style array types and recommend to use ``std::array<>`` / ``std::vector<>``. All types of C arrays are diagnosed. +For incomplete C-style array types appeared in parameters, It would be better to +use ``std::span`` / ``gsl::span`` as replacement. + However, fix-it are potentially dangerous in header files and are therefore not emitted right now. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst index 1ec753ef090de1..b88fde5162e28c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst @@ -43,7 +43,7 @@ Options extern std::string strprintf(const char *format, ...); int i = -42; unsigned int u = 0xffffffff; - return strprintf("%d %u\n", i, u); + return strprintf("%u %d\n", i, u); would be converted to diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst index 59bb722e2c24fc..e70402ad8b3341 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst @@ -103,7 +103,7 @@ Options int i = -42; unsigned int u = 0xffffffff; - printf("%d %u\n", i, u); + printf("%u %d\n", i, u); would be converted to: diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload.cpp index 92dfb718bb51b7..27315199c7ebae 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload.cpp @@ -261,3 +261,9 @@ class Test11 { Test11(const Test11 &) = default; Test11(Test11 &&) = default; }; + +template