[clang-tidy] new check: bugprone-signed-char-misuse

[ztamas]

Summary

This check searches for signed char -> integer conversions which might
indicate programming error, because of the misinterpretation of char
values. A signed char might store the non-ASCII characters as negative
values. The human programmer probably expects that after an integer
conversion the converted value matches with the character code
(a value from [0..255]), however, the actual value is in
[-128..127] interval.

See also:
STR34-C. Cast characters to unsigned char before converting to larger integer sizes
https://wiki.sei.cmu.edu/confluence/display/c/STR34-C.+Cast+characters+to+unsigned+char+before+converting+to+larger+integer+sizes

By now this check is limited to assignment / variable declarations.
If we would catch all signed char -> integer conversion, then it would
produce a lot of findings and also false positives. So I added only
this use case now, but this check can be extended with additional
use cases later.
The CERT documentation mentions another use case when the char is
used for array subscript. Next to that a third use case can be
the signed char - unsigned char comparison, which also a use case
where things happen unexpectedly because of conversion to integer.

[ztamas]

I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore = "sal_Int8".
The check produced 32 findings.

I see 3 false positives which are similar to the DereferenceWithTypdef test case where the code
uses sal_Int8 typedef, but the check still catches the cast, because the typedef information is
somehow lost by the dereferencing. Other use cases seem to be valid catches.

I had a closer look at some of the catches. Two of them were doing the same conversion what
an unsigned char conversion does. So there I replaced the old solution with a cast:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=85b5253492cd1c87cebc98d4c453812562a2f9ef

In other cases, the code is suspicious and surely would fail on non-ASCII characters, but
because of the context that does not happen. For example, a code which works with
font names never could get any special character, since font names are ASCII strings.

The output can be found here:
https://gist.github.com/tzolnai/c7e70d9bb51b78dd8a4036d209b19682

[ztamas]

I run the new check on LLVM codebase with the option CharTypdefsToIgnore = "int8_t".
The check produced 12 findings.

All findings seem valid use cases, where a char -> integer conversion happens.
I had a closer look at some of the catches.

Somewhere I see only type mismatch. Char is used instead of an integer type.
For example, in the finding bellow, trailingBytesForUTF8 is defined as char array, but it is used
everywhere as integer:

/home/zolnai/clang/llvm-project/llvm/lib/Support/ConvertUTF.cpp:623:43: warning: singed char -> integer ('unsigned short') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
        unsigned short extraBytesToRead = trailingBytesForUTF8[*source];

Another use case seems suspicious to me. Here we have some parsing code, if I'm right, which uses the EOF from cstdio header, which is -1,
so might be a similar use case which is mentioned by the CERT specification.

/home/zolnai/clang/llvm-project/llvm/lib/TableGen/TGLexer.cpp:596:20: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
    int NextChar = *CurPtr;

At the location of the third use case, there is already a comment about signed chars ("/* xxx what about signed chars here... */"),
so I interpret it as a validation of the check.

/home/zolnai/clang/llvm-project/llvm/lib/Support/regcomp.c:929:12: warning: singed char -> integer ('int') conversion; consider to cast to unsigned char first. [bugprone-signed-char-misuse]
                for (i = start; i <= finish; i++)

The whole output can be found here:
https://gist.github.com/tzolnai/b1cbe3f021b53a7ca1a6ecbffc1a9bf6

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

23

Please use static for functions. See LLVM Coding Guidelines.

clang-tools-extra/docs/ReleaseNotes.rst

100

One sentence is enough.

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

77

Please add newline.

clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp

75

Please add newline.

[ztamas]

Fixes small things mentioned by reviewer commments.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

22

Anonymous namespace is not needed anymore.

[ztamas]

Remove anonymus namespace

[aaron.ballman]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

44–45

Does this properly handle architectures where char is signed rather than unsigned? Or does this only handle the case where the user wrote signed char?

50

Spurious semicolon?

57

Spurious semicolon?

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

26

Should this check also be registered in the CERT module?

[ztamas]

Update code based on reviewer comments.

• Remove spurious semicolon.
• Add tests about plain char.
• Extend documentation describing how to control char signdness.

[ztamas]

