[clang-tidy] Add check for implicit widening of multiplication result

[lebedev.ri]

Summary

Overflows are never fun.
In most cases (in most of the code), they are rare,
because usually you e.g. don't have as many elements.

However, it's exceptionally easy to fall into this pitfail
in code that deals with images, because, assuming 4-channel 32-bit FP data,
you need *just* ~269 megapixel image to case an overflow
when computing at least the total byte count.

In darktable, there is a *long*, painful history of dealing with such bugs:

• https://github.com/darktable-org/darktable/pull/7740
• https://github.com/darktable-org/darktable/pull/7419
• https://github.com/darktable-org/darktable/commit/eea1989f2c9fa76710db07baaec4c19c1e40e81c
• https://github.com/darktable-org/darktable/commit/70626dd95bf0fab36f2d011dab075e3ebbf7aa28
• https://github.com/darktable-org/darktable/pull/670
• https://github.com/darktable-org/darktable/commit/38c69fb1b2bc90057c569242cb9945a10be0b583

and yet they clearly keep resurfacing still.

It would be immensely helpful to have a diagnostic for those patterns,
which is what this change proposes.

Currently, i only diagnose the most obvious case, where multiplication
is directly widened with no other expressions inbetween,
(i.e. long r = (int)a * (int)b but not even e.g. long r = ((int)a * (int)b))
however that might be worth relaxing later.

[lebedev.ri]

• Properly handle dependent types (bailout instead of crashing)
• Add ASTContext::getCorrespondingSignedType()
• Add fix-it for the unsigned long r = int(x) * int(y) case

[dblaikie]

Haven't reviewed the code in detail, but the idea seems sound. Is there any prior art in other compilers you can draw data/experiences from? & having some data from some reasonably sized codebases on false positive (how often the correct thing is to suppress the warning, versus fixing the warning) would probably be useful. I would guess it doesn't meet the on-by-default bar we would prefer for Clang, but might still be OK.

clang/docs/ReleaseNotes.rst

64–74 ↗

