From db0f9f00d23c343c6a142be9485c2b9abd72a503 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Tue, 22 Sep 2015 22:47:14 +0000 Subject: [PATCH] [analyzer] Make realloc(ptr, 0) handling equivalent to malloc(0). MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently realloc(ptr, 0) is treated as free() which seems to be not correct. C standard (N1570) establishes equivalent behavior for malloc(0) and realloc(ptr, 0): "7.22.3 Memory management functions calloc, malloc, realloc: If the size of the space requested is zero, the behavior is implementation-defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object." The patch equalizes the processing of malloc(0) and realloc(ptr,0). The patch also enables unix.Malloc checker to detect references to zero-allocated memory returned by realloc(ptr,0) ("Use of zero-allocated memory" warning). A patch by Антон Ярцев! Differential Revision: http://reviews.llvm.org/D9040 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@248336 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 38 ++++++++++++------- test/Analysis/malloc.c | 34 +++++++++++++++-- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index d5c5cc1dba..f924a76778 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -508,6 +508,7 @@ private: REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState) REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair) +REGISTER_SET_WITH_PROGRAMSTATE(ReallocSizeZeroSymbols, SymbolRef) // A map from the freed symbol to the symbol representing the return value of // the free function. @@ -891,15 +892,19 @@ ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C, return State; const RefState *RS = State->get(Sym); - if (!RS) - return State; // TODO: change to assert(RS); after realloc() will - // guarantee have a RegionState attached. - - if (!RS->isAllocated()) - return State; - - return TrueState->set(Sym, - RefState::getAllocatedOfSizeZero(RS)); + if (RS) { + if (RS->isAllocated()) + return TrueState->set(Sym, + RefState::getAllocatedOfSizeZero(RS)); + else + return State; + } else { + // Case of zero-size realloc. Historically 'realloc(ptr, 0)' is treated as + // 'free(ptr)' and the returned value from 'realloc(ptr, 0)' is not + // tracked. Add zero-reallocated Sym to the state to catch references + // to zero-allocated memory. + return TrueState->add(Sym); + } } // Assume the value is non-zero going forward. @@ -1487,6 +1492,9 @@ MallocChecker::getCheckIfTracked(CheckerContext &C, Optional MallocChecker::getCheckIfTracked(CheckerContext &C, SymbolRef Sym, bool IsALeakCheck) const { + if (C.getState()->contains(Sym)) + return CK_MallocChecker; + const RefState *RS = C.getState()->get(Sym); assert(RS); return getCheckIfTracked(RS->getAllocationFamily(), IsALeakCheck); @@ -1929,7 +1937,7 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, } if (PrtIsNull && SizeIsZero) - return nullptr; + return State; // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size). assert(!PrtIsNull); @@ -2293,10 +2301,14 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, const Stmt *S) const { assert(Sym); - const RefState *RS = C.getState()->get(Sym); - if (RS && RS->isAllocatedOfSizeZero()) - ReportUseZeroAllocated(C, RS->getStmt()->getSourceRange(), Sym); + if (const RefState *RS = C.getState()->get(Sym)) { + if (RS->isAllocatedOfSizeZero()) + ReportUseZeroAllocated(C, RS->getStmt()->getSourceRange(), Sym); + } + else if (C.getState()->contains(Sym)) { + ReportUseZeroAllocated(C, S->getSourceRange(), Sym); + } } bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const { diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index 3a7247c473..0962d0dbbf 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -263,21 +263,21 @@ void CheckUseZeroAllocated6() { void CheckUseZeroAllocated7() { int *p = realloc(0, 0); - *p = 1; //TODO: warn about use of zero-allocated memory + *p = 1; // expected-warning {{Use of zero-allocated memory}} free(p); } void CheckUseZeroAllocated8() { int *p = malloc(8); int *q = realloc(p, 0); - *q = 1; //TODO: warn about use of zero-allocated memory + *q = 1; // expected-warning {{Use of zero-allocated memory}} free(q); } void CheckUseZeroAllocated9() { int *p = realloc(0, 0); int *q = realloc(p, 0); - *q = 1; //TODO: warn about use of zero-allocated memory + *q = 1; // expected-warning {{Use of zero-allocated memory}} free(q); } @@ -307,6 +307,34 @@ void CheckUseZeroAllocatedPathWarn(_Bool b) { free(p); } +void CheckUseZeroReallocatedPathNoWarn(_Bool b) { + int s = 0; + if (b) + s= 10; + + char *p = malloc(8); + char *q = realloc(p, s); + + if (b) + *q = 1; // no warning + + free(q); +} + +void CheckUseZeroReallocatedPathWarn(_Bool b) { + int s = 10; + if (b) + s= 0; + + char *p = malloc(8); + char *q = realloc(p, s); + + if (b) + *q = 1; // expected-warning {{Use of zero-allocated memory}} + + free(q); +} + // This case tests that storing malloc'ed memory to a static variable which is // then returned is not leaked. In the absence of known contracts for functions // or inter-procedural analysis, this is a conservative answer. -- 2.40.0