Add newlines to the end of the files.

[ztamas]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

44–45

It catches plain char too. I tested on 64 bit linux where char is signed by default.
The funsigned-char and fsigned-char options can be used to override the default behavior. Added some new test cases and also extended the documentation to make this clear.

44–45

Most of the catches I found in LibreOffice / LLVM code had char type.
I found isSignedCharDefault() method in LLVM code where clang handles platform differences, as I see, so I assume clang-tidy works based on that. With the compilation options, the char signedness can be controlled anyway, so I think this will be OK.
I could test only with a 64 bit linux. There plain char is caught by default, as I mentioned, and I could change this behavior with the -funsigned-char option.

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

26

This check does not cover the whole CERT description. I guess a cert check should catch all bugous code related to the CERT issue, right?

[ztamas]

I added the above comments two weeks ago, I just forget to push the submit button.

[sylvestre.ledru]

clang-tools-extra/docs/clang-tidy/checks/list.rst

404

list.rst has changed, you should try to rebase your patch!

[ztamas]

Rebase patch.

[ztamas]

clang-tools-extra/docs/clang-tidy/checks/list.rst

404

I rebased the patch. I added medium severity for this check because the CERT rule has the same severity.

[aaron.ballman]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

98–99

How about: 'signed char' to %0 conversion; consider casting to 'unsigned char' first?

Also, should we have a fix-it to automatically insert the cast to unsigned char in the proper location for the user?

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

22

Backticks around the command line options.

24

By now -> Currently

26

So long as this covers a decent amount of the CERT rule, I think it is fine to document the areas where we don't quite match the CERT rule.

26

bugous -> bogus

[ztamas]

Update docs / warning message, according to reviewer comments.

[ztamas]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

98–99

I don't think it's a good idea to add a cast automatically. I think this kind of issue typically needs a human programmer to check what can be done in the code.
For example, when the program uses a char variable, but it is used as an int intentionally (without adding an int alias). It's not a good practice of course but it can happen. In this case, the best would be to use an int variable instead. Adding a cast might break the code if it's called with negative values.
It also can happen that the code works with actual characters, but handles the negative values on its own. In this case, it might be not enough to add an unsigned char cast, but it might be needed to adjust the surrounding code too.
All in all, based on my experience with the findings in the LibreOffice code base, I would not use an automatic correction here.

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

26

My plan is to extend this check with other use cases in the next months. Can we get back to this question after that? Now, It's hard to tell how well it covers the CERT rule.

[aaron.ballman]

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

98–99

Okay, that makes sense, thank you!

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

26

I checked and I think it covers the rule pretty closely. However, if you're already planning changes to cover new use cases, I'm fine waiting to add the alias.

clang-tools-extra/docs/clang-tidy/checks/list.rst

10

Just an FYI, but the severity columns were recently removed, so you'll probably have to rebase your patch.

clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-fsigned-char.cpp

7

I think you need to update all the diagnostic text in the tests as the diagnostic has changed.

[ztamas]

Update warning message in test files and rebase patch.

[aaron.ballman]

LGTM!

[ztamas]

Thanks for the review!
I applied for the renewal of my commit access. I had commit access only for the SVN repo. After I get access to the GitHub repo I'll push this change.

[lebedev.ri]

A little bit late to the party, but still.
I'm honestly wondering if this should be a proper clang static analyzer data-flow aware check.
This is basically diagnosing every signed char -> signed int promotion, regardless of whether
the char is used as an ASCII char or not. It really seems like a wrong, noise-filled approach..

[ztamas]

In D71174#1815830, @lebedev.ri wrote:

A little bit late to the party, but still.
I'm honestly wondering if this should be a proper clang static analyzer data-flow aware check.
This is basically diagnosing every signed char -> signed int promotion, regardless of whether
the char is used as an ASCII char or not. It really seems like a wrong, noise-filled approach..

