[clang] [-Wunsafe-buffer-usage] Fix false positives for constant cases (PR #92432)
via cfe-commits
cfe-commits at lists.llvm.org
Thu May 16 12:48:13 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() &&
----------------
pkasting wrote:
I don't speak AST, but does this still handle (i.e. not warn on) cases like `constexpr char* p = (buf + 1) - 1;`? Or is the presence of any subtraction enough to trigger a warning?
What about `constexpr char* p = buf + 1; constexpr char* p2 = p - 1;`? Should we be able to detect that this is safe, and not warn?
https://github.com/llvm/llvm-project/pull/92432
More information about the cfe-commits
mailing list