[clang-tools-extra] [clang-tidy] Avoid overflow when dumping unsigned integer values (PR #85060)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 22 12:02:17 PDT 2024


https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/85060

>From 07248879092c0e752811b4c33b98c50d42230d14 Mon Sep 17 00:00:00 2001
From: Daniel Alcaide Nombela <daniel.alcaide.nombela at ericsson.com>
Date: Wed, 13 Mar 2024 11:28:34 +0100
Subject: [PATCH] [clang-tidy] Avoid overflow when dumping unsigned integer
 values

Some options take the maximum unsigned integer value as default, but
they are being dumped to a string as integers. This makes -dump-config
write invalid '-1' values for these options. This change fixes this
issue by using utostr if the option is unsigned.

Change-Id: I551e6bc616071cf7aa10a8c0220d2076ed3d40e6
---
 clang-tools-extra/clang-tidy/ClangTidyCheck.cpp          | 6 ++++++
 clang-tools-extra/clang-tidy/ClangTidyCheck.h            | 9 +++++++--
 clang-tools-extra/docs/ReleaseNotes.rst                  | 2 ++
 .../infrastructure/Inputs/config-files/5/.clang-tidy     | 4 ++++
 .../test/clang-tidy/infrastructure/config-files.cpp      | 8 ++++++++
 5 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/5/.clang-tidy

diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index 3e926236adb451..710b361e16c0a7 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -139,6 +139,12 @@ void ClangTidyCheck::OptionsView::storeInt(ClangTidyOptions::OptionMap &Options,
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+void ClangTidyCheck::OptionsView::storeUnsigned(
+    ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+    uint64_t Value) const {
+  store(Options, LocalName, llvm::utostr(Value));
+}
+
 template <>
 void ClangTidyCheck::OptionsView::store<bool>(
     ClangTidyOptions::OptionMap &Options, StringRef LocalName,
diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index 656a2f008f6e0e..7427aa9bf48f89 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -411,7 +411,10 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     std::enable_if_t<std::is_integral_v<T>>
     store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
           T Value) const {
-      storeInt(Options, LocalName, Value);
+      if constexpr (std::is_signed_v<T>)
+        storeInt(Options, LocalName, Value);
+      else
+        storeUnsigned(Options, LocalName, Value);
     }
 
     /// Stores an option with the check-local name \p LocalName with
@@ -422,7 +425,7 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
           std::optional<T> Value) const {
       if (Value)
-        storeInt(Options, LocalName, *Value);
+        store(Options, LocalName, *Value);
       else
         store(Options, LocalName, "none");
     }
@@ -470,6 +473,8 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
                   int64_t Value) const;
 
+    void storeUnsigned(ClangTidyOptions::OptionMap &Options,
+                       StringRef LocalName, uint64_t Value) const;
 
     std::string NamePrefix;
     const ClangTidyOptions::OptionMap &CheckOptions;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9ef1d38d3c4560..27dabc0ed42f95 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -102,6 +102,8 @@ Improvements to clang-tidy
   similar fashion to what `-header-filter` does for header files.
 - Improved :program:`check_clang_tidy.py` script. Added argument `-export-fixes`
   to aid in clang-tidy and test development.
+- Fixed bug where big values for unsigned check options overflowed into negative values
+  when being printed with ``--dump-config``.
 
 New checks
 ^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/5/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/5/.clang-tidy
new file mode 100644
index 00000000000000..e33f0f8bb33218
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/5/.clang-tidy
@@ -0,0 +1,4 @@
+InheritParentConfig: true
+Checks: 'misc-throw-by-value-catch-by-reference'
+CheckOptions:
+  misc-throw-by-value-catch-by-reference.MaxSize: '1152921504606846976'
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
index ab4f3becb7a9fc..cb0f0bc4d13308 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -64,3 +64,11 @@
 
 // Validate that check options are printed in alphabetical order:
 // RUN: clang-tidy --checks="-*,readability-identifier-naming" --dump-config %S/Inputs/config-files/- -- | grep "readability-identifier-naming\." | sort --check
+
+// Dumped config does not overflow for unsigned options
+// RUN: clang-tidy --dump-config \
+// RUN: --checks="-*,misc-throw-by-value-catch-by-reference" \
+// RUN: -- | grep -v -q "misc-throw-by-value-catch-by-reference.MaxSize: '-1'"
+
+// RUN: clang-tidy --dump-config %S/Inputs/config-files/5/- \
+// RUN: -- | grep -q "misc-throw-by-value-catch-by-reference.MaxSize: '1152921504606846976'"



More information about the cfe-commits mailing list