[clang] 6d1d055 - [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

[via cfe-commits]

Author: ziqingluo-90
Date: 2022-12-16T17:42:07-08:00
New Revision: 6d1d055fad502e0751d59c31ca915737b73e002f

URL: https://github.com/llvm/llvm-project/commit/6d1d055fad502e0751d59c31ca915737b73e002f
DIFF: https://github.com/llvm/llvm-project/commit/6d1d055fad502e0751d59c31ca915737b73e002f.diff

LOG: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

Add an unsafe Gadget in the Unsafe Buffer Usage anslysis to represent
(and search (via matchers) for) unsafe buffer-accessing array subscript operations.

Reviewed By: NoQ, jkorous, aaron.ballman, xazax.hun

Differential Revision: https://reviews.llvm.org/D137379

Added: 
    

Modified: 
    clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
    clang/lib/Analysis/UnsafeBufferUsage.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index e2d3cbe0e3955..fc8800fed3953 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -27,6 +27,7 @@
 
 UNSAFE_GADGET(Increment)
 UNSAFE_GADGET(Decrement)
+UNSAFE_GADGET(ArraySubscript)
 
 #undef SAFE_GADGET
 #undef UNSAFE_GADGET

diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index d2a3c49a87965..6dc2b2bc81063 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -129,6 +129,32 @@ class DecrementGadget : public UnsafeGadget {
 
   const UnaryOperator *getBaseStmt() const override { return Op; }
 };
+
+/// Array subscript expressions on raw pointers as if they're arrays. Unsafe as
+/// it doesn't have any bounds checks for the array.
+class ArraySubscriptGadget : public UnsafeGadget {
+  static constexpr const char *const ArraySubscrTag = "arraySubscr";
+  const ArraySubscriptExpr *ASE;
+
+public:
+  ArraySubscriptGadget(const MatchFinder::MatchResult &Result)
+      : UnsafeGadget(Kind::ArraySubscript),
+        ASE(Result.Nodes.getNodeAs<ArraySubscriptExpr>(ArraySubscrTag)) {}
+
+  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())))
+            .bind(ArraySubscrTag));
+  }
+
+  const ArraySubscriptExpr *getBaseStmt() const override { return ASE; }
+};
 } // namespace
 
 // Scan the function and return a list of gadgets found with provided kits.

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index 59f830d29e3b9..b84e05dae2f11 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -6,3 +6,63 @@ 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 foo(...);   // let arguments of `foo` to hold testing expressions
+
+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}}
+      );
+
+  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],
+      b[3][4],
+      4[b][3],
+      4[3[b]]);
+}
+
+void testArraySubscriptsWithAuto(int *p, int **pp) {
+  int a[10];
+
+  auto ap1 = a;
+
+  foo(ap1[0]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+
+  auto ap2 = p;
+
+  foo(ap2[0]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+
+  auto ap3 = pp;
+
+  foo(pp[0][0]); // expected-warning2{{unchecked operation on raw buffer in expression}}
+
+  auto ap4 = *pp;
+
+  foo(ap4[0]);  // 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}}
+}