[clang] [clang][analyzer] Model `strxfrm` (PR #156507)
Alejandro Álvarez Ayllón via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 5 06:00:37 PDT 2025
https://github.com/alejandro-alvarez-sonarsource updated https://github.com/llvm/llvm-project/pull/156507
>From 736d0dcf2e31f402607656bf100004f8c8dd6539 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Fri, 29 Aug 2025 09:51:04 +0200
Subject: [PATCH 1/8] [clang][analyzer] Model `strxfrm`
Signature:
```c
size_t strxfrm(char *dest, const char *src, size_t n);
```
The modeling covers:
* `src` can never be null
* `dest` can be null only if n is 0, and then the return value is some
unspecified positive integer
* `src` and `dest` must not overlap
* `dest` must have at least `n` bytes of capacity
* The return value can either be:
- `< n`, and the contents of the buffer pointed by `dest`
is invalidated
- `>= n`, and the contents of the buffer pointed by `dest`
is marked as undefined
CPP-6854
---
.../Checkers/CStringChecker.cpp | 100 ++++++++++++++++++
clang/test/Analysis/string.c | 57 ++++++++++
2 files changed, 157 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index cfc6d34a75ca2..296a803bd04c2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -101,6 +101,7 @@ class CStringChecker
static void *getTag() { static int tag; return &tag; }
+
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
void checkLiveSymbols(ProgramStateRef state, SymbolReaper &SR) const;
@@ -163,6 +164,7 @@ class CStringChecker
{{CDM::CLibrary, {"strcasecmp"}, 2}, &CStringChecker::evalStrcasecmp},
{{CDM::CLibrary, {"strncasecmp"}, 3}, &CStringChecker::evalStrncasecmp},
{{CDM::CLibrary, {"strsep"}, 2}, &CStringChecker::evalStrsep},
+ {{CDM::CLibrary, {"strxfrm"}, 3}, &CStringChecker::evalStrxfrm},
{{CDM::CLibrary, {"bcopy"}, 3}, &CStringChecker::evalBcopy},
{{CDM::CLibrary, {"bcmp"}, 3},
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
@@ -211,6 +213,8 @@ class CStringChecker
bool ReturnEnd, bool IsBounded, ConcatFnKind appendK,
bool returnPtr = true) const;
+ void evalStrxfrm(CheckerContext &C, const CallEvent &Call) const;
+
void evalStrcat(CheckerContext &C, const CallEvent &Call) const;
void evalStrncat(CheckerContext &C, const CallEvent &Call) const;
void evalStrlcat(CheckerContext &C, const CallEvent &Call) const;
@@ -2243,6 +2247,102 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
C.addTransition(state);
}
+void CStringChecker::evalStrxfrm(CheckerContext &C,
+ const CallEvent &Call) const {
+ // size_t strxfrm(char *dest, const char *src, size_t n);
+ CurrentFunctionDescription = "locale transformation function";
+
+ ProgramStateRef state = C.getState();
+ const LocationContext *LCtx = C.getLocationContext();
+ SValBuilder &SVB = C.getSValBuilder();
+
+ // Get arguments
+ DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+ SourceArgExpr Source = {{Call.getArgExpr(1), 1}};
+ SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
+
+ // `src` can never be null
+ SVal SrcVal = state->getSVal(Source.Expression, LCtx);
+ state = checkNonNull(C, state, Source, SrcVal);
+ if (!state)
+ return;
+
+ // Check overlaps
+ state = CheckOverlap(C, state, Size, Dest, Source, CK_Regular);
+ if (!state)
+ return;
+
+ // The function returns an implementation-defined length needed for
+ // transformation
+ SVal retVal = SVB.conjureSymbolVal(Call, C.blockCount());
+
+ state = state->BindExpr(Call.getOriginExpr(), LCtx, retVal);
+
+ // Check if size is zero
+ SVal sizeVal = state->getSVal(Size.Expression, LCtx);
+ QualType sizeTy = Size.Expression->getType();
+
+ auto [stateZeroSize, StateSizeNonZero] =
+ assumeZero(C, state, sizeVal, sizeTy);
+
+ // If `n` is 0, we just return the implementation defined length
+ if (stateZeroSize && !StateSizeNonZero) {
+ C.addTransition(stateZeroSize);
+ return;
+ }
+
+ if (!StateSizeNonZero)
+ return;
+
+ // If `n` is not 0, `dest` can not be null.
+ SVal destVal = state->getSVal(Dest.Expression, LCtx);
+ StateSizeNonZero = checkNonNull(C, StateSizeNonZero, Dest, destVal);
+ if (!StateSizeNonZero)
+ return;
+
+ // Check that we can write to the destination buffer
+ StateSizeNonZero = CheckBufferAccess(C, StateSizeNonZero, Dest, Size,
+ AccessKind::write, CK_Regular);
+ if (!StateSizeNonZero)
+ return;
+
+ // Success: return value < `n`
+ // Failure: return value >= `n`
+ auto comparisonVal = SVB.evalBinOp(StateSizeNonZero, BO_LT, retVal, sizeVal,
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+
+ if (comparisonVal) {
+ auto [StateSuccess, StateFailure] =
+ StateSizeNonZero->assume(*comparisonVal);
+
+ if (StateSuccess) {
+ // In this case, the transformation invalidated the buffer.
+ StateSuccess = invalidateDestinationBufferBySize(
+ C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), destVal,
+ sizeVal, Size.Expression->getType());
+
+ C.addTransition(StateSuccess);
+ }
+
+ if (StateFailure) {
+ // In this case, dest buffer content is undefined
+ if (std::optional<Loc> destLoc = destVal.getAs<Loc>()) {
+ StateFailure = StateFailure->bindLoc(*destLoc, UndefinedVal{}, LCtx);
+ }
+
+ C.addTransition(StateFailure);
+ }
+ } else {
+ // Fallback: invalidate the buffer.
+ StateSizeNonZero = invalidateDestinationBufferBySize(
+ C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), destVal,
+ sizeVal, Size.Expression->getType());
+
+ C.addTransition(StateSizeNonZero);
+ }
+}
+
void CStringChecker::evalStrcmp(CheckerContext &C,
const CallEvent &Call) const {
//int strcmp(const char *s1, const char *s2);
diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c
index e017aff3b4a1d..427b99bfdf295 100644
--- a/clang/test/Analysis/string.c
+++ b/clang/test/Analysis/string.c
@@ -1789,3 +1789,60 @@ void CWE124_Buffer_Underwrite__malloc_char_memcpy() {
free(dataBuffer);
}
#endif
+
+//===----------------------------------------------------------------------===
+// strxfrm()
+// It is not a built-in.
+//===----------------------------------------------------------------------===
+
+size_t strxfrm(char *dest, const char *src, size_t n);
+
+void strxfrm_null_dest(const char *src) {
+ strxfrm(NULL, src, 0); // no warning
+ strxfrm(NULL, src, 10); // expected-warning {{Null pointer passed as 1st argument}}
+}
+
+void strxfrm_null_source(char *dest) {
+ strxfrm(dest, NULL, 0); // expected-warning {{Null pointer passed as 2nd argument}}
+}
+
+#ifndef SUPPRESS_OUT_OF_BOUND
+void strxfrm_overflow(const char *src) {
+ char dest[10];
+ strxfrm(dest, src, 55); // expected-warning {{Locale transformation function overflows the destination buffer}}
+}
+#endif
+
+void strxfrm_source_smaller() {
+ char dest[10];
+ char source[5];
+ strxfrm(dest, source, 10);
+}
+
+void strxfrm_overlap(char *dest) {
+ strxfrm(dest, dest, 10); // expected-warning {{Arguments must not be overlapping buffers}}
+}
+
+void strxfrm_regular(const char *src) {
+ size_t n = strxfrm(NULL, src, 0);
+ char *dest = (char*)malloc(n + 1);
+ strxfrm(dest, src, n);
+ free(dest);
+}
+
+int strxfrm_dest_undef(const char *src) {
+ char dest[10] = {0};
+ size_t n = strxfrm(dest, src, sizeof(dest));
+
+ int c = 0;
+ if (n >= sizeof(dest)) {
+ for (int i = 0; i < sizeof(dest); ++i) {
+ c += dest[i]; // expected-warning {{Assigned value is uninitialized}}
+ }
+ } else {
+ for (int i = 0; i < sizeof(dest); ++i) {
+ c += dest[i]; // no-warning
+ }
+ }
+ return c;
+}
>From eea0b6b58d1a3673b7220d0f32310322282d0aa8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Wed, 3 Sep 2025 08:28:56 +0200
Subject: [PATCH 2/8] Format
---
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 296a803bd04c2..9a4457ec764bc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -101,7 +101,6 @@ class CStringChecker
static void *getTag() { static int tag; return &tag; }
-
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
void checkLiveSymbols(ProgramStateRef state, SymbolReaper &SR) const;
@@ -2336,8 +2335,8 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
} else {
// Fallback: invalidate the buffer.
StateSizeNonZero = invalidateDestinationBufferBySize(
- C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), destVal,
- sizeVal, Size.Expression->getType());
+ C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), destVal,
+ sizeVal, Size.Expression->getType());
C.addTransition(StateSizeNonZero);
}
>From 55ca6703ea51839541d45ec58c634b55abdd690b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Wed, 3 Sep 2025 08:33:18 +0200
Subject: [PATCH 3/8] Style
---
.../Checkers/CStringChecker.cpp | 50 +++++++++----------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 9a4457ec764bc..6b305e408354d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2251,7 +2251,7 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
// size_t strxfrm(char *dest, const char *src, size_t n);
CurrentFunctionDescription = "locale transformation function";
- ProgramStateRef state = C.getState();
+ ProgramStateRef State = C.getState();
const LocationContext *LCtx = C.getLocationContext();
SValBuilder &SVB = C.getSValBuilder();
@@ -2261,32 +2261,32 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
// `src` can never be null
- SVal SrcVal = state->getSVal(Source.Expression, LCtx);
- state = checkNonNull(C, state, Source, SrcVal);
- if (!state)
+ SVal SrcVal = State->getSVal(Source.Expression, LCtx);
+ State = checkNonNull(C, State, Source, SrcVal);
+ if (!State)
return;
// Check overlaps
- state = CheckOverlap(C, state, Size, Dest, Source, CK_Regular);
- if (!state)
+ State = CheckOverlap(C, State, Size, Dest, Source, CK_Regular);
+ if (!State)
return;
// The function returns an implementation-defined length needed for
// transformation
- SVal retVal = SVB.conjureSymbolVal(Call, C.blockCount());
+ SVal RetVal = SVB.conjureSymbolVal(Call, C.blockCount());
- state = state->BindExpr(Call.getOriginExpr(), LCtx, retVal);
+ State = State->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
// Check if size is zero
- SVal sizeVal = state->getSVal(Size.Expression, LCtx);
- QualType sizeTy = Size.Expression->getType();
+ SVal SizeVal = State->getSVal(Size.Expression, LCtx);
+ QualType SizeTy = Size.Expression->getType();
- auto [stateZeroSize, StateSizeNonZero] =
- assumeZero(C, state, sizeVal, sizeTy);
+ auto [StateZeroSize, StateSizeNonZero] =
+ assumeZero(C, State, SizeVal, SizeTy);
// If `n` is 0, we just return the implementation defined length
- if (stateZeroSize && !StateSizeNonZero) {
- C.addTransition(stateZeroSize);
+ if (StateZeroSize && !StateSizeNonZero) {
+ C.addTransition(StateZeroSize);
return;
}
@@ -2294,8 +2294,8 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
return;
// If `n` is not 0, `dest` can not be null.
- SVal destVal = state->getSVal(Dest.Expression, LCtx);
- StateSizeNonZero = checkNonNull(C, StateSizeNonZero, Dest, destVal);
+ SVal DestVal = State->getSVal(Dest.Expression, LCtx);
+ StateSizeNonZero = checkNonNull(C, StateSizeNonZero, Dest, DestVal);
if (!StateSizeNonZero)
return;
@@ -2307,27 +2307,27 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
// Success: return value < `n`
// Failure: return value >= `n`
- auto comparisonVal = SVB.evalBinOp(StateSizeNonZero, BO_LT, retVal, sizeVal,
+ auto ComparisonVal = SVB.evalBinOp(StateSizeNonZero, BO_LT, RetVal, SizeVal,
SVB.getConditionType())
.getAs<DefinedOrUnknownSVal>();
- if (comparisonVal) {
+ if (ComparisonVal) {
auto [StateSuccess, StateFailure] =
- StateSizeNonZero->assume(*comparisonVal);
+ StateSizeNonZero->assume(*ComparisonVal);
if (StateSuccess) {
// In this case, the transformation invalidated the buffer.
StateSuccess = invalidateDestinationBufferBySize(
- C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), destVal,
- sizeVal, Size.Expression->getType());
+ C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), DestVal,
+ SizeVal, Size.Expression->getType());
C.addTransition(StateSuccess);
}
if (StateFailure) {
// In this case, dest buffer content is undefined
- if (std::optional<Loc> destLoc = destVal.getAs<Loc>()) {
- StateFailure = StateFailure->bindLoc(*destLoc, UndefinedVal{}, LCtx);
+ if (std::optional<Loc> DestLoc = DestVal.getAs<Loc>()) {
+ StateFailure = StateFailure->bindLoc(*DestLoc, UndefinedVal{}, LCtx);
}
C.addTransition(StateFailure);
@@ -2335,8 +2335,8 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
} else {
// Fallback: invalidate the buffer.
StateSizeNonZero = invalidateDestinationBufferBySize(
- C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), destVal,
- sizeVal, Size.Expression->getType());
+ C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), DestVal,
+ SizeVal, Size.Expression->getType());
C.addTransition(StateSizeNonZero);
}
>From 5b7f66952e766e2a8cb0d61cc91847cc4a0b3000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Thu, 4 Sep 2025 13:34:09 +0200
Subject: [PATCH 4/8] Bind close to transition
---
.../Checkers/CStringChecker.cpp | 21 ++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 6b305e408354d..45be0ea90069a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2266,7 +2266,7 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
if (!State)
return;
- // Check overlaps
+ // Buffer must not overlap
State = CheckOverlap(C, State, Size, Dest, Source, CK_Regular);
if (!State)
return;
@@ -2275,8 +2275,6 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
// transformation
SVal RetVal = SVB.conjureSymbolVal(Call, C.blockCount());
- State = State->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
-
// Check if size is zero
SVal SizeVal = State->getSVal(Size.Expression, LCtx);
QualType SizeTy = Size.Expression->getType();
@@ -2284,17 +2282,22 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
auto [StateZeroSize, StateSizeNonZero] =
assumeZero(C, State, SizeVal, SizeTy);
+ // We can't assume anything about size, just bind the return value and be done
+ if (!StateZeroSize && !StateSizeNonZero) {
+ State = State->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
+ C.addTransition(State);
+ return;
+ }
+
// If `n` is 0, we just return the implementation defined length
if (StateZeroSize && !StateSizeNonZero) {
+ StateZeroSize = StateZeroSize->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
C.addTransition(StateZeroSize);
return;
}
- if (!StateSizeNonZero)
- return;
-
// If `n` is not 0, `dest` can not be null.
- SVal DestVal = State->getSVal(Dest.Expression, LCtx);
+ SVal DestVal = StateSizeNonZero->getSVal(Dest.Expression, LCtx);
StateSizeNonZero = checkNonNull(C, StateSizeNonZero, Dest, DestVal);
if (!StateSizeNonZero)
return;
@@ -2321,6 +2324,7 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), DestVal,
SizeVal, Size.Expression->getType());
+ StateSuccess = StateSuccess->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
C.addTransition(StateSuccess);
}
@@ -2330,6 +2334,7 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
StateFailure = StateFailure->bindLoc(*DestLoc, UndefinedVal{}, LCtx);
}
+ StateFailure = StateFailure->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
C.addTransition(StateFailure);
}
} else {
@@ -2338,6 +2343,8 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), DestVal,
SizeVal, Size.Expression->getType());
+ StateSizeNonZero =
+ StateSizeNonZero->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
C.addTransition(StateSizeNonZero);
}
}
>From d2614454358da173a44d8d4d1ee5ce17f0897605 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Thu, 4 Sep 2025 14:24:34 +0200
Subject: [PATCH 5/8] Set the full buffer as undefined
---
.../Checkers/CStringChecker.cpp | 10 +--
clang/test/Analysis/string.c | 63 ++++++++++++++++---
2 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 45be0ea90069a..3cbd10ead8cd3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2313,13 +2313,12 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
auto ComparisonVal = SVB.evalBinOp(StateSizeNonZero, BO_LT, RetVal, SizeVal,
SVB.getConditionType())
.getAs<DefinedOrUnknownSVal>();
-
if (ComparisonVal) {
auto [StateSuccess, StateFailure] =
StateSizeNonZero->assume(*ComparisonVal);
if (StateSuccess) {
- // In this case, the transformation invalidated the buffer.
+ // The transformation invalidated the buffer.
StateSuccess = invalidateDestinationBufferBySize(
C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), DestVal,
SizeVal, Size.Expression->getType());
@@ -2329,9 +2328,10 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
}
if (StateFailure) {
- // In this case, dest buffer content is undefined
- if (std::optional<Loc> DestLoc = DestVal.getAs<Loc>()) {
- StateFailure = StateFailure->bindLoc(*DestLoc, UndefinedVal{}, LCtx);
+ // `dest` buffer content is undefined
+ if (auto DestLoc = DestVal.getAs<loc::MemRegionVal>()) {
+ StateFailure = StateFailure->killBinding(*DestLoc);
+ StateFailure = StateFailure->bindDefaultInitial(*DestLoc, UndefinedVal{}, LCtx);
}
StateFailure = StateFailure->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c
index 427b99bfdf295..cdd36275568e3 100644
--- a/clang/test/Analysis/string.c
+++ b/clang/test/Analysis/string.c
@@ -1830,19 +1830,66 @@ void strxfrm_regular(const char *src) {
free(dest);
}
-int strxfrm_dest_undef(const char *src) {
- char dest[10] = {0};
+void clang_analyzer_warnIfReached();
+
+int strxfrm_dest_undef(const char *src, int oracle) {
+ char dest[5] = {0};
+ clang_analyzer_eval(dest[0] == 0); // expected-warning {{TRUE}}
+
size_t n = strxfrm(dest, src, sizeof(dest));
+ if (oracle >= sizeof(dest) || oracle < 0) {
+ return 0;
+ }
+
int c = 0;
if (n >= sizeof(dest)) {
- for (int i = 0; i < sizeof(dest); ++i) {
- c += dest[i]; // expected-warning {{Assigned value is uninitialized}}
- }
+ // Since accessing uninitialized sinks the execution, use this trick to check all positions
+ switch (oracle) {
+ case 0:
+ c = dest[0]; // expected-warning {{Assigned value is uninitialized}}
+ break;
+ case 1:
+ c = dest[1]; // expected-warning {{Assigned value is uninitialized}}
+ break;
+ case 2:
+ c = dest[2]; // expected-warning {{Assigned value is uninitialized}}
+ break;
+ case 3:
+ c = dest[3]; // expected-warning {{Assigned value is uninitialized}}
+ break;
+ case 4:
+ c = dest[4]; // expected-warning {{Assigned value is uninitialized}}
+ break;
+ default:
+ clang_analyzer_warnIfReached();
+ }
+ } else {
+ clang_analyzer_eval(n >= 0); // expected-warning {{TRUE}}
+ clang_analyzer_eval(dest[0] == 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(dest[1] == 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(dest[2] == 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(dest[3] == 0); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(dest[4] == 0); // expected-warning {{UNKNOWN}}
+ }
+ return c;
+}
+
+int strxfrm_unknown_dest_undef(const char *src, int oracle) {
+ size_t n = strlen(src);
+
+ char *dest = (char*)malloc(n * 2);
+
+ size_t n2 = strxfrm(dest, src, n);
+
+ int c = 0;
+
+ if (n2 < n) {
+ c += dest[50];
} else {
- for (int i = 0; i < sizeof(dest); ++i) {
- c += dest[i]; // no-warning
- }
+ c += dest[50]; // expected-warning {{Assigned value is uninitialized}}
}
+
+ free(dest);
return c;
}
>From aed3b69e73ebe8a1092a683d39bd19842256967b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Fri, 5 Sep 2025 14:03:07 +0200
Subject: [PATCH 6/8] Ground bind and return
---
.../Checkers/CStringChecker.cpp | 36 +++++++++----------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 3cbd10ead8cd3..f555ae9a13138 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2275,6 +2275,13 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
// transformation
SVal RetVal = SVB.conjureSymbolVal(Call, C.blockCount());
+ auto BindReturnAndTransition = [&RetVal, &Call, LCtx, &C](ProgramStateRef State) {
+ if (State) {
+ State = State->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
+ C.addTransition(State);
+ }
+ };
+
// Check if size is zero
SVal SizeVal = State->getSVal(Size.Expression, LCtx);
QualType SizeTy = Size.Expression->getType();
@@ -2283,18 +2290,12 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
assumeZero(C, State, SizeVal, SizeTy);
// We can't assume anything about size, just bind the return value and be done
- if (!StateZeroSize && !StateSizeNonZero) {
- State = State->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
- C.addTransition(State);
- return;
- }
+ if (!StateZeroSize && !StateSizeNonZero)
+ return BindReturnAndTransition(State);
// If `n` is 0, we just return the implementation defined length
- if (StateZeroSize && !StateSizeNonZero) {
- StateZeroSize = StateZeroSize->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
- C.addTransition(StateZeroSize);
- return;
- }
+ if (StateZeroSize && !StateSizeNonZero)
+ return BindReturnAndTransition(StateZeroSize);
// If `n` is not 0, `dest` can not be null.
SVal DestVal = StateSizeNonZero->getSVal(Dest.Expression, LCtx);
@@ -2322,30 +2323,25 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
StateSuccess = invalidateDestinationBufferBySize(
C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), DestVal,
SizeVal, Size.Expression->getType());
-
- StateSuccess = StateSuccess->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
- C.addTransition(StateSuccess);
+ BindReturnAndTransition(StateSuccess);
}
if (StateFailure) {
// `dest` buffer content is undefined
if (auto DestLoc = DestVal.getAs<loc::MemRegionVal>()) {
StateFailure = StateFailure->killBinding(*DestLoc);
- StateFailure = StateFailure->bindDefaultInitial(*DestLoc, UndefinedVal{}, LCtx);
+ StateFailure =
+ StateFailure->bindDefaultInitial(*DestLoc, UndefinedVal{}, LCtx);
}
- StateFailure = StateFailure->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
- C.addTransition(StateFailure);
+ BindReturnAndTransition(StateFailure);
}
} else {
// Fallback: invalidate the buffer.
StateSizeNonZero = invalidateDestinationBufferBySize(
C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), DestVal,
SizeVal, Size.Expression->getType());
-
- StateSizeNonZero =
- StateSizeNonZero->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
- C.addTransition(StateSizeNonZero);
+ BindReturnAndTransition(StateSizeNonZero);
}
}
>From 58bc8a593d06640852951dbe0e1b7a27b862ed8a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Fri, 5 Sep 2025 14:03:28 +0200
Subject: [PATCH 7/8] Format
---
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index f555ae9a13138..513e63d88ef7a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2275,7 +2275,8 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
// transformation
SVal RetVal = SVB.conjureSymbolVal(Call, C.blockCount());
- auto BindReturnAndTransition = [&RetVal, &Call, LCtx, &C](ProgramStateRef State) {
+ auto BindReturnAndTransition = [&RetVal, &Call, LCtx,
+ &C](ProgramStateRef State) {
if (State) {
State = State->BindExpr(Call.getOriginExpr(), LCtx, RetVal);
C.addTransition(State);
>From b58bef2a2259c1ce16142aa9ed8acd2f7767d21a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Fri, 5 Sep 2025 14:56:14 +0200
Subject: [PATCH 8/8] Be explicit about fallthrough
---
.../Checkers/CStringChecker.cpp | 48 +++++++++----------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 513e63d88ef7a..36f316df0c3ff 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2315,34 +2315,34 @@ void CStringChecker::evalStrxfrm(CheckerContext &C,
auto ComparisonVal = SVB.evalBinOp(StateSizeNonZero, BO_LT, RetVal, SizeVal,
SVB.getConditionType())
.getAs<DefinedOrUnknownSVal>();
- if (ComparisonVal) {
- auto [StateSuccess, StateFailure] =
- StateSizeNonZero->assume(*ComparisonVal);
-
- if (StateSuccess) {
- // The transformation invalidated the buffer.
- StateSuccess = invalidateDestinationBufferBySize(
- C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), DestVal,
- SizeVal, Size.Expression->getType());
- BindReturnAndTransition(StateSuccess);
- }
-
- if (StateFailure) {
- // `dest` buffer content is undefined
- if (auto DestLoc = DestVal.getAs<loc::MemRegionVal>()) {
- StateFailure = StateFailure->killBinding(*DestLoc);
- StateFailure =
- StateFailure->bindDefaultInitial(*DestLoc, UndefinedVal{}, LCtx);
- }
-
- BindReturnAndTransition(StateFailure);
- }
- } else {
+ if (!ComparisonVal) {
// Fallback: invalidate the buffer.
StateSizeNonZero = invalidateDestinationBufferBySize(
C, StateSizeNonZero, Dest.Expression, Call.getCFGElementRef(), DestVal,
SizeVal, Size.Expression->getType());
- BindReturnAndTransition(StateSizeNonZero);
+ return BindReturnAndTransition(StateSizeNonZero);
+ }
+
+ auto [StateSuccess, StateFailure] = StateSizeNonZero->assume(*ComparisonVal);
+
+ if (StateSuccess) {
+ // The transformation invalidated the buffer.
+ StateSuccess = invalidateDestinationBufferBySize(
+ C, StateSuccess, Dest.Expression, Call.getCFGElementRef(), DestVal,
+ SizeVal, Size.Expression->getType());
+ BindReturnAndTransition(StateSuccess);
+ // Fallthrough: We also want to add a transition to the failure state below.
+ }
+
+ if (StateFailure) {
+ // `dest` buffer content is undefined
+ if (auto DestLoc = DestVal.getAs<loc::MemRegionVal>()) {
+ StateFailure = StateFailure->killBinding(*DestLoc);
+ StateFailure =
+ StateFailure->bindDefaultInitial(*DestLoc, UndefinedVal{}, LCtx);
+ }
+
+ BindReturnAndTransition(StateFailure);
}
}
More information about the cfe-commits
mailing list