[-Wunsafe-buffer-usage] Emit warnings about unsafe operations on arrays

[jkorous]

Summary

An almost forgotten patch by @t-rasmud! 😅

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

331

Should this gadget receive the same treatment?

[jkorous]

clang/lib/Analysis/UnsafeBufferUsage.cpp

331

Oh! Yes, it definitely should!
@t-rasmud Can you please take over this patch?

[jkorous]

clang/lib/Analysis/UnsafeBufferUsage.cpp

331

I am sorry, I jumped the gun - the other gadget seems fine.
It looks like it already handles the situation since it expects an expression of pointer type (which reference to an array that decayed to a pointer totally is).

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

331

Yeah I think you're right, this gadget doesn't care about variable type, and the implicit array-to-pointer decay takes care of the expression type for us. Let's add a test for this anyway, just to acknowledge that this works, and then I think the patch is good to go!

[jkorous]

Deprecate in favor of https://reviews.llvm.org/D141725

clang/lib/Analysis/UnsafeBufferUsage.cpp

 class Strategy;
 } // namespace
 
 // Because we're dealing with raw pointers, let's define what we mean by that.
 static auto hasPointerType() {
     return hasType(hasCanonicalType(pointerType()));
 }
 
+// TODO use hasCanonicalType
+static auto hasArrayType() {
+    return anyOf(
+      hasType(arrayType()),
+      hasType(autoType(
+              hasDeducedType(hasUnqualifiedDesugaredType(arrayType())))),
+      // ElaboratedType, e.g., typedef
+      hasType(elaboratedType(hasUnqualifiedDesugaredType(arrayType()))));
+}
+
 namespace {
 /// Gadget is an individual operation in the code that may be of interest to
 /// this analysis. Each (non-abstract) subclass corresponds to a specific
 /// rigid AST structure that constitutes an operation on a pointer-type object.
 /// Discovery of a gadget in the code corresponds to claiming that we understand
 /// what this part of code is doing well enough to potentially improve it.
 /// Gadgets can be warning (immediately deserving a warning) or fixable (not
 /// always deserving a warning per se, but requires our attention to identify
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())),
-                                   unless(hasIndex(integerLiteral(equals(0)))))
-                .bind(ArraySubscrTag));
+
+// clang-format off
+    return stmt(arraySubscriptExpr(
+      hasBase(ignoringParenImpCasts(
+        anyOf(hasPointerType(), hasArrayType()))),
+// clang-format on
   }
 
   const ArraySubscriptExpr *getBaseStmt() const override { return ASE; }
 
   DeclUseList getClaimedVarUseSites() const override {
     if (const auto *DRE =
             dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) {
       return {DRE};
     }
 
     return {};
   }
 };
 
 /// A pointer arithmetic expression of one of the forms:
 ///  \code
 ///  ptr + n | n + ptr | ptr - n | ptr += n | ptr -= n
 ///  \endcode
 class PointerArithmeticGadget : public WarningGadget {
   static constexpr const char *const PointerArithmeticTag = "ptrAdd";
   static constexpr const char *const PointerArithmeticPointerTag = "ptrAddPtr";
   const BinaryOperator *PA; // pointer arithmetic expression
   const Expr * Ptr;         // the pointer expression in `PA`
 
 public:
     PointerArithmeticGadget(const MatchFinder::MatchResult &Result)
       : WarningGadget(Kind::PointerArithmetic),
M.addMatcher(
     stmt(forEveryDescendant(
       stmt(anyOf(
         // Add Gadget::matcher() for every gadget in the registry.
 #define GADGET(x)                                                              \
         x ## Gadget::matcher().bind(#x),
 #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
         // In parallel, match all DeclRefExprs so that to find out
         // whether there are any uncovered by gadgets.
-        declRefExpr(hasPointerType(), to(varDecl())).bind("any_dre"),
+        declRefExpr(anyOf(hasPointerType(), hasArrayType()), to(varDecl())).bind("any_dre"),
         // Also match DeclStmts because we'll need them when fixing
         // their underlying VarDecls that otherwise don't have
         // any backreferences to DeclStmts.
         declStmt().bind("any_ds")
       ))
       // FIXME: Idiomatically there should be a forCallable(equalsNode(D))
       // here, to make sure that the statement actually belongs to the
       // function and not to a nested function. However, forCallable uses

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

void testArraySubscripts(int *p, int **pp) {
   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[1], 1[a],
-      b[3][4],
-      4[b][3],
-      4[3[b]]);
+  foo(a[1], 1[a],   // expected-warning2{{unchecked operation on raw buffer in expression}}
+      b[3][4],      // expected-warning2{{unchecked operation on raw buffer in expression}}
+      4[b][3],      // expected-warning2{{unchecked operation on raw buffer in expression}}
+      4[3[b]]);     // expected-warning2{{unchecked operation on raw buffer in expression}}
 
   // 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[1]);  // expected-warning{{unchecked operation on raw buffer in expression}}
 
   auto ap2 = p;
 
   foo(ap2[1]);  // expected-warning{{unchecked operation on raw buffer in expression}}
 

 }
 
 void testQualifiedParameters(const int * p, const int * const q,
            const int a[10], const int b[10][10],
            int (&c)[10]) {
   foo(p[1], 1[p], p[-1],   // expected-warning3{{unchecked operation on raw buffer in expression}}
       q[1], 1[q], q[-1],   // expected-warning3{{unchecked operation on raw buffer in expression}}
       a[1],                // expected-warning{{unchecked operation on raw buffer in expression}}     `a` is of pointer type
-      b[1][2],             // expected-warning{{unchecked operation on raw buffer in expression}}     `b[1]` is of array type
-      c[1]                 // `c` is of array type
+      b[1][2]              // expected-warning2{{unchecked operation on raw buffer in expression}}     `b[1]` is of array type
       );
 }
 
 struct T {
   int a[10];
   int * b;
   struct {
     int a[10];
     int * b;
   } c;
 };
 
 typedef struct T T_t;
 
 T_t funRetT();
 T_t * funRetTStar();
 
 void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) {
-  foo(sp->a[1],
+  foo(sp->a[1],   // expected-warning{{unchecked operation on raw buffer in expression}}
       sp->b[1],     // expected-warning{{unchecked operation on raw buffer in expression}}
-      sp->c.a[1],
+      sp->c.a[1],   // expected-warning{{unchecked operation on raw buffer in expression}}
       sp->c.b[1],   // expected-warning{{unchecked operation on raw buffer in expression}}
-      s.a[1],
+      s.a[1],       // expected-warning{{unchecked operation on raw buffer in expression}}
       s.b[1],       // expected-warning{{unchecked operation on raw buffer in expression}}
-      s.c.a[1],
+      s.c.a[1],     // expected-warning{{unchecked operation on raw buffer in expression}}
       s.c.b[1],     // expected-warning{{unchecked operation on raw buffer in expression}}
-      sp2->a[1],
+      sp2->a[1],    // expected-warning{{unchecked operation on raw buffer in expression}}
       sp2->b[1],    // expected-warning{{unchecked operation on raw buffer in expression}}
-      sp2->c.a[1],
+      sp2->c.a[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
       sp2->c.b[1],  // expected-warning{{unchecked operation on raw buffer in expression}}
-      s2.a[1],
+      s2.a[1],    // expected-warning{{unchecked operation on raw buffer in expression}}
       s2.b[1],      // expected-warning{{unchecked operation on raw buffer in expression}}
-      s2.c.a[1],
+      s2.c.a[1],           // expected-warning{{unchecked operation on raw buffer in expression}}
       s2.c.b[1],           // expected-warning{{unchecked operation on raw buffer in expression}}
-      funRetT().a[1],
+      funRetT().a[1],      // expected-warning{{unchecked operation on raw buffer in expression}}
       funRetT().b[1],      // expected-warning{{unchecked operation on raw buffer in expression}}
-      funRetTStar()->a[1],
+      funRetTStar()->a[1], // expected-warning{{unchecked operation on raw buffer in expression}}
       funRetTStar()->b[1]  // expected-warning{{unchecked operation on raw buffer in expression}}
       );
 }
 
 int garray[10];
 int * gp = garray;
 int gvar = gp[1];  // FIXME: file scope unsafe buffer access is not warned
 
 void testLambdaCaptureAndGlobal(int * p) {
   int a[10];
 
   auto Lam = [p, a]() {
     return p[1] // expected-warning{{unchecked operation on raw buffer in expression}}
-      + a[1] + garray[1]
+      + a[1] + garray[1] // expected-warning2{{unchecked operation on raw buffer in expression}}
       + gp[1];  // expected-warning{{unchecked operation on raw buffer in expression}}
   };
 }
 
 typedef T_t * T_ptr_t;
 
 void testTypedefs(T_ptr_t p) {
   foo(p[1],      // expected-warning{{unchecked operation on raw buffer in expression}}
-      p[1].a[1], // expected-warning{{unchecked operation on raw buffer in expression}}
+      p[1].a[1], // expected-warning2{{unchecked operation on raw buffer in expression}}
       p[1].b[1]  // expected-warning2{{unchecked operation on raw buffer in expression}}
       );
 }
 
 template<typename T, int N> T f(T t, T * pt, T a[N], T (&b)[N]) {
   foo(pt[1],    // expected-warning{{unchecked operation on raw buffer in expression}}
       a[1],     // expected-warning{{unchecked operation on raw buffer in expression}}
       b[1]);    // `b` is of array type
foo( [p]() { int * q = p;
               q++;  // expected-warning{{unchecked operation on raw buffer in expression}}
               return *q;
        },
        ^(int *p) { p++;   // expected-warning{{unchecked operation on raw buffer in expression}}
        }
      );
 }
 
-void testVariableDecls(int * p) {
+int testVariableDecls(int * p) {
   int * q = p++;      // expected-warning{{unchecked operation on raw buffer in expression}}
   int a[p[1]];        // expected-warning{{unchecked operation on raw buffer in expression}}
   int b = p[1];       // expected-warning{{unchecked operation on raw buffer in expression}}
+  return p[1];        // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
+template<typename T> void fArr(T t[]) {
+  foo(t[1]);   // expected-warning{{unchecked operation on raw buffer in expression}}
+  T ar[8];
+  foo(ar[5]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
+template void fArr<int>(int t[]); // expected-note {{in instantiation of function template specialization 'fArr<int>' requested here}}
+
+int testReturn(int t[]) {
+   return t[1]; // expected-warning{{unchecked operation on raw buffer in expression}}
+}
+
+//FIXME: Array access warnings on 0-indices;ArraySubscriptGadget excludes 0 index for both raw pointers and arrays!
+int testArrayAccesses(int n) {
+    
+    // auto deduced array type
+    int cArr[2][3] = {{1, 2, 3}, {4, 5, 6}};
+    int d = cArr[0][0];
+    foo(cArr[0][0]);
+    foo(cArr[1][2]);     // expected-warning2{{unchecked operation on raw buffer in expression}}
+    auto cPtr = cArr[1][2];  // expected-warning2{{unchecked operation on raw buffer in expression}}
+    foo(cPtr);
+    
+    // Typdefs
+    typedef int A[3];
+    const A tArr = {4, 5, 6};
+    foo(tArr[0], tArr[1]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+    return cArr[0][1];  // expected-warning{{unchecked operation on raw buffer in expression}}
 }
 
+
+
+
+
 #endif