Skip to content

Commit

Permalink
[clang-tidy] Avoid overflow when dumping unsigned integer values (llv…
Browse files Browse the repository at this point in the history
…m#85060)

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.

Fixes llvm#60217
  • Loading branch information
ealcdan authored Apr 23, 2024
1 parent bac5d8e commit c52b18d
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 2 deletions.
6 changes: 6 additions & 0 deletions clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions clang-tools-extra/clang-tidy/ClangTidyCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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``.

- Fixed ``--verify-config`` option not properly parsing checks when using the
literal operator in the ``.clang-tidy`` config.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
InheritParentConfig: true
Checks: 'misc-throw-by-value-catch-by-reference'
CheckOptions:
misc-throw-by-value-catch-by-reference.MaxSize: '1152921504606846976'
Original file line number Diff line number Diff line change
Expand Up @@ -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'"

0 comments on commit c52b18d

Please sign in to comment.