Skip to content

Commit

Permalink
[analyzer] Fix missed leak from MSVC specific allocation functions
Browse files Browse the repository at this point in the history 
Add the wide character strdup variants (wcsdup, _wcsdup) and the MSVC
version of alloca (_alloca) and other differently named function used
by the Malloc checker.

A patch by Alexander Riccio!

Differential Revision: http://reviews.llvm.org/D17688

llvm-svn: 262894
  • Loading branch information
@AnnaZaks
AnnaZaks committed Mar 8, 2016
1 parent 050b211 commit 30d4668
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 13 deletions.
36 changes: 23 additions & 13 deletions clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
View file
Expand Up @@ -169,11 +169,12 @@ class MallocChecker : public Checker<check::DeadSymbols,
{
public:
MallocChecker()
: II_alloca(nullptr), II_malloc(nullptr), II_free(nullptr),
II_realloc(nullptr), II_calloc(nullptr), II_valloc(nullptr),
II_reallocf(nullptr), II_strndup(nullptr), II_strdup(nullptr),
II_kmalloc(nullptr), II_if_nameindex(nullptr),
II_if_freenameindex(nullptr) {}
: II_alloca(nullptr), II_win_alloca(nullptr), II_malloc(nullptr),
II_free(nullptr), II_realloc(nullptr), II_calloc(nullptr),
II_valloc(nullptr), II_reallocf(nullptr), II_strndup(nullptr),
II_strdup(nullptr), II_win_strdup(nullptr), II_kmalloc(nullptr),
II_if_nameindex(nullptr), II_if_freenameindex(nullptr),
II_wcsdup(nullptr), II_win_wcsdup(nullptr) {}

/// In pessimistic mode, the checker assumes that it does not know which
/// functions might free the memory.
Expand Down Expand Up @@ -231,10 +232,11 @@ class MallocChecker : public Checker<check::DeadSymbols,
mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
mutable IdentifierInfo *II_alloca, *II_malloc, *II_free, *II_realloc,
*II_calloc, *II_valloc, *II_reallocf, *II_strndup,
*II_strdup, *II_kmalloc, *II_if_nameindex,
*II_if_freenameindex;
mutable IdentifierInfo *II_alloca, *II_win_alloca, *II_malloc, *II_free,
*II_realloc, *II_calloc, *II_valloc, *II_reallocf,
*II_strndup, *II_strdup, *II_win_strdup, *II_kmalloc,
*II_if_nameindex, *II_if_freenameindex, *II_wcsdup,
*II_win_wcsdup;
mutable Optional<uint64_t> KernelZeroFlagVal;

void initIdentifierInfo(ASTContext &C) const;
Expand Down Expand Up @@ -540,9 +542,15 @@ void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const {
II_valloc = &Ctx.Idents.get("valloc");
II_strdup = &Ctx.Idents.get("strdup");
II_strndup = &Ctx.Idents.get("strndup");
II_wcsdup = &Ctx.Idents.get("wcsdup");
II_kmalloc = &Ctx.Idents.get("kmalloc");
II_if_nameindex = &Ctx.Idents.get("if_nameindex");
II_if_freenameindex = &Ctx.Idents.get("if_freenameindex");

//MSVC uses `_`-prefixed instead, so we check for them too.
II_win_strdup = &Ctx.Idents.get("_strdup");
II_win_wcsdup = &Ctx.Idents.get("_wcsdup");
II_win_alloca = &Ctx.Idents.get("_alloca");
}

bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {
Expand Down Expand Up @@ -585,7 +593,8 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD,
if (Family == AF_Malloc && CheckAlloc) {
if (FunI == II_malloc || FunI == II_realloc || FunI == II_reallocf ||
FunI == II_calloc || FunI == II_valloc || FunI == II_strdup ||
FunI == II_strndup || FunI == II_kmalloc)
FunI == II_win_strdup || FunI == II_strndup || FunI == II_wcsdup ||
FunI == II_win_wcsdup || FunI == II_kmalloc)
return true;
}

Expand All @@ -600,7 +609,7 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD,
}

if (Family == AF_Alloca && CheckAlloc) {
if (FunI == II_alloca)
if (FunI == II_alloca || FunI == II_win_alloca)
return true;
}
}
Expand Down Expand Up @@ -789,11 +798,12 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
State = ProcessZeroAllocation(C, CE, 1, State);
} else if (FunI == II_free) {
State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
} else if (FunI == II_strdup) {
} else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
State = MallocUpdateRefState(C, CE, State);
} else if (FunI == II_strndup) {
State = MallocUpdateRefState(C, CE, State);
} else if (FunI == II_alloca) {
} else if (FunI == II_alloca || FunI == II_win_alloca) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
State = ProcessZeroAllocation(C, CE, 0, State);
Expand Down
76 changes: 76 additions & 0 deletions clang/test/Analysis/malloc.c
View file
Expand Up @@ -4,6 +4,21 @@

