Skip to content

Commit

Permalink
[clang][analyzer] Added partial wide character support to CStringChecker
Browse files Browse the repository at this point in the history 
Support for functions wmemcpy, wcslen, wcsnlen is added to the checker.
Documentation and tests are updated and extended with the new functions.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D130091
  • Loading branch information
@balazske
balazske committed Jul 25, 2022
1 parent 836f790 commit 94ca2be
Show file tree
Hide file tree
Showing 3 changed files with 473 additions and 41 deletions.
25 changes: 17 additions & 8 deletions clang/docs/analyzer/checkers.rst
View file
Expand Up @@ -952,7 +952,7 @@ Check the size argument passed into C string functions for common erroneous patt
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
Expand Down Expand Up @@ -2699,7 +2699,7 @@ Check stream handling functions: ``fopen, tmpfile, fclose, fread, fwrite, fseek,
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
Expand All @@ -2712,7 +2712,7 @@ Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy``.
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
Expand All @@ -2724,15 +2724,24 @@ Check for arguments which are not null-terminated strings; applies to: ``strlen,
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:
Expand Down
100 changes: 67 additions & 33 deletions clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
View file
Expand Up @@ -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 {
Expand Down Expand Up @@ -118,10 +120,14 @@ class CStringChecker : public Checker< eval::Call,
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},
Expand All @@ -135,7 +141,9 @@ class CStringChecker : public Checker< eval::Call,
{{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},
Expand All @@ -152,14 +160,14 @@ 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) 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;

Expand Down Expand Up @@ -240,13 +248,14 @@ class CStringChecker : public Checker< eval::Call,
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;
AnyArgExpr Second, bool IsWide = false) const;
void emitOverlapBug(CheckerContext &C,
ProgramStateRef state,
const Stmt *First,
Expand Down Expand Up @@ -329,7 +338,8 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
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)
Expand All @@ -344,17 +354,36 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
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) {
Expand Down Expand Up @@ -385,11 +414,10 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
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;
Expand All @@ -398,7 +426,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,
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);
Expand Down Expand Up @@ -432,7 +460,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,

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)
Expand All @@ -446,7 +474,8 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,
ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C,
ProgramStateRef state,
SizeArgExpr Size, AnyArgExpr First,
AnyArgExpr Second) const {
AnyArgExpr Second,
bool IsWide) const {
if (!Filter.CheckCStringBufferOverlap)
return state;

Expand Down Expand Up @@ -525,7 +554,7 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C,
// Convert the first buffer's start address to char*.
// Bail out if the cast fails.
ASTContext &Ctx = svalBuilder.getContext();
QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
QualType CharPtrTy = Ctx.getPointerType(IsWide ? Ctx.WideCharTy : Ctx.CharTy);
SVal FirstStart =
svalBuilder.evalCast(*firstLoc, CharPtrTy, First.Expression->getType());
Optional<Loc> FirstStartLoc = FirstStart.getAs<Loc>();
Expand Down Expand Up @@ -1161,7 +1190,7 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
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.
Expand Down Expand Up @@ -1204,11 +1233,11 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
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);
state = CheckOverlap(C, state, Size, Dest, Source, IsWide);

if (!state)
return;
Expand Down Expand Up @@ -1258,7 +1287,8 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallExpr *CE,
}
}

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};
Expand All @@ -1269,7 +1299,8 @@ void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const {

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 {
Expand All @@ -1281,7 +1312,8 @@ void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const {

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 {
Expand All @@ -1293,7 +1325,8 @@ void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const {

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 {
Expand All @@ -1304,7 +1337,8 @@ void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {

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 {
Expand Down Expand Up @@ -2336,7 +2370,7 @@ bool CStringChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {

// 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.
Expand Down

0 comments on commit 94ca2be

Please sign in to comment.