[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf6c54cdbc439: [-Wunsafe-buffer-usage] Ignore array subscript
on literal zero (authored by ziqingluo-90).
Changed prior to commit:
https://reviews.llvm.org/D138321?vs=476581=483718#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138321/new/
https://reviews.llvm.org/D138321
Files:
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
char * charPtrCall(void);
void testArraySubscripts(int *p, int **pp) {
- foo(p[0], // expected-warning{{unchecked operation on raw buffer
in expression}}
- pp[0][0], // expected-warning2{{unchecked operation on raw
buffer in expression}}
- 0[0[pp]], // expected-warning2{{unchecked operation on raw
buffer in expression}}
- 0[pp][0] // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ foo(p[1], // expected-warning{{unchecked operation on raw buffer
in expression}}
+ pp[1][1], // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ 1[1[pp]], // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ 1[pp][1] // expected-warning2{{unchecked operation on raw
buffer in expression}}
);
if (p[3]) { // expected-warning{{unchecked operation on raw buffer
in expression}}
@@ -50,11 +50,18 @@
int a[10], b[10][10];
// Not to warn subscripts on arrays
- foo(a[0], a[1],
- 0[a], 1[a],
+ foo(a[1], 1[a],
b[3][4],
4[b][3],
4[3[b]]);
+
+ // Not to warn when index is zero
+ foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+ ((int*)voidPtrCall())[0],
+ 0[(int*)voidPtrCall()],
+ charPtrCall()[0],
+ 0[charPtrCall()]
+ );
}
void testArraySubscriptsWithAuto(int *p, int **pp) {
@@ -62,19 +69,19 @@
auto ap1 = a;
- foo(ap1[0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap1[1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
auto ap2 = p;
- foo(ap2[0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap2[1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
auto ap3 = pp;
- foo(ap3[0][0]); // expected-warning2{{unchecked operation on raw buffer in
expression}}
+ foo(ap3[1][1]); // expected-warning2{{unchecked operation on raw buffer in
expression}}
auto ap4 = *pp;
- foo(ap4[0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap4[1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
}
void testUnevaluatedContext(int * p) {
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -189,9 +189,9 @@
static Matcher matcher() {
// FIXME: What if the index is integer literal 0? Should this be
// a safe gadget in this case?
-return stmt(
-arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType(
-.bind(ArraySubscrTag));
+return
stmt(arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+ unless(hasIndex(integerLiteral(equals(0)
+.bind(ArraySubscrTag));
}
const ArraySubscriptExpr *getBaseStmt() const override { return ASE; }
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
char * charPtrCall(void);
void testArraySubscripts(int *p, int **pp) {
- foo(p[0], // expected-warning{{unchecked operation on raw buffer in expression}}
- pp[0][0], // expected-warning2{{unchecked operation on raw buffer in expression}}
- 0[0[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
- 0[pp][0] // expected-warning2{{unchecked operation on raw buffer in expression}}
+ foo(p[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+ pp[1][1], // expected-warning2{{unchecked operation on raw buffer in expression}}
+ 1[1[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
+ 1[pp][1] // expected-warning2{{unchecked operation on raw buffer in
[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:219
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+ unless(hasIndex(integerLiteral(equals(0)
.bind("arraySubscr"));
xazax.hun wrote:
> Isn't it the case you still want to cover zero indices but as a safe gadget
> to make sure fixits can be emitted?
> Having to add another gadget for this makes me think maybe categorizing the
> safety of gadgets upfront is not the right model.
So according to the discussion in D140062 it actually *is* the right model to
decide safety up front, and then maybe even have some duplication, because the
safe gadget has to provide a lot more context in the matcher in order for us to
emit any fix at all. So the fixable gadget wouldn't be "same thing but with
different index". It'd be "a completely different thing with completely
arbitrary index".
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138321/new/
https://reviews.llvm.org/D138321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero
xazax.hun added inline comments.
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:219
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+ unless(hasIndex(integerLiteral(equals(0)
.bind("arraySubscr"));
Isn't it the case you still want to cover zero indices but as a safe gadget to
make sure fixits can be emitted?
Having to add another gadget for this makes me think maybe categorizing the
safety of gadgets upfront is not the right model.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138321/new/
https://reviews.llvm.org/D138321
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero
ziqingluo-90 updated this revision to Diff 476581.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138321/new/
https://reviews.llvm.org/D138321
Files:
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
char * baz(void);
void testArraySubscripts(int *p, int **pp) {
- foo(p[0], // expected-warning{{unchecked operation on raw buffer
in expression}}
- pp[0][0], // expected-warning2{{unchecked operation on raw
buffer in expression}}
- 0[0[pp]], // expected-warning2{{unchecked operation on raw
buffer in expression}}
- 0[pp][0] // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ foo(p[1], // expected-warning{{unchecked operation on raw buffer
in expression}}
+ pp[1][1], // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ 1[1[pp]], // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ 1[pp][1] // expected-warning2{{unchecked operation on raw
buffer in expression}}
);
if (p[3]) { // expected-warning{{unchecked operation on raw buffer
in expression}}
@@ -50,11 +50,18 @@
int a[10], b[10][10];
// Not to warn subscripts on arrays
- foo(a[0], a[1],
- 0[a], 1[a],
+ foo(a[1], 1[a],
b[3][4],
4[b][3],
4[3[b]]);
+
+ // Not to warn when index is zero
+ foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+ ((int*)bar())[0],
+ 0[(int*)bar()],
+ baz()[0],
+ 0[baz()]
+ );
}
void testArraySubscriptsWithAuto(int *p, int **pp) {
@@ -62,19 +69,19 @@
auto ap1 = a;
- foo(ap1[0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap1[1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
auto ap2 = p;
- foo(ap2[0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap2[1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
auto ap3 = pp;
- foo(ap3[0][0]); // expected-warning2{{unchecked operation on raw buffer in
expression}}
+ foo(ap3[1][1]); // expected-warning2{{unchecked operation on raw buffer in
expression}}
auto ap4 = *pp;
- foo(ap4[0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap4[1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
}
void testQualifiedParameters(const int * p, const int * const q,
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -214,10 +214,9 @@
}
static Matcher matcher() {
-// FIXME: What if the index is integer literal 0? Should this be
-// a safe gadget in this case?
return stmt(
-arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType(
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+ unless(hasIndex(integerLiteral(equals(0)
.bind("arraySubscr"));
}
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
char * baz(void);
void testArraySubscripts(int *p, int **pp) {
- foo(p[0], // expected-warning{{unchecked operation on raw buffer in expression}}
- pp[0][0], // expected-warning2{{unchecked operation on raw buffer in expression}}
- 0[0[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
- 0[pp][0] // expected-warning2{{unchecked operation on raw buffer in expression}}
+ foo(p[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+ pp[1][1], // expected-warning2{{unchecked operation on raw buffer in expression}}
+ 1[1[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
+ 1[pp][1] // expected-warning2{{unchecked operation on raw buffer in expression}}
);
if (p[3]) { // expected-warning{{unchecked operation on raw buffer in expression}}
@@ -50,11 +50,18 @@
int a[10], b[10][10];
// Not to warn subscripts on arrays
- foo(a[0], a[1],
- 0[a], 1[a],
+ foo(a[1], 1[a],
b[3][4],
4[b][3],
4[3[b]]);
+
+ // Not to warn when index is zero
+ foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+ ((int*)bar())[0],
+ 0[(int*)bar()],
+ baz()[0],
+ 0[baz()]
+
[PATCH] D138321: [-Wunsafe-buffer-usage] Ignore array subscript on literal zero
ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak,
aaron.ballman, gribozavr, xazax.hun.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Improving unsafe array subscript warning reporting.
For array subscripts with a literal zero index, no warning will be emitted.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138321
Files:
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
char * baz(void);
void testArraySubscripts(int *p, int **pp) {
- foo(p[0], // expected-warning{{unchecked operation on raw buffer
in expression}}
- pp[0][0], // expected-warning2{{unchecked operation on raw
buffer in expression}}
- 0[0[pp]], // expected-warning2{{unchecked operation on raw
buffer in expression}}
- 0[pp][0] // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ foo(p[1], // expected-warning{{unchecked operation on raw buffer
in expression}}
+ pp[1][1], // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ 1[1[pp]], // expected-warning2{{unchecked operation on raw
buffer in expression}}
+ 1[pp][1] // expected-warning2{{unchecked operation on raw
buffer in expression}}
);
if (p[3]) { // expected-warning{{unchecked operation on raw buffer
in expression}}
@@ -50,11 +50,18 @@
int a[10], b[10][10];
// Not to warn subscripts on arrays
- foo(a[0], a[1],
- 0[a], 1[a],
+ foo(a[1], 1[a],
b[3][4],
4[b][3],
4[3[b]]);
+
+ // Not to warn when index is zero
+ foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
+ ((int*)bar())[0],
+ 0[(int*)bar()],
+ baz()[0],
+ 0[baz()]
+ );
}
void testArraySubscriptsWithAuto(int *p, int **pp) {
@@ -62,19 +69,19 @@
auto ap1 = a; // expected-warning{{variable 'ap1' participates in unchecked
buffer operations}}
- foo(ap1[0]);
+ foo(ap1[1]);
auto ap2 = p; // expected-warning{{variable 'ap2' participates in unchecked
buffer operations}}
- foo(ap2[0]);
+ foo(ap2[1]);
auto ap3 = pp; // expected-warning{{variable 'ap3' participates in
unchecked buffer operations}}
- foo(ap3[0][0]); // expected-warning{{unchecked operation on raw buffer in
expression}}
+ foo(ap3[1][1]); // expected-warning{{unchecked operation on raw buffer in
expression}}
auto ap4 = *pp; // expected-warning{{variable 'ap4' participates in
unchecked buffer operations}}
- foo(ap4[0]);
+ foo(ap4[1]);
}
void testQualifiedParameters(const int * p, const int * const q,
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -228,10 +228,9 @@
}
static Matcher matcher() {
-// FIXME: What if the index is integer literal 0? Should this be
-// a safe gadget in this case?
return stmt(
-arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType(
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+ unless(hasIndex(integerLiteral(equals(0)
.bind("arraySubscr"));
}
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -29,10 +29,10 @@
char * baz(void);
void testArraySubscripts(int *p, int **pp) {
- foo(p[0], // expected-warning{{unchecked operation on raw buffer in expression}}
- pp[0][0], // expected-warning2{{unchecked operation on raw buffer in expression}}
- 0[0[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
- 0[pp][0] // expected-warning2{{unchecked operation on raw buffer in expression}}
+ foo(p[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+ pp[1][1], // expected-warning2{{unchecked operation on raw buffer in expression}}
+ 1[1[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
+ 1[pp][1] // expected-warning2{{unchecked operation on raw buffer in expression}}
);
if (p[3]) { // expected-warning{{unchecked operation on raw buffer in expression}}
@@ -50,11 +50,18 @@
int a[10], b[10][10];
// Not to warn subscripts
