[clang] [analyzer] Avoid unnecessary super region invalidation in `CStringChecker` (PR #146212)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 3 08:52:52 PDT 2025
https://github.com/flovent updated https://github.com/llvm/llvm-project/pull/146212
>From 9da53c788fc01cd3fc2dd4c178b836035b5d380b Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Sat, 28 Jun 2025 20:58:43 +0800
Subject: [PATCH 01/10] [analyzer] Avoid unnecessary super region invalidation
in `CStringChecker`
---
.../Checkers/CStringChecker.cpp | 77 ++++++++++++-
.../cstring-should-not-invalidate.cpp | 107 ++++++++++++++++++
2 files changed, 178 insertions(+), 6 deletions(-)
create mode 100644 clang/test/Analysis/cstring-should-not-invalidate.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 4d12fdcec1f1a..433fd2ce5f292 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -272,7 +272,8 @@ class CStringChecker : public Checker< eval::Call,
static ProgramStateRef
invalidateDestinationBufferBySize(CheckerContext &C, ProgramStateRef S,
const Expr *BufE, ConstCFGElementRef Elem,
- SVal BufV, SVal SizeV, QualType SizeTy);
+ SVal BufV, SVal SizeV, QualType SizeTy,
+ bool CouldAccessOutOfBound = true);
/// Operation never overflows, do not invalidate the super region.
static ProgramStateRef invalidateDestinationBufferNeverOverflows(
@@ -1211,14 +1212,17 @@ bool CStringChecker::isFirstBufInBound(CheckerContext &C, ProgramStateRef State,
ProgramStateRef CStringChecker::invalidateDestinationBufferBySize(
CheckerContext &C, ProgramStateRef S, const Expr *BufE,
- ConstCFGElementRef Elem, SVal BufV, SVal SizeV, QualType SizeTy) {
+ ConstCFGElementRef Elem, SVal BufV, SVal SizeV, QualType SizeTy,
+ bool CouldAccessOutOfBound) {
auto InvalidationTraitOperations =
- [&C, S, BufTy = BufE->getType(), BufV, SizeV,
- SizeTy](RegionAndSymbolInvalidationTraits &ITraits, const MemRegion *R) {
+ [&C, S, BufTy = BufE->getType(), BufV, SizeV, SizeTy,
+ CouldAccessOutOfBound](RegionAndSymbolInvalidationTraits &ITraits,
+ const MemRegion *R) {
// If destination buffer is a field region and access is in bound, do
// not invalidate its super region.
if (MemRegion::FieldRegionKind == R->getKind() &&
- isFirstBufInBound(C, S, BufV, BufTy, SizeV, SizeTy)) {
+ (!CouldAccessOutOfBound ||
+ isFirstBufInBound(C, S, BufV, BufTy, SizeV, SizeTy))) {
ITraits.setTrait(
R,
RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion);
@@ -2223,6 +2227,67 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
Result = lastElement;
}
+ // For bounded method, amountCopied take the minimum of two values,
+ // for ConcatFnKind::strlcat:
+ // amountCopied = min (size - dstLen - 1 , srcLen)
+ // for others:
+ // amountCopied = min (srcLen, size)
+ // So even if we don't know about amountCopied, as long as one of them will
+ // not cause an out-of-bound access, the whole function's operation will not
+ // too, that will avoid invalidating the superRegion of data member in that
+ // situation.
+ bool CouldAccessOutOfBound = true;
+ if (IsBounded && amountCopied.isUnknown()) {
+ // Get the max number of characters to copy.
+ SizeArgExpr lenExpr = {{Call.getArgExpr(2), 2}};
+ SVal lenVal = state->getSVal(lenExpr.Expression, LCtx);
+
+ // Protect against misdeclared strncpy().
+ lenVal =
+ svalBuilder.evalCast(lenVal, sizeTy, lenExpr.Expression->getType());
+
+ std::optional<NonLoc> lenValNL = lenVal.getAs<NonLoc>();
+
+ auto CouldAccessOutOfBoundForSVal = [&](NonLoc Val) -> bool {
+ return !isFirstBufInBound(C, state, C.getSVal(Dst.Expression),
+ Dst.Expression->getType(), Val,
+ C.getASTContext().getSizeType());
+ };
+
+ if (strLengthNL) {
+ CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*strLengthNL);
+ }
+
+ if (CouldAccessOutOfBound && lenValNL) {
+ switch (appendK) {
+ case ConcatFnKind::none:
+ case ConcatFnKind::strcat: {
+ CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*lenValNL);
+ break;
+ }
+ case ConcatFnKind::strlcat: {
+ if (!dstStrLengthNL)
+ break;
+
+ SVal freeSpace = svalBuilder.evalBinOpNN(state, BO_Sub, *lenValNL,
+ *dstStrLengthNL, sizeTy);
+ if (!isa<NonLoc>(freeSpace))
+ break;
+
+ freeSpace =
+ svalBuilder.evalBinOp(state, BO_Sub, freeSpace,
+ svalBuilder.makeIntVal(1, sizeTy), sizeTy);
+ std::optional<NonLoc> freeSpaceNL = freeSpace.getAs<NonLoc>();
+ if (!freeSpaceNL)
+ break;
+
+ CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*freeSpaceNL);
+ break;
+ }
+ }
+ }
+ }
+
// Invalidate the destination (regular invalidation without pointer-escaping
// the address of the top-level region). This must happen before we set the
// C string length because invalidation will clear the length.
@@ -2232,7 +2297,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// string, but that's still an improvement over blank invalidation.
state = invalidateDestinationBufferBySize(
C, state, Dst.Expression, Call.getCFGElementRef(), *dstRegVal,
- amountCopied, C.getASTContext().getSizeType());
+ amountCopied, C.getASTContext().getSizeType(), CouldAccessOutOfBound);
// Invalidate the source (const-invalidation without const-pointer-escaping
// the address of the top-level region).
diff --git a/clang/test/Analysis/cstring-should-not-invalidate.cpp b/clang/test/Analysis/cstring-should-not-invalidate.cpp
new file mode 100644
index 0000000000000..14c92447c52ca
--- /dev/null
+++ b/clang/test/Analysis/cstring-should-not-invalidate.cpp
@@ -0,0 +1,107 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
+// -analyzer-config c++-inlining=constructors -verify %s
+
+// expected-no-diagnostics
+
+typedef unsigned int size_t;
+
+char *strncpy(char *dest, const char *src, size_t x);
+
+// issue 143807
+struct strncpyTestClass {
+ int *m_ptr;
+ char m_buff[1000];
+
+ void KnownLen(char *src) {
+ m_ptr = new int;
+ strncpy(m_buff, src, sizeof(m_buff)); // known len but unknown src size
+ delete m_ptr; // no warning
+ }
+
+ void KnownSrcLen(size_t n) {
+ m_ptr = new int;
+ strncpy(m_buff, "xyz", n); // known src size but unknown len
+ delete m_ptr; // no warning
+ }
+};
+
+void strncpyTest(char *src, size_t n) {
+ strncpyTestClass rep;
+ rep.KnownLen(src);
+ rep.KnownSrcLen(n);
+}
+
+size_t strlcpy(char *dest, const char *src, size_t size);
+
+struct strlcpyTestClass {
+ int *m_ptr;
+ char m_buff[1000];
+
+ void KnownLen(char *src) {
+ m_ptr = new int;
+ strlcpy(m_buff, src, sizeof(m_buff)); // known len but unknown src size
+ delete m_ptr; // no warning
+ }
+
+ void KnownSrcLen(size_t n) {
+ m_ptr = new int;
+ strlcpy(m_buff, "xyz", n); // known src size but unknown len
+ delete m_ptr; // no warning
+ }
+};
+
+void strlcpyTest(char *src, size_t n) {
+ strlcpyTestClass rep;
+ rep.KnownLen(src);
+ rep.KnownSrcLen(n);
+}
+
+char *strncat(char *s1, const char *s2, size_t n);
+
+struct strncatTestClass {
+ int *m_ptr;
+ char m_buff[1000];
+
+ void KnownLen(char *src) {
+ m_ptr = new int;
+ strncat(m_buff, src, sizeof(m_buff)); // known len but unknown src size
+ delete m_ptr; // no warning
+ }
+
+ void KnownSrcLen(size_t n) {
+ m_ptr = new int;
+ strncat(m_buff, "xyz", n); // known src size but unknown len
+ delete m_ptr; // no warning
+ }
+};
+
+void strncatTest(char *src, size_t n) {
+ strncatTestClass rep;
+ rep.KnownLen(src);
+ rep.KnownSrcLen(n);
+}
+
+size_t strlcat(char *dst, const char *src, size_t size);
+
+struct strlcatTestClass {
+ int *m_ptr;
+ char m_buff[1000];
+
+ void KnownLen(char *src) {
+ m_ptr = new int;
+ strlcat(m_buff, src, sizeof(m_buff)); // known len but unknown src size
+ delete m_ptr; // no warning
+ }
+
+ void KnownSrcLen(size_t n) {
+ m_ptr = new int;
+ strlcat(m_buff, "xyz", n); // known src size but unknown len
+ delete m_ptr; // no warning
+ }
+};
+
+void strlcatTest(char *src, size_t n) {
+ strlcatTestClass rep;
+ rep.KnownLen(src);
+ rep.KnownSrcLen(n);
+}
>From 60e75d062e4753be437bb923a063c2c85efe9cba Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Mon, 30 Jun 2025 22:54:25 +0800
Subject: [PATCH 02/10] address comment for testcase
---
clang/test/Analysis/cstring-should-not-invalidate.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/test/Analysis/cstring-should-not-invalidate.cpp b/clang/test/Analysis/cstring-should-not-invalidate.cpp
index 14c92447c52ca..f0c5e5fc3c490 100644
--- a/clang/test/Analysis/cstring-should-not-invalidate.cpp
+++ b/clang/test/Analysis/cstring-should-not-invalidate.cpp
@@ -1,9 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-// -analyzer-config c++-inlining=constructors -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
// expected-no-diagnostics
-typedef unsigned int size_t;
+typedef decltype(sizeof(int)) size_t;
char *strncpy(char *dest, const char *src, size_t x);
>From 22d662b0eab60de987b3c12ae6475dfb46bc9efb Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Mon, 30 Jun 2025 23:20:14 +0800
Subject: [PATCH 03/10] refactor: use
`invalidateDestinationBufferNeverOverflows`
---
.../Checkers/CStringChecker.cpp | 24 +++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 433fd2ce5f292..04c9a5957bf93 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -272,8 +272,7 @@ class CStringChecker : public Checker< eval::Call,
static ProgramStateRef
invalidateDestinationBufferBySize(CheckerContext &C, ProgramStateRef S,
const Expr *BufE, ConstCFGElementRef Elem,
- SVal BufV, SVal SizeV, QualType SizeTy,
- bool CouldAccessOutOfBound = true);
+ SVal BufV, SVal SizeV, QualType SizeTy);
/// Operation never overflows, do not invalidate the super region.
static ProgramStateRef invalidateDestinationBufferNeverOverflows(
@@ -1212,17 +1211,14 @@ bool CStringChecker::isFirstBufInBound(CheckerContext &C, ProgramStateRef State,
ProgramStateRef CStringChecker::invalidateDestinationBufferBySize(
CheckerContext &C, ProgramStateRef S, const Expr *BufE,
- ConstCFGElementRef Elem, SVal BufV, SVal SizeV, QualType SizeTy,
- bool CouldAccessOutOfBound) {
+ ConstCFGElementRef Elem, SVal BufV, SVal SizeV, QualType SizeTy) {
auto InvalidationTraitOperations =
- [&C, S, BufTy = BufE->getType(), BufV, SizeV, SizeTy,
- CouldAccessOutOfBound](RegionAndSymbolInvalidationTraits &ITraits,
- const MemRegion *R) {
+ [&C, S, BufTy = BufE->getType(), BufV, SizeV,
+ SizeTy](RegionAndSymbolInvalidationTraits &ITraits, const MemRegion *R) {
// If destination buffer is a field region and access is in bound, do
// not invalidate its super region.
if (MemRegion::FieldRegionKind == R->getKind() &&
- (!CouldAccessOutOfBound ||
- isFirstBufInBound(C, S, BufV, BufTy, SizeV, SizeTy))) {
+ isFirstBufInBound(C, S, BufV, BufTy, SizeV, SizeTy)) {
ITraits.setTrait(
R,
RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion);
@@ -2295,9 +2291,13 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// can use LazyCompoundVals to copy the source values into the destination.
// This would probably remove any existing bindings past the end of the
// string, but that's still an improvement over blank invalidation.
- state = invalidateDestinationBufferBySize(
- C, state, Dst.Expression, Call.getCFGElementRef(), *dstRegVal,
- amountCopied, C.getASTContext().getSizeType(), CouldAccessOutOfBound);
+ if (CouldAccessOutOfBound)
+ state = invalidateDestinationBufferBySize(
+ C, state, Dst.Expression, Call.getCFGElementRef(), *dstRegVal,
+ amountCopied, C.getASTContext().getSizeType());
+ else
+ state = invalidateDestinationBufferNeverOverflows(
+ C, state, Call.getCFGElementRef(), *dstRegVal);
// Invalidate the source (const-invalidation without const-pointer-escaping
// the address of the top-level region).
>From 1926e7ece429a28c3fa0a8b301582184b3d792f9 Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Wed, 2 Jul 2025 23:18:17 +0800
Subject: [PATCH 04/10] use UpperCamelCase for variables and remove SizeArgExpr
usage
---
.../Checkers/CStringChecker.cpp | 28 +++++++++----------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 04c9a5957bf93..649fb6014ef6d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2235,14 +2235,14 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
bool CouldAccessOutOfBound = true;
if (IsBounded && amountCopied.isUnknown()) {
// Get the max number of characters to copy.
- SizeArgExpr lenExpr = {{Call.getArgExpr(2), 2}};
- SVal lenVal = state->getSVal(lenExpr.Expression, LCtx);
+ const Expr *LenExpr = Call.getArgExpr(2);
+ SVal LenVal = state->getSVal(LenExpr, LCtx);
// Protect against misdeclared strncpy().
- lenVal =
- svalBuilder.evalCast(lenVal, sizeTy, lenExpr.Expression->getType());
+ LenVal =
+ svalBuilder.evalCast(LenVal, sizeTy, LenExpr->getType());
- std::optional<NonLoc> lenValNL = lenVal.getAs<NonLoc>();
+ std::optional<NonLoc> LenValNL = LenVal.getAs<NonLoc>();
auto CouldAccessOutOfBoundForSVal = [&](NonLoc Val) -> bool {
return !isFirstBufInBound(C, state, C.getSVal(Dst.Expression),
@@ -2254,30 +2254,30 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*strLengthNL);
}
- if (CouldAccessOutOfBound && lenValNL) {
+ if (CouldAccessOutOfBound && LenValNL) {
switch (appendK) {
case ConcatFnKind::none:
case ConcatFnKind::strcat: {
- CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*lenValNL);
+ CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*LenValNL);
break;
}
case ConcatFnKind::strlcat: {
if (!dstStrLengthNL)
break;
- SVal freeSpace = svalBuilder.evalBinOpNN(state, BO_Sub, *lenValNL,
+ SVal FreeSpace = svalBuilder.evalBinOpNN(state, BO_Sub, *LenValNL,
*dstStrLengthNL, sizeTy);
- if (!isa<NonLoc>(freeSpace))
+ if (!isa<NonLoc>(FreeSpace))
break;
- freeSpace =
- svalBuilder.evalBinOp(state, BO_Sub, freeSpace,
+ FreeSpace =
+ svalBuilder.evalBinOp(state, BO_Sub, FreeSpace,
svalBuilder.makeIntVal(1, sizeTy), sizeTy);
- std::optional<NonLoc> freeSpaceNL = freeSpace.getAs<NonLoc>();
- if (!freeSpaceNL)
+ std::optional<NonLoc> FreeSpaceNL = FreeSpace.getAs<NonLoc>();
+ if (!FreeSpaceNL)
break;
- CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*freeSpaceNL);
+ CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*FreeSpaceNL);
break;
}
}
>From 89f195c4852a01d354dec032991f4551a21c6ec0 Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 3 Jul 2025 21:14:51 +0800
Subject: [PATCH 05/10] use `size` instead of `size - dstLen - 1` for strlcat
kind
---
.../Checkers/CStringChecker.cpp | 50 ++++++-------------
1 file changed, 14 insertions(+), 36 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 649fb6014ef6d..29c0dc95929ac 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2234,16 +2234,6 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// situation.
bool CouldAccessOutOfBound = true;
if (IsBounded && amountCopied.isUnknown()) {
- // Get the max number of characters to copy.
- const Expr *LenExpr = Call.getArgExpr(2);
- SVal LenVal = state->getSVal(LenExpr, LCtx);
-
- // Protect against misdeclared strncpy().
- LenVal =
- svalBuilder.evalCast(LenVal, sizeTy, LenExpr->getType());
-
- std::optional<NonLoc> LenValNL = LenVal.getAs<NonLoc>();
-
auto CouldAccessOutOfBoundForSVal = [&](NonLoc Val) -> bool {
return !isFirstBufInBound(C, state, C.getSVal(Dst.Expression),
Dst.Expression->getType(), Val,
@@ -2254,33 +2244,21 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*strLengthNL);
}
- if (CouldAccessOutOfBound && LenValNL) {
- switch (appendK) {
- case ConcatFnKind::none:
- case ConcatFnKind::strcat: {
+ if (CouldAccessOutOfBound) {
+ // Get the max number of characters to copy.
+ const Expr *LenExpr = Call.getArgExpr(2);
+ SVal LenVal = state->getSVal(LenExpr, LCtx);
+
+ // Protect against misdeclared strncpy().
+ LenVal = svalBuilder.evalCast(LenVal, sizeTy, LenExpr->getType());
+
+ std::optional<NonLoc> LenValNL = LenVal.getAs<NonLoc>();
+
+ // Because analyzer doesn't handle expressions like `size -
+ // dstLen - 1` very well, we roughly use `size` for
+ // ConcatFnKind::strlcat here, same with other concat kinds.
+ if (LenValNL)
CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*LenValNL);
- break;
- }
- case ConcatFnKind::strlcat: {
- if (!dstStrLengthNL)
- break;
-
- SVal FreeSpace = svalBuilder.evalBinOpNN(state, BO_Sub, *LenValNL,
- *dstStrLengthNL, sizeTy);
- if (!isa<NonLoc>(FreeSpace))
- break;
-
- FreeSpace =
- svalBuilder.evalBinOp(state, BO_Sub, FreeSpace,
- svalBuilder.makeIntVal(1, sizeTy), sizeTy);
- std::optional<NonLoc> FreeSpaceNL = FreeSpace.getAs<NonLoc>();
- if (!FreeSpaceNL)
- break;
-
- CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*FreeSpaceNL);
- break;
- }
- }
}
}
>From 86fc085fd0fcb51206889b49fe24bb8b4a69117e Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 3 Jul 2025 21:37:00 +0800
Subject: [PATCH 06/10] [NFC] little optimize
---
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 29c0dc95929ac..8de6f4cdabe41 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2252,12 +2252,10 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// Protect against misdeclared strncpy().
LenVal = svalBuilder.evalCast(LenVal, sizeTy, LenExpr->getType());
- std::optional<NonLoc> LenValNL = LenVal.getAs<NonLoc>();
-
// Because analyzer doesn't handle expressions like `size -
// dstLen - 1` very well, we roughly use `size` for
// ConcatFnKind::strlcat here, same with other concat kinds.
- if (LenValNL)
+ if (std::optional<NonLoc> LenValNL = LenVal.getAs<NonLoc>())
CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*LenValNL);
}
}
>From f433147f5ccffb666e1ca53ef71e37ab87d9878d Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 3 Jul 2025 22:02:22 +0800
Subject: [PATCH 07/10] check optional<NonLoc>'s value in lambda (NFC)
---
.../StaticAnalyzer/Checkers/CStringChecker.cpp | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 8de6f4cdabe41..31cb150892a5d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2234,15 +2234,16 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// situation.
bool CouldAccessOutOfBound = true;
if (IsBounded && amountCopied.isUnknown()) {
- auto CouldAccessOutOfBoundForSVal = [&](NonLoc Val) -> bool {
+ auto CouldAccessOutOfBoundForSVal =
+ [&](std::optional<NonLoc> Val) -> bool {
+ if (!Val)
+ return true;
return !isFirstBufInBound(C, state, C.getSVal(Dst.Expression),
- Dst.Expression->getType(), Val,
+ Dst.Expression->getType(), *Val,
C.getASTContext().getSizeType());
};
- if (strLengthNL) {
- CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*strLengthNL);
- }
+ CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(strLengthNL);
if (CouldAccessOutOfBound) {
// Get the max number of characters to copy.
@@ -2255,8 +2256,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// Because analyzer doesn't handle expressions like `size -
// dstLen - 1` very well, we roughly use `size` for
// ConcatFnKind::strlcat here, same with other concat kinds.
- if (std::optional<NonLoc> LenValNL = LenVal.getAs<NonLoc>())
- CouldAccessOutOfBound = CouldAccessOutOfBoundForSVal(*LenValNL);
+ CouldAccessOutOfBound =
+ CouldAccessOutOfBoundForSVal(LenVal.getAs<NonLoc>());
}
}
>From 02ad1b9a3356ccf3d2816a5087f905183a5e4668 Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 3 Jul 2025 22:07:47 +0800
Subject: [PATCH 08/10] add `alpha.unix.cstring` and `unix.cstring` to checker
---
clang/test/Analysis/cstring-should-not-invalidate.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/Analysis/cstring-should-not-invalidate.cpp b/clang/test/Analysis/cstring-should-not-invalidate.cpp
index f0c5e5fc3c490..363415fb7e91a 100644
--- a/clang/test/Analysis/cstring-should-not-invalidate.cpp
+++ b/clang/test/Analysis/cstring-should-not-invalidate.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
// expected-no-diagnostics
@@ -63,7 +63,7 @@ struct strncatTestClass {
void KnownLen(char *src) {
m_ptr = new int;
- strncat(m_buff, src, sizeof(m_buff)); // known len but unknown src size
+ strncat(m_buff, src, sizeof(m_buff) - 1); // known len but unknown src size
delete m_ptr; // no warning
}
>From 23596bb61ebce23fe4e44003eb792d258e47e1f9 Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 3 Jul 2025 22:44:52 +0800
Subject: [PATCH 09/10] Add `debug.ExprInspection` to produce diagnostic that
prove other members is not invalidate
---
.../cstring-should-not-invalidate.cpp | 23 +++++++++++++------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/clang/test/Analysis/cstring-should-not-invalidate.cpp b/clang/test/Analysis/cstring-should-not-invalidate.cpp
index 363415fb7e91a..678274b9a0065 100644
--- a/clang/test/Analysis/cstring-should-not-invalidate.cpp
+++ b/clang/test/Analysis/cstring-should-not-invalidate.cpp
@@ -1,13 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
-
-// expected-no-diagnostics
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -verify %s
typedef decltype(sizeof(int)) size_t;
+void clang_analyzer_eval(bool);
char *strncpy(char *dest, const char *src, size_t x);
+constexpr int initB = 100;
+struct Base {
+ int b;
+ Base(): b(initB) {}
+};
+
// issue 143807
-struct strncpyTestClass {
+struct strncpyTestClass: public Base {
int *m_ptr;
char m_buff[1000];
@@ -28,11 +33,12 @@ void strncpyTest(char *src, size_t n) {
strncpyTestClass rep;
rep.KnownLen(src);
rep.KnownSrcLen(n);
+ clang_analyzer_eval(rep.b == initB); // expected-warning{{TRUE}}
}
size_t strlcpy(char *dest, const char *src, size_t size);
-struct strlcpyTestClass {
+struct strlcpyTestClass: public Base {
int *m_ptr;
char m_buff[1000];
@@ -53,11 +59,12 @@ void strlcpyTest(char *src, size_t n) {
strlcpyTestClass rep;
rep.KnownLen(src);
rep.KnownSrcLen(n);
+ clang_analyzer_eval(rep.b == initB); // expected-warning{{TRUE}}
}
char *strncat(char *s1, const char *s2, size_t n);
-struct strncatTestClass {
+struct strncatTestClass: public Base {
int *m_ptr;
char m_buff[1000];
@@ -78,11 +85,12 @@ void strncatTest(char *src, size_t n) {
strncatTestClass rep;
rep.KnownLen(src);
rep.KnownSrcLen(n);
+ clang_analyzer_eval(rep.b == initB); // expected-warning{{TRUE}}
}
size_t strlcat(char *dst, const char *src, size_t size);
-struct strlcatTestClass {
+struct strlcatTestClass: public Base {
int *m_ptr;
char m_buff[1000];
@@ -103,4 +111,5 @@ void strlcatTest(char *src, size_t n) {
strlcatTestClass rep;
rep.KnownLen(src);
rep.KnownSrcLen(n);
+ clang_analyzer_eval(rep.b == initB); // expected-warning{{TRUE}}
}
>From 38e2027fad82afa95d0c489397432d075446266d Mon Sep 17 00:00:00 2001
From: flovent <flbven at protonmail.com>
Date: Thu, 3 Jul 2025 23:52:30 +0800
Subject: [PATCH 10/10] add an extra testcase for
alpha.unix.cstring.OutOfBounds
---
.../Analysis/cstring-should-not-invalidate.cpp | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/clang/test/Analysis/cstring-should-not-invalidate.cpp b/clang/test/Analysis/cstring-should-not-invalidate.cpp
index 678274b9a0065..aacce5b8debde 100644
--- a/clang/test/Analysis/cstring-should-not-invalidate.cpp
+++ b/clang/test/Analysis/cstring-should-not-invalidate.cpp
@@ -88,6 +88,24 @@ void strncatTest(char *src, size_t n) {
clang_analyzer_eval(rep.b == initB); // expected-warning{{TRUE}}
}
+struct strncatReportOutOfBoundTestClass: public Base {
+ int *m_ptr;
+ char m_buff[1000];
+
+ void KnownLen(char *src) {
+ m_ptr = new int;
+ // expected-warning at +1{{String concatenation function overflows the destination buffer}}
+ strncat(m_buff, src, sizeof(m_buff)); // known len but unknown src size
+ delete m_ptr; // no warning
+ }
+};
+
+void strncatReportOutOfBoundTest(char *src, size_t n) {
+ strncatReportOutOfBoundTestClass rep;
+ rep.KnownLen(src);
+ clang_analyzer_eval(rep.b == initB); // expected-warning{{TRUE}}
+}
+
size_t strlcat(char *dst, const char *src, size_t size);
struct strlcatTestClass: public Base {
More information about the cfe-commits
mailing list