[clang] [analyzer] Fix "sprintf" parameter modeling in CStringChecker (PR #74345)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 4 09:27:56 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balazs Benics (steakhal)
<details>
<summary>Changes</summary>
Review the commits one by one. I plan to merge them manually by pushing both of these at once.
This PR intends to fix #<!-- -->74269.
---
Patch is 40.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74345.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (+203-179)
- (modified) clang/test/Analysis/string.cpp (+21-3)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 31f5b03dcdeba..b7b64c3da4f6c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -121,7 +121,7 @@ class CStringChecker : public Checker< eval::Call,
const CallEvent *Call) const;
using FnCheck = std::function<void(const CStringChecker *, CheckerContext &,
- const CallExpr *)>;
+ const CallEvent &)>;
CallDescriptionMap<FnCheck> Callbacks = {
{{CDF_MaybeBuiltin, {"memcpy"}, 3},
@@ -173,56 +173,53 @@ class CStringChecker : public Checker< eval::Call,
StdCopyBackward{{"std", "copy_backward"}, 3};
FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
- void evalMemcpy(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
- void evalMempcpy(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
- void evalMemmove(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
- void evalBcopy(CheckerContext &C, const CallExpr *CE) const;
- void evalCopyCommon(CheckerContext &C, const CallExpr *CE,
+ void evalMemcpy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+ void evalMempcpy(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+ void evalMemmove(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
+ void evalBcopy(CheckerContext &C, const CallEvent &Call) const;
+ void evalCopyCommon(CheckerContext &C, const CallEvent &Call,
ProgramStateRef state, SizeArgExpr Size,
DestinationArgExpr Dest, SourceArgExpr Source,
bool Restricted, bool IsMempcpy, CharKind CK) const;
- void evalMemcmp(CheckerContext &C, const CallExpr *CE, CharKind CK) const;
+ void evalMemcmp(CheckerContext &C, const CallEvent &Call, CharKind CK) const;
- void evalstrLength(CheckerContext &C, const CallExpr *CE) const;
- void evalstrnLength(CheckerContext &C, const CallExpr *CE) const;
- void evalstrLengthCommon(CheckerContext &C,
- const CallExpr *CE,
+ void evalstrLength(CheckerContext &C, const CallEvent &Call) const;
+ void evalstrnLength(CheckerContext &C, const CallEvent &Call) const;
+ void evalstrLengthCommon(CheckerContext &C, const CallEvent &Call,
bool IsStrnlen = false) const;
- void evalStrcpy(CheckerContext &C, const CallExpr *CE) const;
- void evalStrncpy(CheckerContext &C, const CallExpr *CE) const;
- void evalStpcpy(CheckerContext &C, const CallExpr *CE) const;
- void evalStrlcpy(CheckerContext &C, const CallExpr *CE) const;
- void evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, bool ReturnEnd,
- bool IsBounded, ConcatFnKind appendK,
+ void evalStrcpy(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrncpy(CheckerContext &C, const CallEvent &Call) const;
+ void evalStpcpy(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrlcpy(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
+ bool ReturnEnd, bool IsBounded, ConcatFnKind appendK,
bool returnPtr = true) const;
- void evalStrcat(CheckerContext &C, const CallExpr *CE) const;
- void evalStrncat(CheckerContext &C, const CallExpr *CE) const;
- void evalStrlcat(CheckerContext &C, const CallExpr *CE) 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;
- void evalStrcmp(CheckerContext &C, const CallExpr *CE) const;
- void evalStrncmp(CheckerContext &C, const CallExpr *CE) const;
- void evalStrcasecmp(CheckerContext &C, const CallExpr *CE) const;
- void evalStrncasecmp(CheckerContext &C, const CallExpr *CE) const;
- void evalStrcmpCommon(CheckerContext &C,
- const CallExpr *CE,
- bool IsBounded = false,
- bool IgnoreCase = false) const;
+ void evalStrcmp(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrncmp(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrcasecmp(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrncasecmp(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrcmpCommon(CheckerContext &C, const CallEvent &Call,
+ bool IsBounded = false, bool IgnoreCase = false) const;
- void evalStrsep(CheckerContext &C, const CallExpr *CE) const;
+ void evalStrsep(CheckerContext &C, const CallEvent &Call) const;
- void evalStdCopy(CheckerContext &C, const CallExpr *CE) const;
- void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const;
- void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const;
- void evalMemset(CheckerContext &C, const CallExpr *CE) const;
- void evalBzero(CheckerContext &C, const CallExpr *CE) const;
+ void evalStdCopy(CheckerContext &C, const CallEvent &Call) const;
+ void evalStdCopyBackward(CheckerContext &C, const CallEvent &Call) const;
+ void evalStdCopyCommon(CheckerContext &C, const CallEvent &Call) const;
+ void evalMemset(CheckerContext &C, const CallEvent &Call) const;
+ void evalBzero(CheckerContext &C, const CallEvent &Call) const;
- void evalSprintf(CheckerContext &C, const CallExpr *CE) const;
- void evalSnprintf(CheckerContext &C, const CallExpr *CE) const;
- void evalSprintfCommon(CheckerContext &C, const CallExpr *CE, bool IsBounded,
- bool IsBuiltin) const;
+ void evalSprintf(CheckerContext &C, const CallEvent &Call) const;
+ void evalSnprintf(CheckerContext &C, const CallEvent &Call) const;
+ void evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
+ bool IsBounded, bool IsBuiltin) const;
// Utility methods
std::pair<ProgramStateRef , ProgramStateRef >
@@ -1291,7 +1288,7 @@ bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal,
// evaluation of individual function calls.
//===----------------------------------------------------------------------===//
-void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalCopyCommon(CheckerContext &C, const CallEvent &Call,
ProgramStateRef state, SizeArgExpr Size,
DestinationArgExpr Dest,
SourceArgExpr Source, bool Restricted,
@@ -1313,7 +1310,8 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
// If the size is zero, there won't be any actual memory access, so
// just bind the return value to the destination buffer and return.
if (stateZeroSize && !stateNonZeroSize) {
- stateZeroSize = stateZeroSize->BindExpr(CE, LCtx, destVal);
+ stateZeroSize =
+ stateZeroSize->BindExpr(Call.getOriginExpr(), LCtx, destVal);
C.addTransition(stateZeroSize);
return;
}
@@ -1361,15 +1359,15 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
// If we don't know how much we copied, we can at least
// conjure a return value for later.
if (lastElement.isUnknown())
- lastElement = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
- C.blockCount());
+ lastElement = C.getSValBuilder().conjureSymbolVal(
+ nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
// The byte after the last byte copied is the return value.
- state = state->BindExpr(CE, LCtx, lastElement);
+ state = state->BindExpr(Call.getOriginExpr(), LCtx, lastElement);
} else {
// All other copies return the destination buffer.
// (Well, bcopy() has a void return type, but this won't hurt.)
- state = state->BindExpr(CE, LCtx, destVal);
+ state = state->BindExpr(Call.getOriginExpr(), LCtx, destVal);
}
// Invalidate the destination (regular invalidation without pointer-escaping
@@ -1391,69 +1389,69 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
}
}
-void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMemcpy(CheckerContext &C, const CallEvent &Call,
CharKind CK) const {
// void *memcpy(void *restrict dst, const void *restrict src, size_t n);
// The return value is the address of the destination buffer.
- DestinationArgExpr Dest = {{CE->getArg(0), 0}};
- SourceArgExpr Src = {{CE->getArg(1), 1}};
- SizeArgExpr Size = {{CE->getArg(2), 2}};
+ DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+ SourceArgExpr Src = {{Call.getArgExpr(1), 1}};
+ SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
ProgramStateRef State = C.getState();
constexpr bool IsRestricted = true;
constexpr bool IsMempcpy = false;
- evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy, CK);
+ evalCopyCommon(C, Call, State, Size, Dest, Src, IsRestricted, IsMempcpy, CK);
}
-void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMempcpy(CheckerContext &C, const CallEvent &Call,
CharKind CK) const {
// void *mempcpy(void *restrict dst, const void *restrict src, size_t n);
// The return value is a pointer to the byte following the last written byte.
- DestinationArgExpr Dest = {{CE->getArg(0), 0}};
- SourceArgExpr Src = {{CE->getArg(1), 1}};
- SizeArgExpr Size = {{CE->getArg(2), 2}};
+ DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+ SourceArgExpr Src = {{Call.getArgExpr(1), 1}};
+ SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
constexpr bool IsRestricted = true;
constexpr bool IsMempcpy = true;
- evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
- CK);
+ evalCopyCommon(C, Call, C.getState(), Size, Dest, Src, IsRestricted,
+ IsMempcpy, CK);
}
-void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMemmove(CheckerContext &C, const CallEvent &Call,
CharKind CK) const {
// void *memmove(void *dst, const void *src, size_t n);
// The return value is the address of the destination buffer.
- DestinationArgExpr Dest = {{CE->getArg(0), 0}};
- SourceArgExpr Src = {{CE->getArg(1), 1}};
- SizeArgExpr Size = {{CE->getArg(2), 2}};
+ DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
+ SourceArgExpr Src = {{Call.getArgExpr(1), 1}};
+ SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
constexpr bool IsRestricted = false;
constexpr bool IsMempcpy = false;
- evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
- CK);
+ evalCopyCommon(C, Call, C.getState(), Size, Dest, Src, IsRestricted,
+ IsMempcpy, CK);
}
-void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalBcopy(CheckerContext &C, const CallEvent &Call) const {
// void bcopy(const void *src, void *dst, size_t n);
- SourceArgExpr Src{{CE->getArg(0), 0}};
- DestinationArgExpr Dest = {{CE->getArg(1), 1}};
- SizeArgExpr Size = {{CE->getArg(2), 2}};
+ SourceArgExpr Src{{Call.getArgExpr(0), 0}};
+ DestinationArgExpr Dest = {{Call.getArgExpr(1), 1}};
+ SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
constexpr bool IsRestricted = false;
constexpr bool IsMempcpy = false;
- evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
- CharKind::Regular);
+ evalCopyCommon(C, Call, C.getState(), Size, Dest, Src, IsRestricted,
+ IsMempcpy, CharKind::Regular);
}
-void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalMemcmp(CheckerContext &C, const CallEvent &Call,
CharKind CK) const {
// int memcmp(const void *s1, const void *s2, size_t n);
CurrentFunctionDescription = "memory comparison function";
- AnyArgExpr Left = {CE->getArg(0), 0};
- AnyArgExpr Right = {CE->getArg(1), 1};
- SizeArgExpr Size = {{CE->getArg(2), 2}};
+ AnyArgExpr Left = {Call.getArgExpr(0), 0};
+ AnyArgExpr Right = {Call.getArgExpr(1), 1};
+ SizeArgExpr Size = {{Call.getArgExpr(2), 2}};
ProgramStateRef State = C.getState();
SValBuilder &Builder = C.getSValBuilder();
@@ -1471,7 +1469,8 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
// have to check either of the buffers.
if (stateZeroSize) {
State = stateZeroSize;
- State = State->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType()));
+ State = State->BindExpr(Call.getOriginExpr(), LCtx,
+ Builder.makeZeroVal(Call.getResultType()));
C.addTransition(State);
}
@@ -1497,8 +1496,8 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
State = SameBuffer;
State = CheckBufferAccess(C, State, Left, Size, AccessKind::read);
if (State) {
- State =
- SameBuffer->BindExpr(CE, LCtx, Builder.makeZeroVal(CE->getType()));
+ State = SameBuffer->BindExpr(Call.getOriginExpr(), LCtx,
+ Builder.makeZeroVal(Call.getResultType()));
C.addTransition(State);
}
return;
@@ -1511,33 +1510,35 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE,
State = CheckBufferAccess(C, State, Left, Size, AccessKind::read, CK);
if (State) {
// The return value is the comparison result, which we don't know.
- SVal CmpV = Builder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
- State = State->BindExpr(CE, LCtx, CmpV);
+ SVal CmpV = Builder.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx,
+ C.blockCount());
+ State = State->BindExpr(Call.getOriginExpr(), LCtx, CmpV);
C.addTransition(State);
}
}
}
void CStringChecker::evalstrLength(CheckerContext &C,
- const CallExpr *CE) const {
+ const CallEvent &Call) const {
// size_t strlen(const char *s);
- evalstrLengthCommon(C, CE, /* IsStrnlen = */ false);
+ evalstrLengthCommon(C, Call, /* IsStrnlen = */ false);
}
void CStringChecker::evalstrnLength(CheckerContext &C,
- const CallExpr *CE) const {
+ const CallEvent &Call) const {
// size_t strnlen(const char *s, size_t maxlen);
- evalstrLengthCommon(C, CE, /* IsStrnlen = */ true);
+ evalstrLengthCommon(C, Call, /* IsStrnlen = */ true);
}
-void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
+void CStringChecker::evalstrLengthCommon(CheckerContext &C,
+ const CallEvent &Call,
bool IsStrnlen) const {
CurrentFunctionDescription = "string length function";
ProgramStateRef state = C.getState();
const LocationContext *LCtx = C.getLocationContext();
if (IsStrnlen) {
- const Expr *maxlenExpr = CE->getArg(1);
+ const Expr *maxlenExpr = Call.getArgExpr(1);
SVal maxlenVal = state->getSVal(maxlenExpr, LCtx);
ProgramStateRef stateZeroSize, stateNonZeroSize;
@@ -1547,8 +1548,8 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
// If the size can be zero, the result will be 0 in that case, and we don't
// have to check the string itself.
if (stateZeroSize) {
- SVal zero = C.getSValBuilder().makeZeroVal(CE->getType());
- stateZeroSize = stateZeroSize->BindExpr(CE, LCtx, zero);
+ SVal zero = C.getSValBuilder().makeZeroVal(Call.getResultType());
+ stateZeroSize = stateZeroSize->BindExpr(Call.getOriginExpr(), LCtx, zero);
C.addTransition(stateZeroSize);
}
@@ -1561,7 +1562,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
}
// Check that the string argument is non-null.
- AnyArgExpr Arg = {CE->getArg(0), 0};
+ AnyArgExpr Arg = {Call.getArgExpr(0), 0};
SVal ArgVal = state->getSVal(Arg.Expression, LCtx);
state = checkNonNull(C, state, Arg, ArgVal);
@@ -1584,7 +1585,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
// It's a little unfortunate to be getting this again,
// but it's not that expensive...
- const Expr *maxlenExpr = CE->getArg(1);
+ const Expr *maxlenExpr = Call.getArgExpr(1);
SVal maxlenVal = state->getSVal(maxlenExpr, LCtx);
std::optional<NonLoc> strLengthNL = strLength.getAs<NonLoc>();
@@ -1613,8 +1614,8 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
// no guarantee the full string length will actually be returned.
// All we know is the return value is the min of the string length
// and the limit. This is better than nothing.
- result = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
- C.blockCount());
+ result = C.getSValBuilder().conjureSymbolVal(
+ nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
NonLoc resultNL = result.castAs<NonLoc>();
if (strLengthNL) {
@@ -1637,78 +1638,85 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
// If we don't know the length of the string, conjure a return
// value, so it can be used in constraints, at least.
if (result.isUnknown()) {
- result = C.getSValBuilder().conjureSymbolVal(nullptr, CE, LCtx,
- C.blockCount());
+ result = C.getSValBuilder().conjureSymbolVal(
+ nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
}
}
// Bind the return value.
assert(!result.isUnknown() && "Should have conjured a value by now");
- state = state->BindExpr(CE, LCtx, result);
+ state = state->BindExpr(Call.getOriginExpr(), LCtx, result);
C.addTransition(state);
}
-void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStrcpy(CheckerContext &C,
+ const CallEvent &Call) const {
// char *strcpy(char *restrict dst, const char *restrict src);
- evalStrcpyCommon(C, CE,
+ evalStrcpyCommon(C, Call,
/* ReturnEnd = */ false,
/* IsBounded = */ false,
/* appendK = */ ConcatFnKind::none);
}
-void CStringChecker::evalStrncpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStrncpy(CheckerContext &C,
+ const CallEvent &Call) const {
// char *strncpy(char *restrict dst, const char *restrict src, size_t n);
- evalStrcpyCommon(C, CE,
+ evalStrcpyCommon(C, Call,
/* ReturnEnd = */ false,
/* IsBounded = */ true,
/* appendK = */ ConcatFnKind::none);
}
-void CStringChecker::evalStpcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStpcpy(CheckerContext &C,
+ const CallEvent &Call) const {
// char *stpcpy(char *restrict dst, const char *restrict src);
- evalStrcpyCommon(C, CE,
+ evalStrcpyCommon(C, Call,
/* ReturnEnd = */ true,
/* IsBounded = */ false,
/* appendK = */ ConcatFnKind::none);
}
-void CStringChecker::evalStrlcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStrlcpy(CheckerContext &C,
+ const CallEvent &Call) const {
// size_t strlcpy(char *dest, const char *src, size_t size);
- evalStrcpyCommon(C, CE,
+ evalStrcpyCommon(C, Call,
/* ReturnEnd = */ true,
/* IsBounded = */ true,
/* appendK = */ ConcatFnKind::none,
/* returnPtr = */ false);
}
-void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalStr...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/74345
More information about the cfe-commits
mailing list