lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, aaron.ballman, rjmccall, erichkeane,
dblaikie.
lebedev.ri added a project: LLVM.
lebedev.ri requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 <https://github.com/darktable-org/darktable>, there is a *long*,
painful history of dealing with such bugs:
- 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.
Right now i've added the diagnostic into `-Wall`.
I have not yet looked at clang stage-2 story.
Thoughts?
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D93822
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/implicit-widening-of-multiplication-result.c
clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c
clang/test/Sema/implicit-widening-of-pointer-offset.c
Index: clang/test/Sema/implicit-widening-of-pointer-offset.c
===================================================================
--- /dev/null
+++ clang/test/Sema/implicit-widening-of-pointer-offset.c
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --implicit-check-not="fix-it" %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify -x c++ %s
+
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify=silent %s
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify=silent -x c++ %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-conversion -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify=silent %s
+
+// silent-no-diagnostics
+
+char *t0(char *base, int a, int b) {
+ return base + a * b; // #0
+ // expected-warning@#0 {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+ // expected-note@#0 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:"(ssize_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
+ // expected-note@#0 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"(ssize_t)"
+}
+char *t1(char *base, int a, int b) {
+ return a * b + base; // #1
+ // expected-warning@#1 {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+ // expected-note@#1 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(ssize_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#1 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(ssize_t)"
+}
+
+char *t2(char *base, unsigned int a, int b) {
+ return base + a * b; // #2
+ // expected-warning@#2 {{result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'}}
+ // expected-note@#2 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:"(size_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
+ // expected-note@#2 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"(size_t)"
+}
+
+char *t3(char *base, int a, unsigned int b) {
+ return base + a * b; // #3
+ // expected-warning@#3 {{result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'}}
+ // expected-note@#3 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:"(size_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
+ // expected-note@#3 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"(size_t)"
+}
+
+char *t4(char *base, unsigned int a, unsigned int b) {
+ return base + a * b; // #4
+ // expected-warning@#4 {{result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'}}
+ // expected-note@#4 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:"(size_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
+ // expected-note@#4 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"(size_t)"
+}
+
+char *t5(char *base, int a, int b, int c) {
+ return base + a * b + c; // #5
+ // expected-warning@#5 {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+ // expected-note@#5 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:"(ssize_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:21}:")"
+ // expected-note@#5 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:17-[[@LINE-6]]:17}:"(ssize_t)"
+}
+char *t6(char *base, int a, int b, int c) {
+ return base + a + b * c; // #6
+ // expected-warning@#6 {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+ // expected-note@#6 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:21}:"(ssize_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:25}:")"
+ // expected-note@#6 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:21-[[@LINE-6]]:21}:"(ssize_t)"
+}
+
+char *n11(char *base, int a, int b) {
+ return base + (a * b);
+}
+char *n12(char *base, int a, int b, int c) {
+ return base + (a * b + c);
+}
+char *n13(char *base, int a, int b, int c) {
+ return base + (a * b) + c;
+}
+
+char *n14(char *base, int a, int b) {
+ return base + (long)(a * b);
+}
+char *n15(char *base, int a, int b) {
+ return base + (unsigned long)(a * b);
+}
Index: clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c
===================================================================
--- /dev/null
+++ clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --implicit-check-not="fix-it" %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify -x c++ %s
+
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify=silent %s
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-pointer-offset -verify=silent -x c++ %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-conversion -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify=silent %s
+
+// silent-no-diagnostics
+
+char *t0(char *base, int a, int b) {
+ return &base[a * b]; // #0
+ // expected-warning@#0 {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+ // expected-note@#0 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:16}:"(ssize_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
+ // expected-note@#0 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:"(ssize_t)"
+}
+void t1(char *base, int a, int b) {
+ // FIXME: test `[a * b]base` pattern?
+}
+
+char *t2(char *base, unsigned int a, int b) {
+ return &base[a * b]; // #2
+ // expected-warning@#2 {{result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'}}
+ // expected-note@#2 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:16}:"(size_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
+ // expected-note@#2 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:"(size_t)"
+}
+
+char *t3(char *base, int a, unsigned int b) {
+ return &base[a * b]; // #3
+ // expected-warning@#3 {{result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'}}
+ // expected-note@#3 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:16}:"(size_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
+ // expected-note@#3 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:"(size_t)"
+}
+
+char *t4(char *base, unsigned int a, unsigned int b) {
+ return &base[a * b]; // #4
+ // expected-warning@#4 {{result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t'}}
+ // expected-note@#4 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:16}:"(size_t)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:20-[[@LINE-4]]:20}:")"
+ // expected-note@#4 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:16-[[@LINE-6]]:16}:"(size_t)"
+}
+
+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 *n11(char *base, int a, int b) {
+ return &base[(a * b)];
+}
+char *n12(char *base, int a, int b, int c) {
+ return &base[(a * b + c)];
+}
+char *n13(char *base, int a, int b, int c) {
+ return &base[(a * b) + c];
+}
+
+char *n14(char *base, int a, int b) {
+ return &base[(long)(a * b)];
+}
+char *n15(char *base, int a, int b) {
+ return &base[(unsigned long)(a * b)];
+}
Index: clang/test/Sema/implicit-widening-of-multiplication-result.c
===================================================================
--- /dev/null
+++ clang/test/Sema/implicit-widening-of-multiplication-result.c
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-multiplication-result -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-multiplication-result -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --implicit-check-not="fix-it" %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-of-multiplication-result -verify -x c++ %s
+
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-multiplication-result -verify=silent %s
+// RUN: %clang_cc1 -triple i686-linux-gnu -fsyntax-only -Wimplicit-widening-of-multiplication-result -verify=silent -x c++ %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wimplicit-widening-conversion -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify=silent %s
+
+// silent-no-diagnostics
+
+long t0(int a, int b) {
+ return a * b; // #0
+ // expected-warning@#0 {{performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'}}
+ // expected-note@#0 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#0 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(long)"
+}
+unsigned long t1(int a, int b) {
+ return a * b; // #1
+ // expected-warning@#1 {{performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int'}}
+ // expected-note@#1 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(unsigned long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#1 {{perform multiplication in a wider type}}
+}
+
+long t2(unsigned int a, int b) {
+ return a * b; // #2
+ // expected-warning@#2 {{performing an implicit widening conversion to type 'long' of a multiplication performed in type 'unsigned int'}}
+ // expected-note@#2 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#2 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(unsigned long)"
+}
+unsigned long t3(unsigned int a, int b) {
+ return a * b; // #3
+ // expected-warning@#3 {{performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'unsigned int'}}
+ // expected-note@#3 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(unsigned long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#3 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(unsigned long)"
+}
+
+long t4(int a, unsigned int b) {
+ return a * b; // #4
+ // expected-warning@#4 {{performing an implicit widening conversion to type 'long' of a multiplication performed in type 'unsigned int'}}
+ // expected-note@#4 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#4 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(unsigned long)"
+}
+unsigned long t5(int a, unsigned int b) {
+ return a * b; // #5
+ // expected-warning@#5 {{performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'unsigned int'}}
+ // expected-note@#5 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(unsigned long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#5 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(unsigned long)"
+}
+
+long t6(unsigned int a, unsigned int b) {
+ return a * b; // #6
+ // expected-warning@#6 {{performing an implicit widening conversion to type 'long' of a multiplication performed in type 'unsigned int'}}
+ // expected-note@#6 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#6 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(unsigned long)"
+}
+unsigned long t7(unsigned int a, unsigned int b) {
+ return a * b; // #7
+ // expected-warning@#7 {{performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'unsigned int'}}
+ // expected-note@#7 {{make conversion explicit to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"(unsigned long)("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:14-[[@LINE-4]]:14}:")"
+ // expected-note@#7 {{perform multiplication in a wider type}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:10-[[@LINE-6]]:10}:"(unsigned long)"
+}
+
+long n8(int a, int b) {
+ return (a * b);
+}
+long n9(int a, int b) {
+ return (long)(a * b);
+}
+long n10(int a, int b) {
+ return (unsigned long)(a * b);
+}
+
+long n11(long a, int b) {
+ return a * b;
+}
+long n12(int a, long b) {
+ return a * b;
+}
+
+long n13(int a, int b, int c) {
+ return a + b * c;
+}
+long n14(int a, int b, int c) {
+ return a * b + c;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4723,8 +4723,13 @@
ExprResult Res = CreateBuiltinArraySubscriptExpr(base, lbLoc, idx, rbLoc);
- if (!Res.isInvalid() && isa<ArraySubscriptExpr>(Res.get()))
- CheckSubscriptAccessOfNoDeref(cast<ArraySubscriptExpr>(Res.get()));
+ if (!Res.isInvalid()) {
+ diagnoseImplicitWideningConversionOfMultiplicationOfPointerOffset(
+ Res.get());
+
+ if (isa<ArraySubscriptExpr>(Res.get()))
+ CheckSubscriptAccessOfNoDeref(cast<ArraySubscriptExpr>(Res.get()));
+ }
return Res;
}
@@ -14327,7 +14332,16 @@
// Emit warnings for tricky precedence issues, e.g. "bitfield & 0x4 == 0"
DiagnoseBinOpPrecedence(*this, Opc, TokLoc, LHSExpr, RHSExpr);
- return BuildBinOp(S, TokLoc, Opc, LHSExpr, RHSExpr);
+ ExprResult Res = BuildBinOp(S, TokLoc, Opc, LHSExpr, RHSExpr);
+
+ if (!Res.isInvalid()) {
+ if (Opc == BO_Add || Opc == BO_Sub || Opc == BO_AddAssign ||
+ Opc == BO_SubAssign)
+ diagnoseImplicitWideningConversionOfMultiplicationOfPointerOffset(
+ Res.get());
+ }
+
+ return Res;
}
void Sema::LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -547,6 +547,123 @@
<< FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
}
+static Expr *GetLHSOfMulBinOp(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 Sema::diagnoseImplicitWideningConversionOfMultiplication(Expr *E,
+ QualType Ty,
+ CastKind Kind) {
+ // We are only interested in basic integral casts.
+ if (Kind != CK_IntegralCast)
+ return;
+
+ QualType ExprTy = E->getType();
+
+ // This must be a widening cast. Else we do not care.
+ unsigned SrcWidth = Context.getIntWidth(ExprTy);
+ 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?
+ Expr *LHS = GetLHSOfMulBinOp(E);
+ if (!LHS)
+ return;
+
+ // Ok, looks like we should diagnose this.
+ Diag(E->getBeginLoc(), diag::warn_imp_widening_of_mul_result)
+ << Ty << E->getType();
+
+ Diag(E->getBeginLoc(), diag::note_warn_imp_widening_silence)
+ << 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() == ExprTy->isSignedIntegerType())
+ WideExprTy = Ty;
+ else if (Ty->isSignedIntegerType()) {
+ assert(ExprTy->isUnsignedIntegerType() &&
+ "Expected source type to be signed.");
+ WideExprTy = Context.getCorrespondingUnsignedType(Ty);
+ }
+ // FIXME: else we need getCorrespondingSignedType(), but there isn't one...
+
+ auto Fix = Diag(E->getBeginLoc(), diag::note_warn_imp_widening_fix)
+ << LHS->getSourceRange();
+ if (!WideExprTy.isNull())
+ Fix << FixItHint::CreateInsertion(E->getBeginLoc(),
+ "(" + WideExprTy.getAsString() + ")");
+ }
+}
+
+void Sema::diagnoseImplicitWideningConversionOfMultiplicationOfPointerOffset(
+ Expr *E) {
+ // We are looking for a pointer offset operation,
+ // with one hand being a pointer, and another one being an offset.
+ 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;
+
+ QualType IndexExprType = IndexExpr->getType();
+
+ 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?
+ Expr *LHS = GetLHSOfMulBinOp(IndexExpr);
+ if (!LHS)
+ return;
+
+ // Ok, looks like we should diagnose this.
+ Diag(E->getBeginLoc(), diag::warn_imp_widening_of_ptr_offset)
+ << IndexExprType << TyAsString;
+
+ Diag(IndexExpr->getBeginLoc(), diag::note_warn_imp_widening_silence)
+ << FixItHint::CreateInsertion(IndexExpr->getBeginLoc(),
+ (Twine("(") + TyAsString + ")(").str())
+ << FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")")
+ << IndexExpr->getSourceRange();
+
+ Diag(IndexExpr->getBeginLoc(), diag::note_warn_imp_widening_fix)
+ << LHS->getSourceRange()
+ << FixItHint::CreateInsertion(IndexExpr->getBeginLoc(),
+ (Twine("(") + TyAsString + ")").str());
+}
+
/// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast.
/// If there is already an implicit cast, merge into the existing one.
/// The result is of the given category.
@@ -577,6 +694,7 @@
diagnoseNullableToNonnullConversion(Ty, E->getType(), E->getBeginLoc());
diagnoseZeroToNullptrConversion(Kind, E);
+ diagnoseImplicitWideningConversionOfMultiplication(E, Ty, Kind);
QualType ExprTy = Context.getCanonicalType(E->getType());
QualType TypeTy = Context.getCanonicalType(Ty);
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4745,6 +4745,17 @@
/// Warn when implicitly casting 0 to nullptr.
void diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E);
+ /// Warn when performing an implicit widening conversion of a multiplication,
+ /// when perhaps the whole multiplication should be performed in a wider type.
+ void diagnoseImplicitWideningConversionOfMultiplication(Expr *E, QualType Ty,
+ CastKind Kind);
+
+ /// Warn when performing an implicit widening conversion of a multiplication,
+ /// when perhaps the whole multiplication should be performed in a wider type,
+ /// when the result is used as an offset for the pointer.
+ void
+ diagnoseImplicitWideningConversionOfMultiplicationOfPointerOffset(Expr *E);
+
ParsingDeclState PushParsingDeclaration(sema::DelayedDiagnosticPool &pool) {
return DelayedDiagnostics.push(pool);
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3714,6 +3714,19 @@
"code; pointer may be assumed to always convert to true">,
InGroup<UndefinedBoolConversion>;
+def warn_imp_widening_of_mul_result : Warning<
+ "performing an implicit widening conversion to type %0 "
+ "of a multiplication performed in type %1">,
+ InGroup<ImplicitWideningOfMulResult>, DefaultIgnore;
+def warn_imp_widening_of_ptr_offset : Warning<
+ "result of multiplication in type %0 is used as a pointer offset "
+ "after an implicit widening conversion to type '%1'">,
+ InGroup<ImplicitWideningOfPtrOffset>, DefaultIgnore;
+def note_warn_imp_widening_fix : Note<
+ "perform multiplication in a wider type">;
+def note_warn_imp_widening_silence : Note<
+ "make conversion explicit to silence this warning">;
+
def warn_xor_used_as_pow : Warning<
"result of '%0' is %1; did you mean exponentiation?">,
InGroup<XorUsedAsPow>;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -843,6 +843,14 @@
UnusedPropertyIvar]>,
DiagCategory<"Unused Entity Issue">;
+def ImplicitWideningOfMulResult : DiagGroup<"implicit-widening-of-multiplication-result">;
+def ImplicitWideningOfPtrOffset : DiagGroup<"implicit-widening-of-pointer-offset">;
+
+def ImplicitWideningConversion : DiagGroup<"implicit-widening-conversion", [
+ ImplicitWideningOfMulResult,
+ ImplicitWideningOfPtrOffset,
+ ]>, DiagCategory<"Potentially misplaced implicit widening conversion">;
+
// Format settings.
def FormatInvalidSpecifier : DiagGroup<"format-invalid-specifier">;
def FormatSecurity : DiagGroup<"format-security">;
@@ -956,8 +964,14 @@
// Note that putting warnings in -Wall will not disable them by default. If a
// warning should be active _only_ when -Wall is passed in, mark it as
// DefaultIgnore in addition to putting it here.
-def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool,
- MisleadingIndentation]>;
+def All : DiagGroup<"all", [
+ Most,
+ Parentheses,
+ Switch,
+ SwitchBool,
+ MisleadingIndentation,
+ ImplicitWideningConversion,
+ ]>;
// Warnings that should be in clang-cl /w4.
def : DiagGroup<"CL4", [All, Extra]>;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -51,7 +51,30 @@
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- ...
+- A new diagnostics group ``-Wimplicit-widening-conversion`` was added (as part
+ of ``-Wall`` group) which includes the following diagnostics:
+ - ``-Wimplicit-widening-of-multiplication-result``:
+
+ .. code-block:: c++
+
+ long mul(int a, int b) {
+ return a * b; // expected-warning {{performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'}}
+ }
+
+ - ``-Wimplicit-widening-of-pointer-offset``:
+
+ .. code-block:: c++
+
+ char* ptr_add(char *base, int a, int b) {
+ return base + a * b; // expected-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]; // expected-warning {{result of multiplication in type 'int' is used as a pointer offset after an implicit widening conversion to type 'ssize_t'}}
+ }
+
+ The diagnostics can be silenced by making the widening cast explicit,
+ or fixed, by performing the multiplication in a wider type to begin with.
Non-comprehensive list of changes in this release
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
