https://github.com/jkorous-apple updated
https://github.com/llvm/llvm-project/pull/92432
>From 457353172af271ca6395c8fb01f2091846dabb7f Mon Sep 17 00:00:00 2001
From: Jan Korous <jkor...@apple.com>
Date: Thu, 16 May 2024 10:20:08 -0700
Subject: [PATCH 1/2] [-Wunsafe-buffer-usage] Fix false positives for constant
cases
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 67 +++++++++++++++----
...r-usage-constant-buffer-constant-index.cpp | 17 +++++
2 files changed, 70 insertions(+), 14 deletions(-)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-constant-buffer-constant-index.cpp
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
>From 7e2f59b739fd7545e8a1ff0ba17970293e885e35 Mon Sep 17 00:00:00 2001
From: Jan Korous <jkor...@apple.com>
Date: Thu, 16 May 2024 10:26:40 -0700
Subject: [PATCH 2/2] clang-format
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 34 ++++++++++++------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index dc265b9039a5f..927f5fc8f0fa1 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -421,15 +421,15 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// - call both from Sema and from here
if (const auto *BaseDRE =
- dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts())) {
+ dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts())) {
if (!BaseDRE->getDecl())
return false;
if (const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
- BaseDRE->getDecl()->getType())) {
+ 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
+ // FIXME: ArrIdx.isNegative() we could immediately emit an error as
+ // that's a bug
if (ArrIdx.isNonNegative() &&
ArrIdx.getLimitedValue() < CATy->getLimitedSize())
return true;
@@ -438,16 +438,16 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
}
if (const auto *BaseSL =
- dyn_cast<StringLiteral>(Node.getBase()->IgnoreParenImpCasts())) {
- if (const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
- BaseSL->getType())) {
+ 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;
+ 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;
}
}
}
@@ -459,13 +459,12 @@ AST_MATCHER(BinaryOperator, isSafePtrArithmetic) {
if (Node.getOpcode() != BinaryOperatorKind::BO_Add)
return false;
- const auto *LHSDRE =
- dyn_cast<DeclRefExpr>(Node.getLHS()->IgnoreImpCasts());
+ const auto *LHSDRE = dyn_cast<DeclRefExpr>(Node.getLHS()->IgnoreImpCasts());
if (!LHSDRE)
return false;
const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
- LHSDRE->getDecl()->getType());
+ LHSDRE->getDecl()->getType());
if (!CATy)
return false;
@@ -731,7 +730,8 @@ class PointerArithmeticGadget : public WarningGadget {
hasLHS(expr(hasPointerType()).bind(PointerArithmeticPointerTag)),
hasRHS(HasIntegerType));
- return stmt(binaryOperator(anyOf(PtrAtLeft, PtrAtRight),
unless(isSafePtrArithmetic()))
+ return stmt(binaryOperator(anyOf(PtrAtLeft, PtrAtRight),
+ unless(isSafePtrArithmetic()))
.bind(PointerArithmeticTag));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
