[clang] [-Wunsafe-buffer-usage] Fix false positives for string literals (PR #115554)

via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 8 14:13:37 PST 2024


https://github.com/jkorous-apple created https://github.com/llvm/llvm-project/pull/115554

fixes rdar://139106996

>From 8db304f11ba708abd096b4b8df998c55548e5b4d Mon Sep 17 00:00:00 2001
From: Jan Korous <jkorous at apple.com>
Date: Fri, 8 Nov 2024 14:07:19 -0800
Subject: [PATCH] [-Wunsafe-buffer-usage] Fix false positives for string
 literals

# Conflicts:
#	clang/lib/Analysis/UnsafeBufferUsage.cpp
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 26 ++++++++++++-------
 ...arn-unsafe-buffer-usage-string-literal.cpp | 18 +++++++++++++
 2 files changed, 34 insertions(+), 10 deletions(-)
 create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-string-literal.cpp

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..116d098075b6bf 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -434,16 +434,22 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
   //    already duplicated
   //  - call both from Sema and from here
 
-  const auto *BaseDRE =
-      dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
-  if (!BaseDRE)
-    return false;
-  if (!BaseDRE->getDecl())
-    return false;
-  const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-      BaseDRE->getDecl()->getType());
-  if (!CATy)
-    return false;
+  APInt ArrSize{};
+  if (const auto *BaseDRE =
+    dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts())) {
+    if (!BaseDRE)
+      return false;
+    if (!BaseDRE->getDecl())
+      return false;
+    const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
+        BaseDRE->getDecl()->getType());
+    if (!CATy)
+      return false;
+    ArrSize = CATy->getSize();
+  } else if (const auto *BaseStrLit = dyn_cast<StringLiteral>(Node.getBase()->IgnoreParenImpCasts())) {
+    // Add 1 for the terminating null character.
+    ArrSize = APInt{64, BaseStrLit->getLength() + 1, false};
+  }
 
   if (const auto *IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
     const APInt ArrIdx = IdxLit->getValue();
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-string-literal.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-string-literal.cpp
new file mode 100644
index 00000000000000..e983a8f135d8a4
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-string-literal.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-everything -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -verify %s
+
+// CHECK-NOT: [-Wunsafe-buffer-usage]
+
+
+void foo(unsigned idx) {
+  char c = '0';
+  c = "abc"[0];
+  c = "abc"[1];
+  c = "abc"[2];
+  c = "abc"[3];
+  c = "abc"[4]; // expected-warning{{unsafe buffer access}}
+  c = "abc"[idx]; // expected-warning{{unsafe buffer access}}
+  c = ""[0];
+  c = ""[1]; // expected-warning{{unsafe buffer access}}
+}



More information about the cfe-commits mailing list