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

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Mon May 20 11:59:45 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())) {
----------------
haoNoQ wrote:

I suspect that this could have been a:
```c++
const Expr *BaseE = Node.getBase()->IgnoreParenImpCasts();
if (isa<DeclRefExpr, StringLiteral>(BaseE)) {
  if (const auto *CATy = Finder->getASTContext().getAsConstantArrayType(BaseE->getType())) {
    ...
  }
}
```
Which would also eliminate duplication. (Unless `DeclRefExpr->getDecl()->getType()` is somehow significantly different from `DeclRefExpr->getType()`.)

Another thing we can try is to simply eliminate the `isa` entirely. Like, we know that it's "an" expression, and its type is a constant-size array type. Do we really need more? Why not simply trust the type system? (It's not like the C type system has ever lied to us right? 😅)

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


More information about the cfe-commits mailing list