I've considered creating a static analyzer check instead before I implemented this clang-tidy check
and it seemed to me path analysis would not help too much. At least in those cases, what I found
in LibreOffice codebase, there was no additional knowledge in the code about the character's value range.
(Actually, in one use case the method name contained "ASCII", but this information can't be used by the
static analyzer I guess.)
For example, when the code reads a file it's not known whether the file will contain only ASCII characters
or not. Or if you have a function working with characters (without having any explicit check in it about
character value range), then the static analyzer won't help there anyway.
I'm not sure how many test cases can be filtered out with a static analyzer check in practice, but I feel like
it's not much more effective than a clang-tidy check in this case. As I recall path analysis stops working if it
reaches a loop, right? It loses the information from before the loop or something like that, which makes a static
analyzer check even less useful, because characters are rare to go alone.

[sylvestre.ledru]

In D71174#1774249, @ztamas wrote:

I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore = "sal_Int8".
The check produced 32 findings.

Interesting. It found 31 issues on the Firefox code base!
I can share the list if you are interested!

[ztamas]

In D71174#1818883, @sylvestre.ledru wrote:

In D71174#1774249, @ztamas wrote:

I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore = "sal_Int8".
The check produced 32 findings.

Interesting. It found 31 issues on the Firefox code base!
I can share the list if you are interested!

Yes, share it, please! It would be very useful to see how the check works on another project.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
+#include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
     CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
         "bugprone-not-null-terminated-result");
     CheckFactories.registerCheck<ParentVirtualCallCheck>(
         "bugprone-parent-virtual-call");
     CheckFactories.registerCheck<PosixReturnCheck>(
         "bugprone-posix-return");
+    CheckFactories.registerCheck<SignedCharMisuseCheck>(
+        "bugprone-signed-char-misuse");
     CheckFactories.registerCheck<SizeofContainerCheck>(
         "bugprone-sizeof-container");
     CheckFactories.registerCheck<SizeofExpressionCheck>(
         "bugprone-sizeof-expression");
     CheckFactories.registerCheck<StringConstructorCheck>(
         "bugprone-string-constructor");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "bugprone-string-integer-assignment");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   MacroRepeatedSideEffectsCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
   NotNullTerminatedResultCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
+  SignedCharMisuseCheck.cpp
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousEnumUsageCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   SuspiciousMissingCommaCheck.cpp

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h

