[-Wunsafe-buffer-usage] Ignore safe array subscripts

Don't emit warnings for array subscripts on constant size arrays where the index is constant and within bounds.

Example:
int arr[10];
arr[5] = 0; //safe, no warning

This patch recognizes only array indices that are integer literals - it doesn't understand more complex expressions (arithmetic on constants, etc.).

Author: jkorous-apple

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -406,6 +406,31 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
   }
   return false;
 }
+
+AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
+  const DeclRefExpr * BaseDRE = dyn_cast_or_null<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
+  if (!BaseDRE)
+    return false;
+  if (!BaseDRE->getDecl())
+    return false;
+  auto BaseVarDeclTy = BaseDRE->getDecl()->getType();
+  if (!BaseVarDeclTy->isConstantArrayType())
+    return false;
+  const auto * CATy = dyn_cast_or_null<ConstantArrayType>(BaseVarDeclTy);
+  if (!CATy)
+    return false;
+  const APInt ArrSize = CATy->getSize();
+
+  if (const auto * IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
+    const APInt ArrIdx = IdxLit->getValue();
+    // FIXME: ArrIdx.isNegative() we could immediately emit an error as that's a bug
+    if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < ArrSize.getLimitedValue())
+      return true;
+  }
+
+  return false;
+}
+
 } // namespace clang::ast_matchers
 
 namespace {
@@ -598,16 +623,16 @@ class ArraySubscriptGadget : public WarningGadget {
   }
 
   static Matcher matcher() {
-    // FIXME: What if the index is integer literal 0? Should this be
-    // a safe gadget in this case?
-      // clang-format off
+    // clang-format off
       return stmt(arraySubscriptExpr(
             hasBase(ignoringParenImpCasts(
               anyOf(hasPointerType(), hasArrayType()))),
-            unless(hasIndex(
-                anyOf(integerLiteral(equals(0)), arrayInitIndexExpr())
-             )))
-            .bind(ArraySubscrTag));
+            unless(anyOf(
+              isSafeArraySubscript(),
+              hasIndex(
+                  anyOf(integerLiteral(equals(0)), arrayInitIndexExpr())
+              )
+            ))).bind(ArraySubscrTag));
     // clang-format on
   }
 

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

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN: %clang_cc1 -std=c++20 -Wno-everything -Wunsafe-buffer-usage \
 // RUN:            -fsafe-buffer-usage-suggestions \
 // RUN:            -verify %s
 
@@ -22,3 +22,19 @@ struct Foo {
 void foo2(Foo& f, unsigned idx) {
   f.member_buffer[idx] = 0; // expected-warning{{unsafe buffer access}}
 }
+
+void constant_idx_safe(unsigned idx) {
+  int buffer[10];
+  buffer[9] = 0;
+}
+
+void constant_idx_safe0(unsigned idx) {
+  int buffer[10];
+  buffer[0] = 0;
+}
+
+void constant_idx_unsafe(unsigned idx) {
+  int buffer[10];       // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}}
+                        // expected-note@-1{{change type of 'buffer' to 'std::array' to harden it}}
+  buffer[10] = 0;       // expected-note{{used in buffer access here}}
+}