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

via cfe-commits cfe-commits at lists.llvm.org
Mon May 20 11:32:56 PDT 2024


================
@@ -420,25 +420,63 @@ 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() &&
----------------
jkorous-apple wrote:

This code is just a short-term solution until we get to do it properly. I don't want to invest effort into polishing the code because we should replace it.

See this FIXME:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/UnsafeBufferUsage.cpp#L416

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


More information about the cfe-commits mailing list