+//===--- SignedCharMisuseCheck.h - clang-tidy -------------------*- C++ -*-===//
+//
+// 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_SIGNEDCHARMISUSECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIGNEDCHARMISUSECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds ``signed char`` -> integer conversions which might indicate a programming
+/// error. The basic problem with the ``signed char``, that it might store the
+/// non-ASCII characters as negative values. The human programmer probably
+/// expects that after an integer conversion the converted value matches with the
+/// character code (a value from [0..255]), however, the actual value is in
+/// [-128..127] interval. This also applies to the plain ``char`` type on
+/// those implementations which represent ``char`` similar to ``signed char``.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-signed-char-misuse.html
+class SignedCharMisuseCheck : public ClangTidyCheck {
+public:
+  SignedCharMisuseCheck(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;
+
+private:
+  const std::string CharTypdefsToIgnoreList;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIGNEDCHARMISUSECHECK_H

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

+//===--- SignedCharMisuseCheck.cpp - clang-tidy ---------------------------===//
+//
+// 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 "SignedCharMisuseCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static Matcher<TypedefDecl> hasAnyListedName(const std::string &Names) {
+  const std::vector<std::string> NameList =
+      utils::options::parseStringList(Names);
+  return hasAnyName(std::vector<StringRef>(NameList.begin(), NameList.end()));
+}
+
+SignedCharMisuseCheck::SignedCharMisuseCheck(StringRef Name,
+                                             ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")) {}
+
+void SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList);
+}
+
+void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+  // We can ignore typedefs which are some kind of integer types
+  // (e.g. typedef char sal_Int8). In this case, we don't need to
+  // worry about the misinterpretation of char values.
+  const auto IntTypedef = qualType(
+      hasDeclaration(typedefDecl(hasAnyListedName(CharTypdefsToIgnoreList))));
+
+  const auto SignedCharType = expr(hasType(qualType(
+      allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef)))));
+
+  const auto IntegerType = qualType(allOf(isInteger(), unless(isAnyCharacter()),
+                                          unless(booleanType())))
+                               .bind("integerType");
+
+  // We are interested in signed char -> integer conversion.
+  const auto ImplicitCastExpr =
+      implicitCastExpr(hasSourceExpression(SignedCharType),
+                       hasImplicitDestinationType(IntegerType))
+          .bind("castExpression");
+
+  const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
+  const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
+  const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
+
+  // We catch any type of casts to an integer. We need to have these cast
+  // expressions explicitly to catch only those casts which are direct children
+  // of an assignment/declaration.
+  const auto CastExpr = expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
+                                   StaticCastExpr, FunctionalCastExpr));
+
+  // Catch assignments with the suspicious type conversion.
+  const auto AssignmentOperatorExpr = expr(binaryOperator(
+      hasOperatorName("="), hasLHS(hasType(IntegerType)), hasRHS(CastExpr)));
+
+  Finder->addMatcher(AssignmentOperatorExpr, this);
+
+  // Catch declarations with the suspicious type conversion.
+  const auto Declaration =
+      varDecl(isDefinition(), hasType(IntegerType), hasInitializer(CastExpr));
+
+  Finder->addMatcher(Declaration, this);
+}
+
+void SignedCharMisuseCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *CastExpression =
+      Result.Nodes.getNodeAs<ImplicitCastExpr>("castExpression");
+  const auto *IntegerType = Result.Nodes.getNodeAs<QualType>("integerType");
+  assert(CastExpression);
+  assert(IntegerType);
+
+  // Ignore the match if we know that the value is not negative.
+  // The potential misinterpretation happens for negative values only.
+  Expr::EvalResult EVResult;
+  if (!CastExpression->isValueDependent() &&
+      CastExpression->getSubExpr()->EvaluateAsInt(EVResult, *Result.Context)) {
+    llvm::APSInt Value1 = EVResult.Val.getInt();
+    if (Value1.isNonNegative())
+      return;
+  }
+
+  diag(CastExpression->getBeginLoc(),
+       "'signed char' to %0 conversion; "
+       "consider casting to 'unsigned char' first.")
+      << *IntegerType;
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-not-null-terminated-result
   <clang-tidy/checks/bugprone-not-null-terminated-result>` check
 
   Finds function calls where it is possible to cause a not null-terminated
   result. Usually the proper length of a string is ``strlen(str) + 1`` or equal
   length of this expression, because the null terminator needs an extra space.
   Without the null terminator it can result in undefined behaviour when the
   string is read.
 
+- New :doc:`bugprone-signed-char-misuse
+  <clang-tidy/checks/bugprone-signed-char-misuse>` check.
+
+  Finds ``signed char`` -> integer conversions which might indicate a programming
+  error.
+
 - New :doc:`cert-mem57-cpp
   <clang-tidy/checks/cert-mem57-cpp>` check.
 
   Checks if an object of type with extended alignment is allocated by using
   the default ``operator new``.
 
 - New alias :doc:`cert-pos44-c
   <clang-tidy/checks/cert-pos44-c>` to

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst

+.. title:: clang-tidy - bugprone-signed-char-misuse
+
+bugprone-signed-char-misuse
+===========================
+
+Finds ``signed char`` -> integer conversions which might indicate a programming
+error. The basic problem with the ``signed char``, that it might store the
+non-ASCII characters as negative values. The human programmer probably
+expects that after an integer conversion the converted value matches with the
+character code (a value from [0..255]), however, the actual value is in
+[-128..127] interval. This also applies to the plain ``char`` type on
+those implementations which represent ``char`` similar to ``signed char``.
+
+To avoid this kind of misinterpretation, the desired way of converting from a
+``signed char`` to an integer value is converting to ``unsigned char`` first,
+which stores all the characters in the positive [0..255] interval which matches
+with the known character codes.
+
+It depends on the actual platform whether ``char`` is handled as ``signed char``
+by default and so it is caught by this check or not. To change the default behavior
+you can use ``-funsigned-char`` and ``-fsigned-char`` compilation options.
+
+Currently, this check is limited to assignments and variable declarations,
+where a ``signed char`` is assigned to an integer variable. There are other
+use cases where the same misinterpretation might lead to similar bogus
+behavior.
+
+See also:
+`STR34-C. Cast characters to unsigned char before converting to larger integer sizes
+<https://wiki.sei.cmu.edu/confluence/display/c/STR34-C.+Cast+characters+to+unsigned+char+before+converting+to+larger+integer+sizes>`_
+
+A good example from the CERT description when a ``char`` variable is used to
+read from a file that might contain non-ASCII characters. The problem comes
+up when the code uses the ``-1`` integer value as EOF, while the 255 character
+code is also stored as ``-1`` in two's complement form of char type.
+See a simple example of this bellow. This code stops not only when it reaches
+the end of the file, but also when it gets a character with the 255 code.
+
+.. code-block:: c++
+
+  #define EOF (-1)
+
+  int read(void) {
+    char CChar;
+    int IChar = EOF;
+
+    if (readChar(CChar)) {
+      IChar = CChar;
+    }
+    return IChar;
+  }
+
+A proper way to fix the code above is converting the ``char`` variable to
+an ``unsigned char`` value first.
+
+.. code-block:: c++
+
+  #define EOF (-1)
+
+  int read(void) {
+    char CChar;
+    int IChar = EOF;
+
+    if (readChar(CChar)) {
+      IChar = static_cast<unsigned char>(CChar);
+    }
+    return IChar;
+  }
+
+.. option:: CharTypdefsToIgnore
+
+  A semicolon-separated list of typedef names. In this list, we can list
+  typedefs for ``char`` or ``signed char``, which will be ignored by the
+  check. This is useful when a typedef introduces an integer alias like
+  ``sal_Int8`` or ``int8_t``. In this case, human misinterpretation is not
+  an issue.

