From 30d46687740e51dbc4443cb72d10f0d0eed8d2c5 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Tue, 8 Mar 2016 01:21:51 +0000 Subject: [PATCH] [analyzer] Fix missed leak from MSVC specific allocation functions 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 --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 36 +++++---- clang/test/Analysis/malloc.c | 76 +++++++++++++++++++ 2 files changed, 99 insertions(+), 13 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index fee030feb6d20..f365e536676bc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -169,11 +169,12 @@ class MallocChecker : public Checker BT_MismatchedDealloc; mutable std::unique_ptr BT_OffsetFree[CK_NumCheckKinds]; mutable std::unique_ptr 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 KernelZeroFlagVal; void initIdentifierInfo(ASTContext &C) const; @@ -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 { @@ -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; } @@ -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; } } @@ -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); diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 881eb38ad8401..30d72691956a6 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -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); @@ -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(); @@ -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 @@ -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 @@ -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}} @@ -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'; @@ -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. @@ -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() {