void clang_analyzer_eval(int);

// Without -fms-compatibility, wchar_t isn't a builtin type. MSVC defines
// _WCHAR_T_DEFINED if wchar_t is available. Microsoft recommends that you use
// the builtin type: "Using the typedef version can cause portability
// problems", but we're ok here because we're not actually running anything.
// Also of note is this cryptic warning: "The wchar_t type is not supported
// when you compile C code".
//
// See the docs for more:
// https://msdn.microsoft.com/en-us/library/dh8che7s.aspx
#if !defined(_WCHAR_T_DEFINED)
// "Microsoft implements wchar_t as a two-byte unsigned value"
typedef unsigned short wchar_t;
#define _WCHAR_T_DEFINED
#endif // !defined(_WCHAR_T_DEFINED)

typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
void *alloca(size_t);
Expand All @@ -13,9 +28,15 @@ void *realloc(void *ptr, size_t size);
void *reallocf(void *ptr, size_t size);
void *calloc(size_t nmemb, size_t size);
char *strdup(const char *s);
wchar_t *wcsdup(const wchar_t *s);
char *strndup(const char *s, size_t n);
int memcmp(const void *s1, const void *s2, size_t n);

// Windows variants
char *_strdup(const char *strSource);
wchar_t *_wcsdup(const wchar_t *strSource);
void *_alloca(size_t size);

void myfoo(int *p);
void myfooint(int p);
char *fooRetPtr();
Expand Down Expand Up @@ -55,6 +76,10 @@ void allocaTest() {
int *p = alloca(sizeof(int));
} // no warn

void winAllocaTest() {
int *p = _alloca(sizeof(int));
} // no warn

void allocaBuiltinTest() {
int *p = __builtin_alloca(sizeof(int));
} // no warn
Expand Down Expand Up @@ -210,6 +235,11 @@ void CheckUseZeroAllocatedNoWarn2() {
int *p = alloca(0); // no warning
}

void CheckUseZeroWinAllocatedNoWarn2() {
int *p = _alloca(0); // no warning
}


void CheckUseZeroAllocatedNoWarn3() {
int *p = malloc(0);
int *q = realloc(p, 8); // no warning
Expand All @@ -233,6 +263,11 @@ char CheckUseZeroAllocated2() {
return *p; // expected-warning {{Use of zero-allocated memory}}
}

char CheckUseZeroWinAllocated2() {
char *p = _alloca(0);
return *p; // expected-warning {{Use of zero-allocated memory}}
}

void UseZeroAllocated(int *p) {
if (p)
*p = 7; // expected-warning {{Use of zero-allocated memory}}
Expand Down Expand Up @@ -1076,6 +1111,21 @@ void testStrdup(const char *s, unsigned validIndex) {
s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}

void testWinStrdup(const char *s, unsigned validIndex) {
char *s2 = _strdup(s);
s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}

void testWcsdup(const wchar_t *s, unsigned validIndex) {
wchar_t *s2 = wcsdup(s);
s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}

void testWinWcsdup(const wchar_t *s, unsigned validIndex) {
wchar_t *s2 = _wcsdup(s);
s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}

int testStrndup(const char *s, unsigned validIndex, unsigned size) {
char *s2 = strndup(s, size);
s2 [validIndex + 1] = 'b';
Expand All @@ -1091,6 +1141,24 @@ void testStrdupContentIsDefined(const char *s, unsigned validIndex) {
free(s2);
}

void testWinStrdupContentIsDefined(const char *s, unsigned validIndex) {
char *s2 = _strdup(s);
char result = s2[1];// no warning
free(s2);
}

void testWcsdupContentIsDefined(const wchar_t *s, unsigned validIndex) {
wchar_t *s2 = wcsdup(s);
wchar_t result = s2[1];// no warning
free(s2);
}

void testWinWcsdupContentIsDefined(const wchar_t *s, unsigned validIndex) {
wchar_t *s2 = _wcsdup(s);
wchar_t result = s2[1];// no warning
free(s2);
}

// ----------------------------------------------------------------------------
// Test the system library functions to which the pointer can escape.
// This tests false positive suppression.
Expand Down Expand Up @@ -1444,6 +1512,14 @@ char *testLeakWithinReturn(char *str) {
return strdup(strdup(str)); // expected-warning{{leak}}
}

char *testWinLeakWithinReturn(char *str) {
return _strdup(_strdup(str)); // expected-warning{{leak}}
}

wchar_t *testWinWideLeakWithinReturn(wchar_t *str) {
return _wcsdup(_wcsdup(str)); // expected-warning{{leak}}
}

void passConstPtr(const char * ptr);

void testPassConstPointer() {
Expand Down

0 comments on commit 30d4668

Please sign in to comment.