clang-tools-extra/docs/clang-tidy/checks/list.rst

 .. title:: clang-tidy - Clang-Tidy Checks
 
 Clang-Tidy Checks
 =================
 
 .. csv-table::
    :header: "Name", "Offers fixes"
    :widths: 50, 20
 
    `abseil-duration-addition <abseil-duration-addition.html>`_, "Yes"
    `abseil-duration-comparison <abseil-duration-comparison.html>`_, "Yes"
    `abseil-duration-conversion-cast <abseil-duration-conversion-cast.html>`_, "Yes"
    `abseil-duration-division <abseil-duration-division.html>`_, "Yes"
    `abseil-duration-factory-float <abseil-duration-factory-float.html>`_, "Yes"
    `abseil-duration-factory-scale <abseil-duration-factory-scale.html>`_, "Yes"
    `abseil-duration-subtraction <abseil-duration-subtraction.html>`_, "Yes"
    `abseil-duration-unnecessary-conversion <abseil-duration-unnecessary-conversion.html>`_, "Yes"
    `abseil-faster-strsplit-delimiter <abseil-faster-strsplit-delimiter.html>`_, "Yes"
.. csv-table::
    `bugprone-macro-repeated-side-effects <bugprone-macro-repeated-side-effects.html>`_,
    `bugprone-misplaced-operator-in-strlen-in-alloc <bugprone-misplaced-operator-in-strlen-in-alloc.html>`_,
    `bugprone-misplaced-widening-cast <bugprone-misplaced-widening-cast.html>`_, "Yes"
    `bugprone-move-forwarding-reference <bugprone-move-forwarding-reference.html>`_, "Yes"
    `bugprone-multiple-statement-macro <bugprone-multiple-statement-macro.html>`_,
    `bugprone-not-null-terminated-result <bugprone-not-null-terminated-result.html>`_, "Yes"
    `bugprone-parent-virtual-call <bugprone-parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone-posix-return.html>`_, "Yes"
+   `bugprone-signed-char-misuse <bugprone-signed-char-misuse.html>`_,
    `bugprone-sizeof-container <bugprone-sizeof-container.html>`_,
    `bugprone-sizeof-expression <bugprone-sizeof-expression.html>`_,
    `bugprone-string-constructor <bugprone-string-constructor.html>`_, "Yes"
    `bugprone-string-integer-assignment <bugprone-string-integer-assignment.html>`_, "Yes"
    `bugprone-string-literal-with-embedded-nul <bugprone-string-literal-with-embedded-nul.html>`_,
    `bugprone-suspicious-enum-usage <bugprone-suspicious-enum-usage.html>`_,
    `bugprone-suspicious-memset-usage <bugprone-suspicious-memset-usage.html>`_, "Yes"
    `bugprone-suspicious-missing-comma <bugprone-suspicious-missing-comma.html>`_,
