https://github.com/Harald-R updated https://github.com/llvm/llvm-project/pull/206349
>From 52aae0a24a66177b81f4bea598d0c32cb25d1805 Mon Sep 17 00:00:00 2001 From: Harald-R <[email protected]> Date: Wed, 24 Jun 2026 13:56:25 +0300 Subject: [PATCH 1/4] Add bugprone-container-bounds-check-overflow check --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../ContainerBoundsCheckOverflowCheck.cpp | 157 ++++++++++++++++++ .../ContainerBoundsCheckOverflowCheck.h | 38 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../container-bounds-check-overflow.rst | 33 ++++ .../docs/clang-tidy/checks/list.rst | 1 + .../container-bounds-check-overflow.cpp | 153 +++++++++++++++++ 8 files changed, 392 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 3aa39d10ceb5d..485749a13b47f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -21,6 +21,7 @@ #include "ChainedComparisonCheck.h" #include "CommandProcessorCheck.h" #include "ComparePointerToMemberVirtualFunctionCheck.h" +#include "ContainerBoundsCheckOverflowCheck.h" #include "CopyConstructorInitCheck.h" #include "CopyConstructorMutatesArgumentCheck.h" #include "CrtpConstructorAccessibilityCheck.h" @@ -150,6 +151,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-command-processor"); CheckFactories.registerCheck<ComparePointerToMemberVirtualFunctionCheck>( "bugprone-compare-pointer-to-member-virtual-function"); + CheckFactories.registerCheck<ContainerBoundsCheckOverflowCheck>( + "bugprone-container-bounds-check-overflow"); CheckFactories.registerCheck<CopyConstructorInitCheck>( "bugprone-copy-constructor-init"); CheckFactories.registerCheck<CopyConstructorMutatesArgumentCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 43e85b1407f21..8db97682e1505 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -18,6 +18,7 @@ add_clang_library(clangTidyBugproneModule STATIC ChainedComparisonCheck.cpp CommandProcessorCheck.cpp ComparePointerToMemberVirtualFunctionCheck.cpp + ContainerBoundsCheckOverflowCheck.cpp CopyConstructorInitCheck.cpp CopyConstructorMutatesArgumentCheck.cpp CrtpConstructorAccessibilityCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp new file mode 100644 index 0000000000000..d677b3609bf53 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp @@ -0,0 +1,157 @@ +//===----------------------------------------------------------------------===// +// +// 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 "ContainerBoundsCheckOverflowCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +ContainerBoundsCheckOverflowCheck::ContainerBoundsCheckOverflowCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoredContainers(utils::options::parseStringList( + Options.get("IgnoredContainers", ""))) {} + +void ContainerBoundsCheckOverflowCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoredContainers", + utils::options::serializeStringList(IgnoredContainers)); +} + +void ContainerBoundsCheckOverflowCheck::registerMatchers(MatchFinder *Finder) { + auto RecordMatcher = cxxRecordDecl(); + if (!IgnoredContainers.empty()) + RecordMatcher = cxxRecordDecl(unless(hasAnyName(IgnoredContainers))); + auto SizeMethodCall = cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("size"))), + on(hasType(hasCanonicalType(hasDeclaration(RecordMatcher))))); + // The size can either be a direct size() call or a reference to a variable + // initialized from one (e.g. auto s = v.size();) + auto SizeExpr = ignoringParenImpCasts( + expr(anyOf(SizeMethodCall, declRefExpr(to(varDecl(hasInitializer( + ignoringParenImpCasts(SizeMethodCall))))))) + .bind("size_expr")); + + // Operands must be unsigned integers, as overflow in signed integer addition + // is undefined behavior + auto Addition = + binaryOperator(hasOperatorName("+"), hasLHS(hasType(isUnsignedInteger())), + hasRHS(hasType(isUnsignedInteger()))) + .bind("addition"); + auto Comparison = hasAnyOperatorName("<", "<=", ">", ">="); + // Match cases: [Addition] </<=/>/>= [Size] + Finder->addMatcher(binaryOperator(Comparison, hasLHS(Addition), + hasRHS(ignoringParenImpCasts(SizeExpr))) + .bind("comparison_addition_lhs"), + this); + // Match cases: [Size] </<=/>/>= [Addition] + Finder->addMatcher(binaryOperator(Comparison, hasRHS(Addition), + hasLHS(ignoringParenImpCasts(SizeExpr))) + .bind("comparison_addition_rhs"), + this); +} + +void ContainerBoundsCheckOverflowCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Addition = Result.Nodes.getNodeAs<BinaryOperator>("addition"); + const auto *SizeExpr = Result.Nodes.getNodeAs<Expr>("size_expr"); + if (!Addition || !SizeExpr) + return; + const auto *ComparisonAddLhs = + Result.Nodes.getNodeAs<BinaryOperator>("comparison_addition_lhs"); + const auto *ComparisonAddRhs = + Result.Nodes.getNodeAs<BinaryOperator>("comparison_addition_rhs"); + const auto NoComparison = !ComparisonAddLhs && !ComparisonAddRhs; + if (NoComparison) + return; + + auto AdditionType = Addition->getType().getCanonicalType(); + auto SizeExprType = SizeExpr->getType().getCanonicalType(); + + auto &Context = *Result.Context; + // If the type of the addition is smaller than the type of the size() call, + // then the addition will be promoted to the size() type before the + // comparison, so there is no risk of overflow. The case where the type of the + // addition is larger than the type of the size() call is not handled by this + // check + if (Context.getTypeSize(AdditionType) != Context.getTypeSize(SizeExprType)) + return; + + const auto *Comparison = + ComparisonAddLhs ? ComparisonAddLhs : ComparisonAddRhs; + + auto Diag = diag(Comparison->getOperatorLoc(), + "potential overflow in unsigned integer addition " + "before comparison"); + + // A fix is only produced when both operands of the addition are simple + // expressions. If an operand is itself a compound expression (e.g. 'a + b' in + // 'a + b + c'), only a diagnostic is emitted, as rewriting such cases is + // error-prone + const bool HasCompoundOperand = + isa<BinaryOperator>(Addition->getLHS()->IgnoreParenImpCasts()) || + isa<BinaryOperator>(Addition->getRHS()->IgnoreParenImpCasts()); + if (HasCompoundOperand) + return; + + // Introduce parentheses around the addition to avoid changing the order of + // operations when replacing the comparison with a logical AND/OR expression. + // The parentheses are only added if the original expression is not already + // wrapped in parentheses + bool NeedsParens = true; + const auto &Parents = Context.getParents(*Comparison); + if (!Parents.empty()) { + if (Parents[0].get<ParenExpr>() || Parents[0].get<IfStmt>() || + Parents[0].get<WhileStmt>()) + NeedsParens = false; + } + + auto GetText = [&](SourceRange Range) -> StringRef { + return Lexer::getSourceText(CharSourceRange::getTokenRange(Range), + *Result.SourceManager, getLangOpts()); + }; + const std::string StrA = GetText(Addition->getLHS()->getSourceRange()).str(); + const std::string StrB = GetText(Addition->getRHS()->getSourceRange()).str(); + const std::string StrSize = GetText(SizeExpr->getSourceRange()).str(); + + const auto ComparisonType = Comparison->getOpcodeStr(); + std::string Replacement; + if (ComparisonAddLhs) { + // Matches cases where the addition is on the left side of the comparison + // (a + b < size()) -> (a < size() && b < size() - a) + // (a + b <= size()) -> (a <= size() && b <= size() - a) + // (a + b > size()) -> (a > size() || b > size() - a) + // (a + b >= size()) -> (a >= size() || b >= size() - a) + const auto *Expr = + (ComparisonType == "<" || ComparisonType == "<=") ? " && " : " || "; + Replacement = (NeedsParens ? "(" : "") + StrA + " " + ComparisonType.str() + + " " + StrSize + Expr + StrB + " " + ComparisonType.str() + + " " + StrSize + " - " + StrA + (NeedsParens ? ")" : ""); + } else { + // Matches cases where the addition is on the right side of the comparison, + // (size() < a + b) -> (size() < a || size() - a < b) + // (size() <= a + b) -> (size() <= a || size() - a <= b) + // (size() > a + b) -> (size() > a && size() - a > b) + // (size() >= a + b) -> (size() >= a && size() - a >= b) + const auto *Expr = + (ComparisonType == "<" || ComparisonType == "<=") ? " || " : " && "; + Replacement = (NeedsParens ? "(" : "") + StrSize + " " + + ComparisonType.str() + " " + StrA + Expr + StrSize + " - " + + StrA + " " + ComparisonType.str() + " " + StrB + + (NeedsParens ? ")" : ""); + } + + Diag << FixItHint::CreateReplacement(Comparison->getSourceRange(), + Replacement); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h new file mode 100644 index 0000000000000..2809863857c0c --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h @@ -0,0 +1,38 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CONTAINERBOUNDSCHECKOVERFLOWCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CONTAINERBOUNDSCHECKOVERFLOWCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Check for potential overflow in unsigned integer addition before comparison +/// with a container's size() method. For example a + b > v.size() can overflow +/// if a and b are large enough, leading to incorrect behavior +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/container-bounds-check-overflow.html +class ContainerBoundsCheckOverflowCheck : public ClangTidyCheck { +public: + ContainerBoundsCheckOverflowCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + +private: + const std::vector<StringRef> IgnoredContainers; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CONTAINERBOUNDSCHECKOVERFLOWCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 42e4020f5f85c..21fede6e7a653 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -221,6 +221,12 @@ New checks Finds assignments within selection statements. +- New :doc:`bugprone-container-bounds-check-overflow + <clang-tidy/checks/bugprone/container-bounds-check-overflow>` check. + + Finds potential overflow in unsigned integer addition before comparison + with a container's ``size()`` method. + - New :doc:`bugprone-missing-end-comparison <clang-tidy/checks/bugprone/missing-end-comparison>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst new file mode 100644 index 0000000000000..887a1784800b1 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst @@ -0,0 +1,33 @@ +.. title:: clang-tidy - bugprone-container-bounds-check-overflow + +bugprone-container-bounds-check-overflow +======================================== + +This check finds potential overflow in unsigned integer addition before comparison with a container's +``size()`` method. It flags all of the following combinations: +- ``a + b < v.size()`` +- ``a + b <= v.size()`` +- ``a + b > v.size()`` +- ``a + b >= v.size()`` +- ``v.size() < a + b`` +- ``v.size() <= a + b`` +- ``v.size() > a + b`` +- ``v.size() >= a + b`` + +The addition ``a + b`` can overflow if ``a`` and ``b`` are large enough, leading to incorrect behavior. +For example, if ``a`` is ``UINT_MAX`` and ``b`` is ``1``, then ``a + b`` will wrap around to ``0``, +and the comparison can be true, even if the container is empty. + +The comparison is flagged only if size of the unsigned integers being added is the same +as the size of the container's ``size()`` return type. Smaller types are promoted to the size +of the container's ``size()`` return type before the addition, so they are safe from overflow. + +Options +------- + +.. option:: IgnoredContainers + + When set, the check will ignore the specified containers. The value is a + comma-separated list of fully qualified container names. For example, to ignore + ``std::array`` and ``CustomClass``, set the option to ``::std::array,::CustomClass``. + The default is empty, meaning no containers are ignored. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2a44dc78fbc89..2aac5b79b2b8d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -90,6 +90,7 @@ Clang-Tidy Checks :doc:`bugprone-chained-comparison <bugprone/chained-comparison>`, :doc:`bugprone-command-processor <bugprone/command-processor>`, :doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`, + :doc:`bugprone-container-bounds-check-overflow <bugprone/container-bounds-check-overflow>`, "Yes" :doc:`bugprone-copy-constructor-init <bugprone/copy-constructor-init>`, "Yes" :doc:`bugprone-copy-constructor-mutates-argument <bugprone/copy-constructor-mutates-argument>`, :doc:`bugprone-crtp-constructor-accessibility <bugprone/crtp-constructor-accessibility>`, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp new file mode 100644 index 0000000000000..9023e1639df07 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp @@ -0,0 +1,153 @@ +// RUN: %check_clang_tidy %s bugprone-container-bounds-check-overflow %t -- -config='{CheckOptions: { bugprone-container-bounds-check-overflow.IgnoredContainers: "::CustomClass"}}' + +#include <cstddef> +#include <vector> +#include <string> + +#define IS_WITHIN_BOUNDS(a, b, c) (a + b > c) +#define IS_WITHIN_BOUNDS2(a, b, c) ((a) + (b) > (c)) + +namespace { + +class CustomClass { +public: + size_t size() const { + return 0; + } +}; + +size_t size() { + return 0; +} + +// The check must work inside templates and across multiple instantiations +template <typename T> +void templated(size_t a, size_t b, const std::vector<T> &v) { + if (a + b > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (a > v.size() || b > v.size() - a) {} +} + +} + +void positives(size_t a, size_t b, const std::vector<int> &v) { + if (a + b > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (a > v.size() || b > v.size() - a) {} + if (a + b >= v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (a >= v.size() || b >= v.size() - a) {} + if (a + b < v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (a < v.size() && b < v.size() - a) {} + if (a + b <= v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (a <= v.size() && b <= v.size() - a) {} + if (v.size() < a + b) {} + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (v.size() < a || v.size() - a < b) {} + if (v.size() <= a + b) {} + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (v.size() <= a || v.size() - a <= b) {} + if (v.size() > a + b) {} + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (v.size() > a && v.size() - a > b) {} + if (v.size() >= a + b) {} + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (v.size() >= a && v.size() - a >= b) {} + + // Introduces parantheses to avoid changing the order of operations + if (true && a + b > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (true && (a > v.size() || b > v.size() - a)) {} + if (a + b > v.size() && true) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if ((a > v.size() || b > v.size() - a) && true) {} + + // Avoid introducing parantheses if the comparison is already wrapped in one + while(a + b > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: while(a > v.size() || b > v.size() - a) {} + auto result = a + b > v.size(); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: auto result = (a > v.size() || b > v.size() - a); + result = (a + b > v.size()); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: result = (a > v.size() || b > v.size() - a); + (void)result; + + // Confirm the fix works well with different named variables and container types + size_t x = 1; + size_t y = 2; + std::string str = "Hello, world!"; + if (x + y > str.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (x > str.size() || y > str.size() - x) {} + + // The check should match local variables that are assigned the result of a size() call + auto local_size = v.size(); + if (a + b < local_size) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (a < local_size && b < local_size - a) {} + if (local_size < a + b) {} + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (local_size < a || local_size - a < b) {} + + IS_WITHIN_BOUNDS(x, y, v.size()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + IS_WITHIN_BOUNDS2(x, y, v.size()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + + // Chained additions: a compound operand of the addition produces a diagnostic only, with no fix + size_t c = 10; + if (a + b + c > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + if (v.size() < a + b + c) {} + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + if (a + (b + c) > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + if (v.size() < a + b + c || v.empty()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + if (!v.empty() && a + b + c > v.size()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + + // Instantiating templated function which contains the target pattern + templated(a, b, std::vector<int>{}); + templated(a, b, std::vector<double>{}); +} + +void negatives(size_t a, size_t b, const std::vector<int> &v) { + // Cannot overflow because of the comparison order + if (a > v.size() || b > v.size() - a) {} + if (b > v.size() || a > v.size() - b) {} + + // Cannot overflow because the operands of '+' are smaller than the result of size(); the addition result gets promoted to size_t before the comparison + unsigned short x = 1; + unsigned short y = 2; + if (x + y > v.size()) {} + + // Intentionally ignored class + CustomClass custom; + if (a + b > custom.size()) {} + if (a + b >= custom.size()) {} + if (a + b < custom.size()) {} + if (a + b <= custom.size()) {} + if (custom.size() < a + b) {} + if (custom.size() <= a + b) {} + if (custom.size() > a + b) {} + if (custom.size() >= a + b) {} + + // Call to a non-member size() method is not matched + if (a + b > size()) {} + + // A plain variable (not a size() call) is not matched + size_t n = 10; + if (a + b > n) {} + + // Signed operands cannot overflow in a way this check cares about (signed overflow is UB), so they must not be matched + int i = 1; + int j = 2; + if (i + j > v.size()) {} + if (v.size() < i + j) {} +} + >From 4f61732eb4a46ec986cec49faec50273699e405b Mon Sep 17 00:00:00 2001 From: Harald-R <[email protected]> Date: Mon, 29 Jun 2026 20:40:00 +0300 Subject: [PATCH 2/4] Change option description --- .../checks/bugprone/container-bounds-check-overflow.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst index 887a1784800b1..c1d36f5baadca 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst @@ -29,5 +29,5 @@ Options When set, the check will ignore the specified containers. The value is a comma-separated list of fully qualified container names. For example, to ignore - ``std::array`` and ``CustomClass``, set the option to ``::std::array,::CustomClass``. - The default is empty, meaning no containers are ignored. + ``std::array`` and ``CustomClass``, set the option to `::std::array,::CustomClass`. + The default is empty string, meaning no containers are ignored. >From 4403a1729ed747741d57b1c7f9579f8ccdb9abe3 Mon Sep 17 00:00:00 2001 From: Harald-R <[email protected]> Date: Tue, 30 Jun 2026 10:19:59 +0300 Subject: [PATCH 3/4] Align documentation and code comment --- .../clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h | 6 +++--- .../checks/bugprone/container-bounds-check-overflow.rst | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h index 2809863857c0c..d9207ff9d0900 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h @@ -13,9 +13,9 @@ namespace clang::tidy::bugprone { -/// Check for potential overflow in unsigned integer addition before comparison -/// with a container's size() method. For example a + b > v.size() can overflow -/// if a and b are large enough, leading to incorrect behavior +/// Finds potential overflow in unsigned integer addition before comparison with +/// a container's size() method. For example a + b > v.size() can overflow if a +/// and b are large enough, leading to incorrect behavior. /// /// For the user-facing documentation see: /// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/container-bounds-check-overflow.html diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst index c1d36f5baadca..79f353584eb76 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/container-bounds-check-overflow.rst @@ -3,7 +3,7 @@ bugprone-container-bounds-check-overflow ======================================== -This check finds potential overflow in unsigned integer addition before comparison with a container's +Finds potential overflow in unsigned integer addition before comparison with a container's ``size()`` method. It flags all of the following combinations: - ``a + b < v.size()`` - ``a + b <= v.size()`` >From df8f9cc21e201948bae072f2de82f307647ba738 Mon Sep 17 00:00:00 2001 From: Harald-R <[email protected]> Date: Thu, 2 Jul 2026 11:50:21 +0300 Subject: [PATCH 4/4] Add options for configuring size method name and free-standing functions --- .../ContainerBoundsCheckOverflowCheck.cpp | 25 ++++++++++++++----- .../ContainerBoundsCheckOverflowCheck.h | 2 ++ .../container-bounds-check-overflow/size.h | 10 ++++++++ .../container-bounds-check-overflow.cpp | 22 ++++++++++++++-- 4 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/container-bounds-check-overflow/size.h diff --git a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp index d677b3609bf53..f29eba6edad4c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.cpp @@ -19,12 +19,21 @@ ContainerBoundsCheckOverflowCheck::ContainerBoundsCheckOverflowCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IgnoredContainers(utils::options::parseStringList( - Options.get("IgnoredContainers", ""))) {} + Options.get("IgnoredContainers", ""))), + SizeMethodNames(utils::options::parseStringList( + Options.get("SizeMethodNames", "size;length"))), + IncludedFreeStandingSizeFuncNames(utils::options::parseStringList( + Options.get("IncludedFreeStandingSizeFuncNames", "::std::size"))) {} void ContainerBoundsCheckOverflowCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoredContainers", utils::options::serializeStringList(IgnoredContainers)); + Options.store(Opts, "SizeMethodNames", + utils::options::serializeStringList(SizeMethodNames)); + Options.store( + Opts, "IncludedFreeStandingSizeFuncNames", + utils::options::serializeStringList(IncludedFreeStandingSizeFuncNames)); } void ContainerBoundsCheckOverflowCheck::registerMatchers(MatchFinder *Finder) { @@ -32,13 +41,17 @@ void ContainerBoundsCheckOverflowCheck::registerMatchers(MatchFinder *Finder) { if (!IgnoredContainers.empty()) RecordMatcher = cxxRecordDecl(unless(hasAnyName(IgnoredContainers))); auto SizeMethodCall = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("size"))), + callee(cxxMethodDecl(hasAnyName(SizeMethodNames))), on(hasType(hasCanonicalType(hasDeclaration(RecordMatcher))))); - // The size can either be a direct size() call or a reference to a variable - // initialized from one (e.g. auto s = v.size();) + auto FreeStandingSizeCall = callExpr( + callee(functionDecl(hasAnyName(IncludedFreeStandingSizeFuncNames)))); + // The size can either be a direct member size method call, a free-standing + // size function call, or a reference to a variable initialized from one (e.g. + // auto s = v.size();) auto SizeExpr = ignoringParenImpCasts( - expr(anyOf(SizeMethodCall, declRefExpr(to(varDecl(hasInitializer( - ignoringParenImpCasts(SizeMethodCall))))))) + expr(anyOf(SizeMethodCall, FreeStandingSizeCall, + declRefExpr(to(varDecl( + hasInitializer(ignoringParenImpCasts(SizeMethodCall))))))) .bind("size_expr")); // Operands must be unsigned integers, as overflow in signed integer addition diff --git a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h index d9207ff9d0900..a0924aa4a62e9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/ContainerBoundsCheckOverflowCheck.h @@ -31,6 +31,8 @@ class ContainerBoundsCheckOverflowCheck : public ClangTidyCheck { private: const std::vector<StringRef> IgnoredContainers; + const std::vector<StringRef> SizeMethodNames; + const std::vector<StringRef> IncludedFreeStandingSizeFuncNames; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/container-bounds-check-overflow/size.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/container-bounds-check-overflow/size.h new file mode 100644 index 0000000000000..7eaaf6abba1f2 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/container-bounds-check-overflow/size.h @@ -0,0 +1,10 @@ +#ifndef SIZE_H +#define SIZE_H + +namespace std { + +template<class C> constexpr auto size(const C& c) noexcept(noexcept(c.size())) -> decltype(c.size()); + +} // namespace std + +#endif // SIZE_H diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp index 9023e1639df07..336cb6e08c1f1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/container-bounds-check-overflow.cpp @@ -1,5 +1,15 @@ -// RUN: %check_clang_tidy %s bugprone-container-bounds-check-overflow %t -- -config='{CheckOptions: { bugprone-container-bounds-check-overflow.IgnoredContainers: "::CustomClass"}}' - +// RUN: %check_clang_tidy %s bugprone-container-bounds-check-overflow %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-container-bounds-check-overflow.IgnoredContainers: "::CustomClass" \ +// RUN: }}' -- -I %S/Inputs/container-bounds-check-overflow +// RUN: %check_clang_tidy %s bugprone-container-bounds-check-overflow %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-container-bounds-check-overflow.IgnoredContainers: "::CustomClass", \ +// RUN: bugprone-container-bounds-check-overflow.SizeMethodNames: "size;length", \ +// RUN: bugprone-container-bounds-check-overflow.IncludedFreeStandingSizeFuncNames: "::std::size" \ +// RUN: }}' -- -I %S/Inputs/container-bounds-check-overflow + +#include "size.h" #include <cstddef> #include <vector> #include <string> @@ -84,6 +94,14 @@ void positives(size_t a, size_t b, const std::vector<int> &v) { // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] // CHECK-FIXES: if (x > str.size() || y > str.size() - x) {} + // Confirm the check matches for all supported size method names, including free-standing size functions + if (x + y > str.length()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (x > str.length() || y > str.length() - x) {} + if (x + y > std::size(str)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: potential overflow in unsigned integer addition before comparison [bugprone-container-bounds-check-overflow] + // CHECK-FIXES: if (x > std::size(str) || y > std::size(str) - x) {} + // The check should match local variables that are assigned the result of a size() call auto local_size = v.size(); if (a + b < local_size) {} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