(On Diff #313788)

Does this only trigger when the sizeof(char*) > sizeof(int)? (judging by the test coverage I think that's the case)

(ultimately, might be worth committing the two diagnostics separately - usual sort of reasons, separation of concerns, etc)

clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c

24 ↗

(On Diff #313788)

Question is unclear - is there uncertainty about whether this should be tested? Or is this a false negative case? In which case I'd probably include the test showing no diagnostic and mention it could be fixed/improved?

[lebedev.ri]

Thank you for taking a look!

In D93822#2474236, @dblaikie wrote:

Haven't reviewed the code in detail, but the idea seems sound.

That's reassuring!

Is there any prior art in other compilers you can draw data/experiences from?

As far as i can tell, no other compiler diagnoses these cases,
however e.g. GitHub's CodeQL static analysis tool complains about this by default:
https://github.com/darktable-org/rawspeed/security/code-scanning/7?query=342efe49c0bef4bf2450051586d899f9d303ae62

& having some data from some reasonably sized codebases on false positive
(how often the correct thing is to suppress the warning, versus fixing the warning)
would probably be useful.

I don't have those numbers presently (other than the long long diffs of fixes for true-positives i have already linked),
but as a rule of thumb, iff a particular project in question
never allocates more than UINT_MAX bytes / UINT_MAX / sizeof(element) elements,
*all* issued diagnostics for said project *will* be false-positives by definition.
So YMMV.

I would guess it doesn't meet the on-by-default bar we would prefer for Clang,

Yeah, based on my initial observations, it's pretty chatty.

but might still be OK.

I'm open to input here, but it would be good to have this at least in -Wextra (-Weverything always being a last resort option).

clang/docs/ReleaseNotes.rst

64–74 ↗

(On Diff #313788)

Does this only trigger when the sizeof(char*) > sizeof(int)? (judging by the test coverage I think that's the case)

That should be the case, since sizeof(size_t) == sizeof(char*) and
// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify=silent %s

(ultimately, might be worth committing the two diagnostics separately - usual sort of reasons, separation of concerns, etc)

Should i split this into two reviews to begin with?

clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c

24 ↗

(On Diff #313788)

I may be misremembering things, but IIRC a[b] and b[a] is the same thing,
but i'm not sure how to exercise the second spelling.
I.e. i'm just not sure how to write a test for it.

[dblaikie]

In D93822#2474355, @lebedev.ri wrote:

Thank you for taking a look!

In D93822#2474236, @dblaikie wrote:

Haven't reviewed the code in detail, but the idea seems sound.

That's reassuring!

Is there any prior art in other compilers you can draw data/experiences from?

As far as i can tell, no other compiler diagnoses these cases,
however e.g. GitHub's CodeQL static analysis tool complains about this by default:
https://github.com/darktable-org/rawspeed/security/code-scanning/7?query=342efe49c0bef4bf2450051586d899f9d303ae62

& having some data from some reasonably sized codebases on false positive
(how often the correct thing is to suppress the warning, versus fixing the warning)
would probably be useful.

I don't have those numbers presently (other than the long long diffs of fixes for true-positives i have already linked),
but as a rule of thumb, iff a particular project in question
never allocates more than UINT_MAX bytes / UINT_MAX / sizeof(element) elements,
*all* issued diagnostics for said project *will* be false-positives by definition.
So YMMV.

I would guess it doesn't meet the on-by-default bar we would prefer for Clang,

Yeah, based on my initial observations, it's pretty chatty.

but might still be OK.

I'm open to input here, but it would be good to have this at least in -Wextra (-Weverything always being a last resort option).

Yeah, not sure how @rsmith, @rtrieu, or @aaron.ballman feel about what the bar for warnings is these days - used to be a bit more hardcore about the "if it can't be on by default it shouldn't be in clang" than we are these days, I think. I'd guess -Wextra might be suitable.

clang/docs/ReleaseNotes.rst

64–74 ↗

(On Diff #313788)

Should i split this into two reviews to begin with?

Nah, doubt it's worth splitting up right now - see how a few folks feel about the idea in general, and then if there's enough mechanical details to discuss might be worth splitting out one on top of the other.

clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c

24 ↗

(On Diff #313788)

I may be misremembering things, but IIRC a[b] and b[a] is the same thing,

Yep, that's the case - a[b] where one of them is a pointer and teh other is an integer, is equivalent to *(a + b), so that means it's the same as b[a].

I guess you'd write it the same as t0, but with the expressions reversed? return *(a * b)[base];

[lebedev.ri]

NFC, rebased, added test with multiplication of shorts.

clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c

24 ↗

(On Diff #313788)

That's what i tried, and it does't work - https://godbolt.org/z/as4vP4

[dblaikie]

clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c

24 ↗

(On Diff #313788)

Sorry, my mistake - the * was wrong. This works: https://godbolt.org/z/j9jqx3

[lebedev.ri]

• Add test for inverse array subscript expression
• Support it by skipping paren expressions
• Actually ensure that we only diagnose only truly implicit casts, but not implicit casts that are implicit steps of an explicit cast

clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c

24 ↗

(On Diff #313788)

Aha, thanks.

[lebedev.ri]

Ping

In D93822#2474475, @dblaikie wrote:

In D93822#2474355, @lebedev.ri wrote:

Thank you for taking a look!

In D93822#2474236, @dblaikie wrote:

I would guess it doesn't meet the on-by-default bar we would prefer for Clang,

Yeah, based on my initial observations, it's pretty chatty.

but might still be OK.

I'm open to input here, but it would be good to have this at least in -Wextra (-Weverything always being a last resort option).

Yeah, not sure how @rsmith, @rtrieu, or @aaron.ballman feel about what the bar for warnings is these days - used to be a bit more hardcore about the "if it can't be on by default it shouldn't be in clang" than we are these days, I think. I'd guess -Wextra might be suitable.

[aaron.ballman]

In D93822#2474475, @dblaikie wrote:

In D93822#2474355, @lebedev.ri wrote:

I'm open to input here, but it would be good to have this at least in -Wextra (-Weverything always being a last resort option).

Yeah, not sure how @rsmith, @rtrieu, or @aaron.ballman feel about what the bar for warnings is these days - used to be a bit more hardcore about the "if it can't be on by default it shouldn't be in clang" than we are these days, I think. I'd guess -Wextra might be suitable.

I'd like to see data before making a determination, but my gut instinct is that this diagnostic will be far too chatty to be in Clang (even in -Wextra) but that this would be a great flow-sensitive check for the clang static analyzer (I've added some analyzer folks to the thread for their input). Static analyzers frequently implement this functionality because they can more easily notice things that limit the range of values possible for a given object and use that extra information when deciding whether overflow is likely or not. e.g., if the analyzer encounters if (foo > 10) return; in a method then it can understand that subsequent uses of foo after that statement are bounded such that foo <= 10. I think we'll ultimately need this flow sensitivity to reduce the false positive rate to something more useful for math-heavy code bases.

Assuming that the data shows a high false positive rate, if you don't have the appetite to work on the robust check in the static analyzer, another option would be to implement this functionality in clang-tidy (which is allowed to be more chatty than the Clang frontend or the static analyzer in terms of false positives). However, I still think we eventually will want the static analyzer check and so my preference is for that approach (so that we don't wind up needing to carry around two implementations of effectively the same check).

[NoQ]

this would be a great flow-sensitive check for the clang static analyzer

You could indeed use static analyzer's path sensitivity to get rid of these obvious false positives where the multiplication is performed pretty much immediately after a branch (or assert) that avoids overflow, where "pretty much immediately" requires the static analyzer to encounter the overflow check while interpreting the function that contains the potential overflow. It's still not going to be good enough to be a good on-by-default checker because such checks may be performed either before the function is invoked, or in a nested function call for which the body is not immediately available in the translation unit. A good on-by-default checker could be achieved if taint analysis is added to the mixture (i.e., only check for overflows of values that are known to definitely be arbitrary) but that'd limit the power of the checker dramatically because such knowledge is rarely available (you may add annotations for that but that's probably a lot of work in your code).

Warnings with false positives may still be useful when they define a clear coding convention that you want the users to follow. In your case such convention seems to be "always use integer types that are wide enough to avoid overflows" and it says nothing about overflow checks being made, so i guess you don't absolutely need to bother with flow/path sensitivity. I think the answer should be data-driven. Does your codebase already contain overflow checks that cause false positives all over the place? If so, static analyzer to the rescue. If most of your multiplications are unchecked and you already rely on the integer type to be wide enough then you're probably good as-is.

[lebedev.ri]

@aaron.ballman thank you for taking a look!
@NoQ thank you for commenting.

Indeed, i very much don't want to diagnose just the cases
where the overflow can be proven to happen, doing that
i believe, would remove most of the true-positive reports.

So indeed, this is beginning to look like a coding guideline,
so let's circumvent all this FUD, and move onto clang-tidy.

[lebedev.ri]

Hmm. I kinda dropped the ball here.
As discussed, move it into a clang-tidy check.
Looks good now?

[aaron.ballman]

The CI is showing build failures and there are some clang-tidy nits to be addressed as well.

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

29

I think we should skip parens at the very least. x * y should check the same as (x) * (y).

32

I think that could be handled in a follow-up if we find the need, WDYT?

75

Might be worth it to have tests around signed char, unsigned char, and char explicitly, as that gets awkward.

126

You already are using the way?

clang/lib/AST/ASTContext.cpp

10158

This looks to be getting the same value as in the getCorrespondingUnsignedType() call?

10204

It looks like _ExtInt is missing from both this function and the corresponding unsigned one.

[lebedev.ri]

In D93822#2664157, @aaron.ballman wrote:

The CI is showing build failures and there are some clang-tidy nits to be addressed as well.

Thank you for taking a look!

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

32

Sure.

75

Are you asking for test coverage, or for explicit handling of those types?
I'll add tests.

clang/lib/AST/ASTContext.cpp

10158

Yep. Are both of them incorrect?
I'm not sure what should be returned here.

10204

Likewise, i'm not sure what should be returned for that.
Just the original _ExtInt?

[lebedev.ri]

Addressed review comments.
Thanks!

[aaron.ballman]

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

68

We might want to consider letting the user select between static_cast and C-style casts via an option.

75

Just test coverage to make sure we're doing the right thing for them and not triggering these assertions.

131

One thing that's awkward about this is that there's no portable ssize_t type -- that's a POSIX type but it doesn't exist on all platforms (like Windows). We shouldn't print out a typecast that's going to cause compile errors, but we also shouldn't use the underlying type for ssize_t as that may be incorrect for other target architectures.

clang-tools-extra/docs/clang-tidy/checks/bugprone-implicit-widening-of-multiplication-result.rst

7–10

I think the documentation would be stronger if it explained why this diagnostic is helpful (show some examples of bugs that it catches and that the suggested fixes remove the bug).

clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-char.cpp

10 ↗

(On Diff #334929)

Can you also test the fixit behavior in addition to the diagnostic behavior (at least one test for each kind of fix-it)?

clang/lib/AST/ASTContext.cpp

10158

I think I confused myself on the code, I think both are correct.

10204

_ExtInt has signed and unsigned variants. You can use ASTContext::getExtIntType() to get the type with the correct signedness. However, I see now that it's not a builtin type (huh, I thought it was!), so it'd have to be above the switch but after the enum handling.

[lebedev.ri]

@aaron.ballman thank you for taking a look!
Addressing all review notes other than ssize_t and static_cast<>().

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

29

This is more about the future case for which there's FIXME few lines down.

75

It seems to kinda just work, because of the promotion to int.

126

No, that's not it, because SizeTy.getAsString() will not be size_t/ssize_t, but long/etc.

clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-char.cpp

10 ↗

(On Diff #334929)

I would love to do that, but as we have briefly talked with @njames93,
fix-it testing in clang-tidy, as compared to clang proper,
is rudimentary. I basically can not add tests here.
As far as i can see, it doesn't want to apply fix-its, because there are two of them.

So this is the best i can do, take it or leave it. /s

clang/lib/AST/ASTContext.cpp

10158

Actually, this is correct.
Note that we don't return it, but the later code will flip it's sign.

[aaron.ballman]

FWIW, it looks like tests are still failing on Windows according to the CI pipeline.

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

126

Ah, I see what you mean now, thanks.

131

I'm still not quite certain what to do about this. Would it make sense to use the underlying type on platforms that don't have ssize_t? Relatedly, if we're going to suggest this as a replacement, we should also insert an include for the correct header file.

clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-char.cpp

10 ↗

(On Diff #334929)

Ah, thank you for clarifying the issue that the two fixits make it hard to test.

[lebedev.ri]

@aaron.ballman thank you for taking a look!
Addressed all review notes.

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

131

I've been thinking about this, and i really can't come up with a better fix than using ptrdiff_t.

[lebedev.ri]

Hardcode triple/target in tests, should fix 32-bit CI.

[aaron.ballman]

LGTM aside from some small nits in the documentation.

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

131

I'm not super excited about using ptrdiff_t as there are not likely to be pointer subtractions in the vicinity of the code being diagnosed, but at the same time, ptrdiff_t is functionally the same as size_t except with a sign bit, which is what ssize_t is. I'll hold my nose for now, but if we get bug reports about this use, we may want to investigate more involved solutions.

clang-tools-extra/docs/clang-tidy/checks/bugprone-implicit-widening-of-multiplication-result.rst

41

Add info about the default behavior.

46

Same here.

[lebedev.ri]

Address final nits.

[lebedev.ri]

@aaron.ballman thank you so much for the review!

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

131

Yep. signed size_t isn't a valid type, so at best i guess we could avoid emitting such a fixit.
In C++, std::make_signed<size_t>::type is valid, but it's kinda mouthful.

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

 #include "BranchCloneCheck.h"
 #include "CopyConstructorInitCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DynamicStaticInitializersCheck.h"
 #include "ExceptionEscapeCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
 #include "ForwardingReferenceOverloadCheck.h"
+#include "ImplicitWideningOfMultiplicationResultCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncorrectRoundingsCheck.h"
 #include "InfiniteLoopCheck.h"
 #include "IntegerDivisionCheck.h"
 #include "LambdaFunctionNameCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ExceptionEscapeCheck>(
         "bugprone-exception-escape");
     CheckFactories.registerCheck<FoldInitTypeCheck>(
         "bugprone-fold-init-type");
     CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
         "bugprone-forward-declaration-namespace");
     CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
         "bugprone-forwarding-reference-overload");
+    CheckFactories.registerCheck<ImplicitWideningOfMultiplicationResultCheck>(
+        "bugprone-implicit-widening-of-multiplication-result");
     CheckFactories.registerCheck<InaccurateEraseCheck>(
         "bugprone-inaccurate-erase");
     CheckFactories.registerCheck<IncorrectRoundingsCheck>(
         "bugprone-incorrect-roundings");
     CheckFactories.registerCheck<InfiniteLoopCheck>(
         "bugprone-infinite-loop");
     CheckFactories.registerCheck<IntegerDivisionCheck>(
         "bugprone-integer-division");

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

add_clang_library(clangTidyBugproneModule
   BugproneTidyModule.cpp
   CopyConstructorInitCheck.cpp
   DanglingHandleCheck.cpp
   DynamicStaticInitializersCheck.cpp
   ExceptionEscapeCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   ForwardingReferenceOverloadCheck.cpp
+  ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
   IncorrectRoundingsCheck.cpp
   InfiniteLoopCheck.cpp
   IntegerDivisionCheck.cpp
   LambdaFunctionNameCheck.cpp
   MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp

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

+//===--- ImplicitWideningOfMultiplicationResultCheck.h ----------*- 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_IMPLICITWIDENINGOFMULTIPLICATIONRESULTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_IMPLICITWIDENINGOFMULTIPLICATIONRESULTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Diagnoses instances of an implicit widening of multiplication result.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-implicit-widening-of-multiplication-result.html
+class ImplicitWideningOfMultiplicationResultCheck : public ClangTidyCheck {
+  void handleImplicitCastExpr(const ImplicitCastExpr *ICE,
+                                     ASTContext *Context);
+  void handlePointerOffsetting(const Expr *E, ASTContext *Context);
+
+public:
+  ImplicitWideningOfMultiplicationResultCheck(StringRef Name,
+                                              ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_IMPLICITWIDENINGOFMULTIPLICATIONRESULTCHECK_H

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

+//===--- ImplicitWideningOfMultiplicationResultCheck.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 "ImplicitWideningOfMultiplicationResultCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace {
+AST_MATCHER(ImplicitCastExpr, isPartOfExplicitCast) {
+  return Node.isPartOfExplicitCast();
+}
+} // namespace
+} // namespace clang
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static const Expr *GetLHSOfMulBinOp(const Expr *E) {
+  // Is this:  long r = int(x) * int(y);  ?
+  // FIXME: shall we skip brackets/casts/etc?
+  auto *BO = dyn_cast<BinaryOperator>(E);
+  if (!BO || BO->getOpcode() != BO_Mul)
+    // FIXME: what about:  long r = int(x) + (int(y) * int(z));  ?
+    return nullptr;
+  return BO->getLHS();
+}
+
+void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
+    const ImplicitCastExpr *ICE, ASTContext *Context) {
+  const Expr *E = ICE->getSubExpr()->IgnoreParens();
+  QualType Ty = ICE->getType();
+  QualType ETy = E->getType();
+
+  assert(!ETy->isDependentType() && !Ty->isDependentType() &&
+         "Don't expect to ever get here in template Context.");
+
+  // This must be a widening cast. Else we do not care.
+  unsigned SrcWidth = Context->getIntWidth(ETy);
+  unsigned TgtWidth = Context->getIntWidth(Ty);
+  if (TgtWidth <= SrcWidth)
+    return;
+
+  // Does the index expression look like it might be unintentionally computed
+  // in a narrower-than-wanted type?
+  const Expr *LHS = GetLHSOfMulBinOp(E);
+  if (!LHS)
+    return;
+
+  // Ok, looks like we should diagnose this.
+  diag(E->getBeginLoc(), "performing an implicit widening conversion to type "
+                         "%0 of a multiplication performed in type %1")
+      << Ty << E->getType();
+
+  diag(E->getBeginLoc(), "make conversion explicit to silence this warning",
+       DiagnosticIDs::Note)
+      << FixItHint::CreateInsertion(E->getBeginLoc(),
+                                    "(" + Ty.getAsString() + ")(")
+      << FixItHint::CreateInsertion(E->getEndLoc(), ")") << E->getSourceRange();
+
+  QualType WideExprTy;
+  // Get Ty of the same signedness as ExprTy, because we only want to suggest
+  // to widen the computation, but not change it's signedness domain.
+  if (Ty->isSignedIntegerType() == ETy->isSignedIntegerType())
+    WideExprTy = Ty;
+  else if (Ty->isSignedIntegerType()) {
+    assert(ETy->isUnsignedIntegerType() &&
+           "Expected source type to be signed.");
+    WideExprTy = Context->getCorrespondingUnsignedType(Ty);
+  } else {
+    assert(Ty->isUnsignedIntegerType() &&
+           "Expected target type to be unsigned.");
+    assert(ETy->isSignedIntegerType() &&
+           "Expected source type to be unsigned.");
+    WideExprTy = Context->getCorrespondingSignedType(Ty);
+  }
+
+  diag(E->getBeginLoc(), "perform multiplication in a wider type",
+       DiagnosticIDs::Note)
+      << LHS->getSourceRange()
+      << FixItHint::CreateInsertion(E->getBeginLoc(),
+                                    "(" + WideExprTy.getAsString() + ")");
+}
+
+void ImplicitWideningOfMultiplicationResultCheck::handlePointerOffsetting(
+    const Expr *E, ASTContext *Context) {
+  // We are looking for a pointer offset operation,
+  // with one hand being a pointer, and another one being an offset.
+  const Expr *PointerExpr, *IndexExpr;
+  if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+    PointerExpr = BO->getLHS();
+    IndexExpr = BO->getRHS();
+  } else if (auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) {
+    PointerExpr = ASE->getLHS();
+    IndexExpr = ASE->getRHS();
+  } else
+    return;
+
+  if (IndexExpr->getType()->isPointerType())
+    std::swap(PointerExpr, IndexExpr);
+
+  if (!PointerExpr->getType()->isPointerType() ||
+      IndexExpr->getType()->isPointerType())
+    return;
+
+  IndexExpr = IndexExpr->IgnoreParens();
+
+  QualType IndexExprType = IndexExpr->getType();
+
+  // If the index expression's type is not known (i.e. we are in a template),
+  // we can't do anything here.
+  if (IndexExprType->isDependentType())
+    return;
+
+  QualType SSizeTy = Context->getSignedSizeType();
+  QualType USizeTy = Context->getSizeType();
+  QualType SizeTy = IndexExprType->isSignedIntegerType() ? SSizeTy : USizeTy;
+  // FIXME: is there a way to actually get the QualType for size_t/ssize_t?
+  StringRef TyAsString =
+      IndexExprType->isSignedIntegerType() ? "ssize_t" : "size_t";
+
+  // So, is size_t actually wider than the result of the multiplication?
+  if (Context->getIntWidth(IndexExprType) >= Context->getIntWidth(SizeTy))
+    return;
+
+  // Does the index expression look like it might be unintentionally computed
+  // in a narrower-than-wanted type?
+  const Expr *LHS = GetLHSOfMulBinOp(IndexExpr);
+  if (!LHS)
+    return;
+
+  // Ok, looks like we should diagnose this.
+  diag(E->getBeginLoc(),
+       "result of multiplication in type %0 is used as a pointer offset after "
+       "an implicit widening conversion to type '%1'")
+      << IndexExprType << TyAsString;
+
+  diag(IndexExpr->getBeginLoc(),
+       "make conversion explicit to silence this warning", DiagnosticIDs::Note)
+      << FixItHint::CreateInsertion(IndexExpr->getBeginLoc(),
+                                    (Twine("(") + TyAsString + ")(").str())
+      << FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")")
+      << IndexExpr->getSourceRange();
+
+  diag(IndexExpr->getBeginLoc(), "perform multiplication in a wider type",
+       DiagnosticIDs::Note)
+      << LHS->getSourceRange()
+      << FixItHint::CreateInsertion(IndexExpr->getBeginLoc(),
+                                    (Twine("(") + TyAsString + ")").str());
+}
+
+void ImplicitWideningOfMultiplicationResultCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(implicitCastExpr(unless(anyOf(isInTemplateInstantiation(),
+                                                   isPartOfExplicitCast())),
+                                      hasCastKind(CK_IntegralCast))
+                         .bind("x"),
+                     this);
+  Finder->addMatcher(
+      arraySubscriptExpr(unless(isInTemplateInstantiation())).bind("x"), this);
+  Finder->addMatcher(binaryOperator(unless(isInTemplateInstantiation()),
+                                    hasType(isAnyPointer()),
+                                    hasAnyOperatorName("+", "-", "+=", "-="))
+                         .bind("x"),
+                     this);
+}
+
+void ImplicitWideningOfMultiplicationResultCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs<ImplicitCastExpr>("x"))
+    handleImplicitCastExpr(MatchedDecl, Result.Context);
+  else if (const auto *MatchedDecl =
+               Result.Nodes.getNodeAs<ArraySubscriptExpr>("x"))
+    handlePointerOffsetting(MatchedDecl, Result.Context);
+  else if (const auto *MatchedDecl =
+               Result.Nodes.getNodeAs<BinaryOperator>("x"))
+    handlePointerOffsetting(MatchedDecl, Result.Context);
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 
 - Added command line option `--fix-notes` to apply fixes found in notes
   attached to warnings. These are typically cases where we are less confident
   the fix will have the desired effect.
 
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-implicit-widening-of-multiplication-result
+  <clang-tidy/checks/bugprone-implicit-widening-of-multiplication-result>` check.
+
+  Diagnoses instances of an implicit widening of multiplication result.
+
 - New :doc:`concurrency-thread-canceltype-asynchronous
   <clang-tidy/checks/concurrency-thread-canceltype-asynchronous>` check.
 
   Finds ``pthread_setcanceltype`` function calls where a thread's cancellation
   type is set to asynchronous.
 
 - New :doc:`altera-unroll-loops
   <clang-tidy/checks/altera-unroll-loops>` check.

clang-tools-extra/docs/clang-tidy/checks/bugprone-implicit-widening-of-multiplication-result.rst

+.. title:: clang-tidy - bugprone-implicit-widening-of-multiplication-result
+
+bugprone-implicit-widening-of-multiplication-result
+===================================================
+
+The check diagnoses instances where a result of a multiplication is implicitly
+widened, and suggests (with fix-it) to either silence the code by making
+widening explicit, or to perform the multiplication in a wider type,
+to avoid the widening afterwards.
+
+Examples:
+
+.. code-block:: c++
+
+  long mul(int a, int b) {
+    return a * b; // warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  }
+
+  char* ptr_add(char *base, int a, int b) {
+    return base + a * b; // warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  }
+
+  char ptr_subscript(char *base, int a, int b) {
+    return base[a * b]; // warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  }

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

.. csv-table::
    `bugprone-branch-clone <bugprone-branch-clone.html>`_,
    `bugprone-copy-constructor-init <bugprone-copy-constructor-init.html>`_, "Yes"
    `bugprone-dangling-handle <bugprone-dangling-handle.html>`_,
    `bugprone-dynamic-static-initializers <bugprone-dynamic-static-initializers.html>`_,
    `bugprone-exception-escape <bugprone-exception-escape.html>`_,
    `bugprone-fold-init-type <bugprone-fold-init-type.html>`_,
    `bugprone-forward-declaration-namespace <bugprone-forward-declaration-namespace.html>`_,
    `bugprone-forwarding-reference-overload <bugprone-forwarding-reference-overload.html>`_,
+   `bugprone-implicit-widening-of-multiplication-result <bugprone-implicit-widening-of-multiplication-result.html>`_, "Yes"
    `bugprone-inaccurate-erase <bugprone-inaccurate-erase.html>`_, "Yes"
    `bugprone-incorrect-roundings <bugprone-incorrect-roundings.html>`_,
    `bugprone-infinite-loop <bugprone-infinite-loop.html>`_,
    `bugprone-integer-division <bugprone-integer-division.html>`_,
    `bugprone-lambda-function-name <bugprone-lambda-function-name.html>`_,
    `bugprone-macro-parentheses <bugprone-macro-parentheses.html>`_, "Yes"
    `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>`_, "Yes"

clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-array-subscript-expression.cpp

+// RUN: %check_clang_tidy -std=c99 %s bugprone-implicit-widening-of-multiplication-result %t -- -- -x c
+// RUN: %check_clang_tidy %s bugprone-implicit-widening-of-multiplication-result %t -- -- -x c++
+
+char *t0(char *base, int a, int b) {
+  return &base[a * b];
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:16: note: perform multiplication in a wider type
+}
+void *t1(char *base, int a, int b) {
+  return &((a * b)[base]);
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:13: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:13: note: perform multiplication in a wider type
+}
+
+char *t2(char *base, unsigned int a, int b) {
+  return &base[a * b];
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:16: note: perform multiplication in a wider type
+}
+
+char *t3(char *base, int a, unsigned int b) {
+  return &base[a * b];
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:16: note: perform multiplication in a wider type
+}
+
+char *t4(char *base, unsigned int a, unsigned int b) {
+  return &base[a * b];
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'
+  // CHECK-NOTES: :[[@LINE-2]]:16: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:16: note: perform multiplication in a wider type
+}
+
+char *n5(char *base, int a, int b, int c) {
+  return &base[a * b + c];
+}
+char *n6(char *base, int a, int b, int c) {
+  return &base[a + b * c];
+}
+
+char *t7(char *base, int a, int b) {
+  return &base[(a * b)];
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:17: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:17: note: perform multiplication in a wider type
+}
+char *n8(char *base, int a, int b, int c) {
+  return &base[(a * b + c)];
+}
+char *n9(char *base, int a, int b, int c) {
+  return &base[(a * b) + c];
+}
+
+char *n10(char *base, int a, int b) {
+  return &base[(long)(a * b)];
+}
+char *n11(char *base, int a, int b) {
+  return &base[(unsigned long)(a * b)];
+}
+
+#ifdef __cplusplus
+template <typename T>
+char *template_test(char *base, T a, T b) {
+  return &base[a * b];
+}
+char *template_test_instantiation(char *base, int a, int b) {
+  return template_test(base, a, b);
+}
+#endif

clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-pointer-offset.cpp

+// RUN: %check_clang_tidy -std=c99 %s bugprone-implicit-widening-of-multiplication-result %t -- -- -x c
+// RUN: %check_clang_tidy %s bugprone-implicit-widening-of-multiplication-result %t -- -- -x c++
+
+char *t0(char *base, int a, int b) {
+  return base + a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:17: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:17: note: perform multiplication in a wider type
+}
+char *t1(char *base, int a, int b) {
+  return a * b + base;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+
+char *t2(char *base, unsigned int a, int b) {
+  return base + a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'
+  // CHECK-NOTES: :[[@LINE-2]]:17: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:17: note: perform multiplication in a wider type
+}
+
+char *t3(char *base, int a, unsigned int b) {
+  return base + a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'
+  // CHECK-NOTES: :[[@LINE-2]]:17: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:17: note: perform multiplication in a wider type
+}
+
+char *t4(char *base, unsigned int a, unsigned int b) {
+  return base + a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'
+  // CHECK-NOTES: :[[@LINE-2]]:17: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:17: note: perform multiplication in a wider type
+}
+
+char *t5(char *base, int a, int b, int c) {
+  return base + a * b + c;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:17: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:17: note: perform multiplication in a wider type
+}
+char *t6(char *base, int a, int b, int c) {
+  return base + a + b * c;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:21: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:21: note: perform multiplication in a wider type
+}
+
+char *n7(char *base, int a, int b) {
+  return base + (a * b);
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:18: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:18: note: perform multiplication in a wider type
+}
+char *n8(char *base, int a, int b, int c) {
+  return base + (a * b) + c;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'
+  // CHECK-NOTES: :[[@LINE-2]]:18: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:18: note: perform multiplication in a wider type
+}
+char *n9(char *base, int a, int b, int c) {
+  return base + (a * b + c);
+}
+
+char *n10(char *base, int a, int b) {
+  return base + (long)(a * b);
+}
+char *n11(char *base, int a, int b) {
+  return base + (unsigned long)(a * b);
+}
+
+#ifdef __cplusplus
+template <typename T>
+char *template_test(char *base, T a, T b) {
+  return base + a * b;
+}
+char *template_test_instantiation(char *base, int a, int b) {
+  return template_test(base, a, b);
+}
+#endif

clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result.cpp

+// RUN: %check_clang_tidy -std=c99 %s bugprone-implicit-widening-of-multiplication-result %t -- -- -x c
+// RUN: %check_clang_tidy %s bugprone-implicit-widening-of-multiplication-result %t -- -- -x c++
+
+long t0(int a, int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+unsigned long t1(int a, int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+
+long t2(unsigned int a, int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'unsigned int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+unsigned long t3(unsigned int a, int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'unsigned int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+
+long t4(int a, unsigned int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'unsigned int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+unsigned long t5(int a, unsigned int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'unsigned int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+
+long t6(unsigned int a, unsigned int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'unsigned int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+unsigned long t7(unsigned int a, unsigned int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'unsigned int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+
+long t8(short a, int b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+long t9(short a, short b) {
+  return a * b;
+  // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type
+}
+
+long t10(int a, int b) {
+  return (a * b);
+  // CHECK-NOTES: :[[@LINE-1]]:11: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES: :[[@LINE-2]]:11: note: make conversion explicit to silence this warning
+  // CHECK-NOTES: :[[@LINE-3]]:11: note: perform multiplication in a wider type
+}
+long n11(int a, int b) {
+  return (long)(a * b);
+}
+long n12(int a, int b) {
+  return (unsigned long)(a * b);
+}
+
+long n13(long a, int b) {
+  return a * b;
+}
+long n14(int a, long b) {
+  return a * b;
+}
+
+long n15(int a, int b, int c) {
+  return a + b * c;
+}
+long n16(int a, int b, int c) {
+  return a * b + c;
+}
+
+#ifdef __cplusplus
+template <typename T1, typename T2>
+T2 template_test(T1 a, T1 b) {
+  return a * b;
+}
+long template_test_instantiation(int a, int b) {
+  return template_test<int, long>(a, b);
+}
+#endif

clang/include/clang/AST/ASTContext.h

public:
   // Per C99 6.2.5p6, for every signed integer type, there is a corresponding
   // unsigned integer type.  This method takes a signed type, and returns the
   // corresponding unsigned integer type.
   // With the introduction of fixed point types in ISO N1169, this method also
   // accepts fixed point types and returns the corresponding unsigned type for
   // a given fixed point type.
   QualType getCorrespondingUnsignedType(QualType T) const;
 
+  // Per C99 6.2.5p6, for every signed integer type, there is a corresponding
+  // unsigned integer type.  This method takes an unsigned type, and returns the
+  // corresponding signed integer type.
+  // With the introduction of fixed point types in ISO N1169, this method also
+  // accepts fixed point types and returns the corresponding signed type for
+  // a given fixed point type.
+  QualType getCorrespondingSignedType(QualType T) const;
+
   // Per ISO N1169, this method accepts fixed point types and returns the
   // corresponding saturated type for a given fixed point type.
   QualType getCorrespondingSaturatedType(QualType Ty) const;
 
   // This method accepts fixed point types and returns the corresponding signed
   // type. Unlike getCorrespondingUnsignedType(), this only accepts unsigned
   // fixed point types because there are unsigned integer types like bool and
   // char8_t that don't have signed equivalents.

clang/lib/AST/ASTContext.cpp

case BuiltinType::SatFract:
     return SatUnsignedFractTy;
   case BuiltinType::SatLongFract:
     return SatUnsignedLongFractTy;
   default:
     llvm_unreachable("Unexpected signed integer or fixed point type");
   }
 }
 
+QualType ASTContext::getCorrespondingSignedType(QualType T) const {
+  assert((T->hasUnsignedIntegerRepresentation() ||
+          T->isUnsignedFixedPointType()) &&
+         "Unexpected type");
+
+  // Turn <4 x unsigned int> -> <4 x signed int>
+  if (const auto *VTy = T->getAs<VectorType>())
+    return getVectorType(getCorrespondingSignedType(VTy->getElementType()),
+                         VTy->getNumElements(), VTy->getVectorKind());
+
+  // For enums, we return the signed version of the base type.
+  if (const auto *ETy = T->getAs<EnumType>())
+    T = ETy->getDecl()->getIntegerType();
+
+  switch (T->castAs<BuiltinType>()->getKind()) {
+  case BuiltinType::Char_U:
+  case BuiltinType::UChar:
+    return SignedCharTy;
+  case BuiltinType::UShort:
+    return ShortTy;
+  case BuiltinType::UInt:
+    return IntTy;
+  case BuiltinType::ULong:
+    return LongTy;
+  case BuiltinType::ULongLong:
+    return LongLongTy;
+  case BuiltinType::UInt128:
+    return Int128Ty;
+  // wchar_t is special. It is either unsigned or not, but when it's unsigned,
+  // there's no matching "signed wchar_t". Therefore we return the signed
+  // version of it's underlying type instead.
+  case BuiltinType::WChar_U:
+    return getSignedWCharType();
+
+  case BuiltinType::UShortAccum:
+    return ShortAccumTy;
+  case BuiltinType::UAccum:
+    return AccumTy;
+  case BuiltinType::ULongAccum:
+    return LongAccumTy;
+  case BuiltinType::SatUShortAccum:
+    return SatShortAccumTy;
+  case BuiltinType::SatUAccum:
+    return SatAccumTy;
+  case BuiltinType::SatULongAccum:
+    return SatLongAccumTy;
+  case BuiltinType::UShortFract:
+    return ShortFractTy;
+  case BuiltinType::UFract:
+    return FractTy;
+  case BuiltinType::ULongFract:
+    return LongFractTy;
+  case BuiltinType::SatUShortFract:
+    return SatShortFractTy;
+  case BuiltinType::SatUFract:
+    return SatFractTy;
+  case BuiltinType::SatULongFract:
+    return SatLongFractTy;
+  default:
+    llvm_unreachable("Unexpected unsigned integer or fixed point type");
+  }
+}
+
 ASTMutationListener::~ASTMutationListener() = default;
 
 void ASTMutationListener::DeducedReturnType(const FunctionDecl *FD,
                                             QualType ReturnType) {}
 
 //===----------------------------------------------------------------------===//
 //                          Builtin Type Computation
 //===----------------------------------------------------------------------===//