.. csv-table:: Aliases..
    `hicpp-use-equals-default <hicpp-use-equals-default.html>`_, `modernize-use-equals-default <modernize-use-equals-default.html>`_, "Yes"
    `hicpp-use-equals-delete <hicpp-use-equals-delete.html>`_, `modernize-use-equals-delete <modernize-use-equals-delete.html>`_, "Yes"
    `hicpp-use-noexcept <hicpp-use-noexcept.html>`_, `modernize-use-noexcept <modernize-use-noexcept.html>`_, "Yes"
    `hicpp-use-nullptr <hicpp-use-nullptr.html>`_, `modernize-use-nullptr <modernize-use-nullptr.html>`_, "Yes"
    `hicpp-use-override <hicpp-use-override.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
    `hicpp-vararg <hicpp-vararg.html>`_, `cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_,
 
 .. toctree::
    :glob:
    :hidden:
 
    *

clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-fsigned-char.cpp

+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t -- -- -fsigned-char
+
+int PlainChar() {
+  char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}

clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-funsigned-char.cpp

+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t -- -- -funsigned-char
+
+// Test framework needs something to catch, otherwise it fails.
+int SignedChar() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int PlainChar() {
+  char CCharacter = -5;
+  int NCharacter = CCharacter; // no warning
+  return NCharacter;
+}

clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp

+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: bugprone-signed-char-misuse.CharTypdefsToIgnore, value: "sal_Int8;int8_t"}]}' \
+// RUN: --
+
+///////////////////////////////////////////////////////////////////
+/// Test cases correctly caught by the check.
+
+// Check that a simple test case is still caught.
+int SimpleAssignment() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+typedef signed char sal_Char;
+
+int TypedefNotInIgnorableList() {
+  sal_Char CCharacter = 'a';
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+///////////////////////////////////////////////////////////////////
+/// Test cases correctly ignored by the check.
+
+typedef signed char sal_Int8;
+
+int OneIgnorableTypedef() {
+  sal_Int8 CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+typedef signed char int8_t;
+
+int OtherIgnorableTypedef() {
+  int8_t CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+///////////////////////////////////////////////////////////////////
+/// Test cases which should be caught by the check.
+
+namespace boost {
+
+template <class T>
+class optional {
+  T *member;
+
+public:
+  optional(T value) {
+    member = new T(value);
+  }
+
+  T operator*() { return *member; }
+};
+
+} // namespace boost
+
+int DereferenceWithTypdef(boost::optional<sal_Int8> param) {
+  int NCharacter = *param;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}

clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t
+
+///////////////////////////////////////////////////////////////////
+/// Test cases correctly caught by the check.
+
+int SimpleVarDeclaration() {
+  signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int SimpleAssignment() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CStyleCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = (int)CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int StaticCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = static_cast<int>(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:33: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int FunctionalCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = int(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int NegativeConstValue() {
+  const signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CharPointer(signed char *CCharacter) {
+  int NCharacter = *CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+///////////////////////////////////////////////////////////////////
+/// Test cases correctly ignored by the check.
+
+int UnsignedCharCast() {
+  unsigned char CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+int PositiveConstValue() {
+  const signed char CCharacter = 5;
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of declaration expression.
+int DescendantCast() {
+  signed char CCharacter = 'a';
+  int NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of assignment expression.
+int DescendantCastAssignment() {
+  signed char CCharacter = 'a';
+  int NCharacter;
+  NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolVarDeclaration() {
+  signed char CCharacter = 'a';
+  bool BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolAssignment() {
+  signed char CCharacter = 'a';
+  bool BCharacter;
+  BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// char is an integer type in clang; make sure to ignore it.
+unsigned char CharToCharCast() {
+  signed char SCCharacter = 'a';
+  unsigned char USCharacter;
+  USCharacter = SCCharacter;
+
+  return USCharacter;
+}