[-Wunsafe-buffer-usage] Ignore array subscript on literal zero

[ziqingluo-90]

Summary

Improving unsafe array subscript warning reporting.
For array subscripts with a literal zero index, no warning will be emitted.

[xazax.hun]

clang/lib/Analysis/UnsafeBufferUsage.cpp

192

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.

[NoQ]

LGTM!

clang/lib/Analysis/UnsafeBufferUsage.cpp

192

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".

clang/lib/Analysis/UnsafeBufferUsage.cpp

public:
 
   static bool classof(const Gadget *G) {
     return G->getKind() == Kind::ArraySubscript;
   }
 
   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())))
+    return stmt(arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType())),
+                                   unless(hasIndex(integerLiteral(equals(0)))))
                 .bind(ArraySubscrTag));
   }
 
   const ArraySubscriptExpr *getBaseStmt() const override { return ASE; }
 
   DeclUseList getClaimedVarUseSites() const override {
     if (const auto *DRE =
             dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) {
       return {DRE};

clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

void testIncrement(char *p) {
   --p; // expected-warning{{unchecked operation on raw buffer in expression}}
   p--; // expected-warning{{unchecked operation on raw buffer in expression}}
 }
 
 void * voidPtrCall(void);
 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}}
     void * q = p;
 
     foo(((int*)q)[10]); // expected-warning{{unchecked operation on raw buffer in expression}}
   }
 
   foo(((int*)voidPtrCall())[3], // expected-warning{{unchecked operation on raw buffer in expression}}
       3[(int*)voidPtrCall()],   // expected-warning{{unchecked operation on raw buffer in expression}}
       charPtrCall()[3],         // expected-warning{{unchecked operation on raw buffer in expression}}
       3[charPtrCall()]          // expected-warning{{unchecked operation on raw buffer in expression}}
       );
 
   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) {
   int a[10];
 
   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) {
   //TODO: do not warn for unevaluated context
   foo(sizeof(p[1]),             // expected-warning{{unchecked operation on raw buffer in expression}}
       sizeof(decltype(p[1])));  // expected-warning{{unchecked operation on raw buffer in expression}}
 }