balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Support for functions wmemcpy, wcslen, wcsnlen is added to the checker.
Documentation and tests are updated and extended with the new functions.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130091
Files:
clang/docs/analyzer/checkers.rst
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/test/Analysis/cstring.c
Index: clang/test/Analysis/cstring.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/cstring.c
@@ -0,0 +1,126 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring \
+// RUN: -analyzer-checker=alpha.unix.cstring \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -w -verify %s
+
+typedef __SIZE_TYPE__ size_t;
+typedef __WCHAR_TYPE__ wchar_t;
+void *memcpy(void *dest, const void *src,
+ size_t count);
+wchar_t *wmemcpy(wchar_t *restrict dest, const wchar_t *restrict src,
+ size_t count);
+size_t strlen(const char *s);
+size_t wcslen(const wchar_t *s);
+size_t strnlen(const char *s, size_t maxlen);
+size_t wcsnlen(const wchar_t *s, size_t maxlen);
+
+static const char str[] = "Hello world";
+static const wchar_t w_str[] = L"Hello world";
+
+extern void clang_analyzer_eval(int);
+
+void test_sizeof(void) {
+ char buffer[32];
+ memcpy(buffer, str, sizeof(str));
+ memcpy(buffer, str, sizeof(str) + 1); // expected-warning {{Memory copy function accesses out-of-bound array element [alpha.unix.cstring.OutOfBounds]}}
+}
+
+void w_test_sizeof(void) {
+ wchar_t w_buffer[32];
+ wmemcpy(w_buffer, w_str, sizeof(w_str) / sizeof(w_str[0]));
+ wmemcpy(w_buffer, w_str, (sizeof(w_str) / sizeof(w_str[0])) + 1); // expected-warning {{Memory copy function accesses out-of-bound array element}}
+}
+
+void test_strlen(void) {
+ char buffer[32];
+ // FIXME: This should work with 'str' instead of 'str1'
+ const char str1[] = "Hello world";
+ memcpy(buffer, str1, strlen(str1) + 1);
+ memcpy(buffer, str1, strlen(str1) + 2); // expected-warning {{Memory copy function accesses out-of-bound array element}}
+}
+
+void w_test_wcslen(void) {
+ wchar_t w_buffer[32];
+ // FIXME: This should work with 'w_str' instead of 'w_str1'
+ const wchar_t w_str1[] = L"Hello world";
+ wmemcpy(w_buffer, w_str1, wcslen(w_str1) + 1);
+ wmemcpy(w_buffer, w_str1, wcslen(w_str1) + 2); // expected-warning {{Memory copy function accesses out-of-bound array element}}
+}
+
+void test_strnlen(void) {
+ const char str1[] = "0123456789";
+ const char str2[] = "";
+ clang_analyzer_eval(strnlen(str1, 1) == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(strnlen(str1, 100) == 10); // expected-warning {{TRUE}}
+ clang_analyzer_eval(strnlen(str2, 10) == 0); // expected-warning {{TRUE}}
+}
+
+void w_test_wcsnlen(void) {
+ const wchar_t str1[] = L"0123456789";
+ const wchar_t str2[] = L"";
+ clang_analyzer_eval(wcsnlen(str1, 1) == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(wcsnlen(str1, 100) == 10); // expected-warning {{TRUE}}
+ clang_analyzer_eval(wcsnlen(str2, 10) == 0); // expected-warning {{TRUE}}
+}
+
+void test_strlen_with_zero(void) {
+ char buffer[32];
+ const char str1[] = "Hello\0world";
+ clang_analyzer_eval(strlen(str1) == 11); // expected-warning {{TRUE}}
+}
+
+void test_dst_overflow(void) {
+ char buffer[] = "Helloworld";
+ memcpy(buffer, str, sizeof(str)); // expected-warning {{Memory copy function overflows the destination buffer [alpha.unix.cstring.OutOfBounds]}}
+}
+
+void w_test_dst_overflow(void) {
+ wchar_t buffer[] = L"Helloworld";
+ wmemcpy(buffer, w_str, sizeof(w_str) / sizeof(w_str[0])); // expected-warning {{Memory copy function overflows the destination buffer}}
+}
+
+void test_dst_offset(void) {
+ char buffer[] = "Hello world";
+ memcpy(buffer + 1, str, sizeof(str)); // expected-warning {{Memory copy function overflows the destination buffer}}
+}
+
+void w_test_dst_offset(void) {
+ wchar_t buffer[] = L"Hello world";
+ wmemcpy(buffer + 1, w_str, sizeof(w_str) / sizeof(w_str[0])); // expected-warning {{Memory copy function overflows the destination buffer}}
+}
+
+int test_strlen_null() {
+ return strlen(0); // expected-warning {{Null pointer passed as 1st argument to string length function [unix.cstring.NullArg]}}
+}
+
+int test_wcslen_null() {
+ return wcslen(0); // expected-warning {{Null pointer passed as 1st argument to string length function}}
+}
+
+int test_strnlen_null() {
+ return strnlen(0, 10); // expected-warning {{Null pointer passed as 1st argument to string length function}}
+}
+
+int test_wcsnlen_null() {
+ return wcsnlen(0, 10); // expected-warning {{Null pointer passed as 1st argument to string length function}}
+}
+
+void test_overlap() {
+ int a[4] = {0};
+ memcpy(a + 2, a + 1, sizeof(int));
+ memcpy(a + 2, a + 1, sizeof(int) * 2); // expected-warning {{Arguments must not be overlapping buffers [alpha.unix.cstring.BufferOverlap]}}
+}
+
+void w_test_overlap() {
+ wchar_t a[4] = {0};
+ memcpy(a + 2, a + 1, sizeof(wchar_t));
+ memcpy(a + 2, a + 1, sizeof(wchar_t) * 2); // expected-warning {{Arguments must not be overlapping buffers}}
+}
+
+void test_not_null_terminated() {
+ int y = strlen((char *)&test_not_null_terminated); // expected-warning {{Argument to string length function is the address of the function 'test_not_null_terminated', which is not a null-terminated string [alpha.unix.cstring.NotNullTerminated]}}
+}
+
+void w_test_not_null_terminated() {
+ int y = wcslen((wchar_t *)&w_test_not_null_terminated); // expected-warning {{Argument to string length function is the address of the function 'w_test_not_null_terminated', which is not a null-terminated string}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -26,9 +26,11 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/raw_ostream.h"
+#include <functional>
using namespace clang;
using namespace ento;
+using namespace std::placeholders;
namespace {
struct AnyArgExpr {
@@ -118,10 +120,14 @@
const LocationContext *LCtx,
const CallEvent *Call) const;
- typedef void (CStringChecker::*FnCheck)(CheckerContext &,
- const CallExpr *) const;
+ using FnCheck = std::function<void(const CStringChecker *, CheckerContext &,
+ const CallExpr *)>;
+
CallDescriptionMap<FnCheck> Callbacks = {
- {{CDF_MaybeBuiltin, "memcpy", 3}, &CStringChecker::evalMemcpy},
+ {{CDF_MaybeBuiltin, "memcpy", 3},
+ std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, false)},
+ {{CDF_MaybeBuiltin, "wmemcpy", 3},
+ std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, true)},
{{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy},
{{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp},
{{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove},
@@ -135,7 +141,9 @@
{{CDF_MaybeBuiltin, "strncat", 3}, &CStringChecker::evalStrncat},
{{CDF_MaybeBuiltin, "strlcat", 3}, &CStringChecker::evalStrlcat},
{{CDF_MaybeBuiltin, "strlen", 1}, &CStringChecker::evalstrLength},
+ {{CDF_MaybeBuiltin, "wcslen", 1}, &CStringChecker::evalstrLength},
{{CDF_MaybeBuiltin, "strnlen", 2}, &CStringChecker::evalstrnLength},
+ {{CDF_MaybeBuiltin, "wcsnlen", 2}, &CStringChecker::evalstrnLength},
{{CDF_MaybeBuiltin, "strcmp", 2}, &CStringChecker::evalStrcmp},
{{CDF_MaybeBuiltin, "strncmp", 3}, &CStringChecker::evalStrncmp},
{{CDF_MaybeBuiltin, "strcasecmp", 2}, &CStringChecker::evalStrcasecmp},
@@ -152,14 +160,14 @@
StdCopyBackward{{"std", "copy_backward"}, 3};
FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
- void evalMemcpy(CheckerContext &C, const CallExpr *CE) const;
+ void evalMemcpy(CheckerContext &C, const CallExpr *CE, bool IsWide) const;
void evalMempcpy(CheckerContext &C, const CallExpr *CE) const;
void evalMemmove(CheckerContext &C, const CallExpr *CE) const;
void evalBcopy(CheckerContext &C, const CallExpr *CE) const;
void evalCopyCommon(CheckerContext &C, const CallExpr *CE,
ProgramStateRef state, SizeArgExpr Size,
DestinationArgExpr Dest, SourceArgExpr Source,
- bool Restricted, bool IsMempcpy) const;
+ bool Restricted, bool IsMempcpy, bool IsWide) const;
void evalMemcmp(CheckerContext &C, const CallExpr *CE) const;
@@ -240,10 +248,11 @@
AnyArgExpr Arg, SVal l) const;
ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state,
AnyArgExpr Buffer, SVal Element,
- AccessKind Access) const;
+ AccessKind Access, bool IsWide = false) const;
ProgramStateRef CheckBufferAccess(CheckerContext &C, ProgramStateRef State,
AnyArgExpr Buffer, SizeArgExpr Size,
- AccessKind Access) const;
+ AccessKind Access,
+ bool IsWide = false) const;
ProgramStateRef CheckOverlap(CheckerContext &C, ProgramStateRef state,
SizeArgExpr Size, AnyArgExpr First,
AnyArgExpr Second) const;
@@ -329,7 +338,8 @@
ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
ProgramStateRef state,
AnyArgExpr Buffer, SVal Element,
- AccessKind Access) const {
+ AccessKind Access,
+ bool IsWide) const {
// If a previous check has failed, propagate the failure.
if (!state)
@@ -344,17 +354,36 @@
if (!ER)
return state;
- if (ER->getValueType() != C.getASTContext().CharTy)
- return state;
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ ASTContext &Ctx = svalBuilder.getContext();
+
+ // Get the index of the accessed element.
+ NonLoc Idx = ER->getIndex();
+
+ if (!IsWide) {
+ if (ER->getValueType() != Ctx.CharTy)
+ return state;
+ } else {
+ if (ER->getValueType() != Ctx.WideCharTy)
+ return state;
+
+ QualType SizeTy = Ctx.getSizeType();
+ NonLoc WideSize =
+ svalBuilder
+ .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
+ SizeTy)
+ .castAs<NonLoc>();
+ SVal Offset = svalBuilder.evalBinOpNN(state, BO_Mul, Idx, WideSize, SizeTy);
+ if (Offset.isUnknown())
+ return state;
+ Idx = Offset.castAs<NonLoc>();
+ }
// Get the size of the array.
const auto *superReg = cast<SubRegion>(ER->getSuperRegion());
DefinedOrUnknownSVal Size =
getDynamicExtent(state, superReg, C.getSValBuilder());
- // Get the index of the accessed element.
- DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
-
ProgramStateRef StInBound, StOutBound;
std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, Size);
if (StOutBound && !StInBound) {
@@ -385,11 +414,10 @@
return StInBound;
}
-ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,
- ProgramStateRef State,
- AnyArgExpr Buffer,
- SizeArgExpr Size,
- AccessKind Access) const {
+ProgramStateRef
+CStringChecker::CheckBufferAccess(CheckerContext &C, ProgramStateRef State,
+ AnyArgExpr Buffer, SizeArgExpr Size,
+ AccessKind Access, bool IsWide) const {
// If a previous check has failed, propagate the failure.
if (!State)
return nullptr;
@@ -398,7 +426,7 @@
ASTContext &Ctx = svalBuilder.getContext();
QualType SizeTy = Size.Expression->getType();
- QualType PtrTy = Ctx.getPointerType(Ctx.CharTy);
+ QualType PtrTy = Ctx.getPointerType(IsWide ? Ctx.WideCharTy : Ctx.CharTy);
// Check that the first buffer is non-null.
SVal BufVal = C.getSVal(Buffer.Expression);
@@ -432,7 +460,7 @@
SVal BufEnd =
svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
- State = CheckLocation(C, State, Buffer, BufEnd, Access);
+ State = CheckLocation(C, State, Buffer, BufEnd, Access, IsWide);
// If the buffer isn't large enough, abort.
if (!State)
@@ -1161,7 +1189,7 @@
ProgramStateRef state, SizeArgExpr Size,
DestinationArgExpr Dest,
SourceArgExpr Source, bool Restricted,
- bool IsMempcpy) const {
+ bool IsMempcpy, bool IsWide) const {
CurrentFunctionDescription = "memory copy function";
// See if the size argument is zero.
@@ -1204,8 +1232,8 @@
return;
// Ensure the accesses are valid and that the buffers do not overlap.
- state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write);
- state = CheckBufferAccess(C, state, Source, Size, AccessKind::read);
+ state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write, IsWide);
+ state = CheckBufferAccess(C, state, Source, Size, AccessKind::read, IsWide);
if (Restricted)
state = CheckOverlap(C, state, Size, Dest, Source);
@@ -1258,7 +1286,8 @@
}
}
-void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const {
+void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE,
+ bool IsWide) 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};
@@ -1269,7 +1298,8 @@
constexpr bool IsRestricted = true;
constexpr bool IsMempcpy = false;
- evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy);
+ evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy,
+ IsWide);
}
void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const {
@@ -1281,7 +1311,8 @@
constexpr bool IsRestricted = true;
constexpr bool IsMempcpy = true;
- evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy);
+ evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
+ false);
}
void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const {
@@ -1293,7 +1324,8 @@
constexpr bool IsRestricted = false;
constexpr bool IsMempcpy = false;
- evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy);
+ evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
+ false);
}
void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {
@@ -1304,7 +1336,8 @@
constexpr bool IsRestricted = false;
constexpr bool IsMempcpy = false;
- evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy);
+ evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
+ false);
}
void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const {
@@ -2336,7 +2369,7 @@
// Check and evaluate the call.
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
- (this->*Callback)(C, CE);
+ Callback(this, C, CE);
// If the evaluate call resulted in no change, chain to the next eval call
// handler.
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -952,7 +952,7 @@
unix.cstring.NullArg (C)
"""""""""""""""""""""""""
Check for null pointers being passed as arguments to C string functions:
-``strlen, strnlen, strcpy, strncpy, strcat, strncat, strcmp, strncmp, strcasecmp, strncasecmp``.
+``strlen, strnlen, strcpy, strncpy, strcat, strncat, strcmp, strncmp, strcasecmp, strncasecmp, wcslen, wcsnlen``.
.. code-block:: c
@@ -2699,7 +2699,7 @@
alpha.unix.cstring.BufferOverlap (C)
""""""""""""""""""""""""""""""""""""
-Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy``.
+Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy, wmemcpy``.
.. code-block:: c
@@ -2712,7 +2712,7 @@
alpha.unix.cstring.NotNullTerminated (C)
""""""""""""""""""""""""""""""""""""""""
-Check for arguments which are not null-terminated strings; applies to: ``strlen, strnlen, strcpy, strncpy, strcat, strncat``.
+Check for arguments which are not null-terminated strings; applies to: ``strlen, strnlen, strcpy, strncpy, strcat, strncat, wcslen, wcsnlen``.
.. code-block:: c
@@ -2724,15 +2724,24 @@
alpha.unix.cstring.OutOfBounds (C)
""""""""""""""""""""""""""""""""""
-Check for out-of-bounds access in string functions; applies to:`` strncopy, strncat``.
+Check for out-of-bounds access in string functions, such as:
+``memcpy, bcopy, strcpy, strncpy, strcat, strncat, memmove, memcmp, memset`` and more.
-This check also applies to string literals, except there is a known bug in that
-the analyzer cannot detect embedded NULL characters.
+This check also works with string literals, except there is a known bug in that
+the analyzer cannot detect embedded NULL characters when determining the string length.
.. code-block:: c
- void test() {
- int y = strlen((char *)&test); // warn
+ void test1() {
+ const char str[] = "Hello world";
+ char buffer[] = "Hello world";
+ memcpy(buffer, str, sizeof(str) + 1); // warn
+ }
+
+ void test2() {
+ const char str[] = "Hello world";
+ char buffer[] = "Helloworld";
+ memcpy(buffer, str, sizeof(str)); // warn
}
.. _alpha-unix-cstring-UninitializedRead:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
