[clang] [-Wunsafe-buffer-usage] Fix false positives for constant cases (PR #92432)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

Author: None (jkorous-apple)

<details>
<summary>Changes</summary>

addresses https://github.com/llvm/llvm-project/issues/92191

---
Full diff: https://github.com/llvm/llvm-project/pull/92432.diff


2 Files Affected:

- (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+53-14) 
- (added) clang/test/SemaCXX/warn-unsafe-buffer-usage-constant-buffer-constant-index.cpp (+17) 


``````````diff
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c42e70d5b95ac..dc265b9039a5f 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -420,25 +420,64 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
   //    already duplicated
   //  - call both from Sema and from here
 
-  const auto *BaseDRE =
-      dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
-  if (!BaseDRE)
+  if (const auto *BaseDRE =
+      dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts())) {
+    if (!BaseDRE->getDecl())
+      return false;
+    if (const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
+      BaseDRE->getDecl()->getType())) {
+      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() < CATy->getLimitedSize())
+          return true;
+      }
+    }
+  }
+
+  if (const auto *BaseSL =
+      dyn_cast<StringLiteral>(Node.getBase()->IgnoreParenImpCasts())) {
+    if (const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
+      BaseSL->getType())) {
+      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() < CATy->getLimitedSize())
+            return true;
+      }
+    }
+  }
+
+  return false;
+}
+
+AST_MATCHER(BinaryOperator, isSafePtrArithmetic) {
+  if (Node.getOpcode() != BinaryOperatorKind::BO_Add)
     return false;
-  if (!BaseDRE->getDecl())
+
+  const auto *LHSDRE =
+      dyn_cast<DeclRefExpr>(Node.getLHS()->IgnoreImpCasts());
+  if (!LHSDRE)
     return false;
+
   const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-      BaseDRE->getDecl()->getType());
+    LHSDRE->getDecl()->getType());
   if (!CATy)
     return false;
 
-  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() < CATy->getLimitedSize())
-      return true;
-  }
+  const auto *RHSIntLit = dyn_cast<IntegerLiteral>(Node.getRHS());
+  if (!RHSIntLit)
+    return false;
+
+  const APInt BufferOffset = RHSIntLit->getValue();
+
+  if (BufferOffset.isNonNegative() &&
+      BufferOffset.getLimitedValue() < CATy->getLimitedSize())
+    return true;
 
   return false;
 }
@@ -692,7 +731,7 @@ class PointerArithmeticGadget : public WarningGadget {
               hasLHS(expr(hasPointerType()).bind(PointerArithmeticPointerTag)),
               hasRHS(HasIntegerType));
 
-    return stmt(binaryOperator(anyOf(PtrAtLeft, PtrAtRight))
+    return stmt(binaryOperator(anyOf(PtrAtLeft, PtrAtRight), unless(isSafePtrArithmetic()))
                     .bind(PointerArithmeticTag));
   }
 
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-constant-buffer-constant-index.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-constant-buffer-constant-index.cpp
new file mode 100644
index 0000000000000..abacfcfee0296
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-constant-buffer-constant-index.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-everything -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -verify %s
+
+void char_literal() {
+  if ("abc"[2] == 'c')
+    return;
+  if ("def"[3] == '0')
+    return;
+}
+
+void const_size_buffer_arithmetic() {
+  char kBuf[64] = {};
+  const char* p = kBuf + 1;
+}
+
+// expected-no-diagnostics

``````````

</details>


https://github.com/llvm/llvm-project/pull/92432