From c89ad07d39a435a4069f890417747487f4b3abbf Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Thu, 7 Feb 2013 23:05:47 +0000 Subject: [PATCH] [analyzer] Report bugs when freeing memory with offset pointer The malloc checker will now catch the case when a previously malloc'ed region is freed, but the pointer passed to free does not point to the start of the allocated memory. For example: int *p1 = malloc(sizeof(int)); p1++; free(p1); // warn From the "memory.LeakPtrValChanged enhancement to unix.Malloc" entry in the list of potential checkers. A patch by Branden Archer! llvm-svn: 174678 --- .../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 91 ++++++++++++++----- clang/test/Analysis/malloc.c | 100 +++++++++++++++++++++ 2 files changed, 171 insertions(+), 20 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 9afd8cf..2242b21 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -129,6 +129,7 @@ class MallocChecker : public Checker BT_Leak; mutable OwningPtr BT_UseFree; mutable OwningPtr BT_BadFree; + mutable OwningPtr BT_OffsetFree; mutable IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc, *II_valloc, *II_reallocf, *II_strndup, *II_strdup; @@ -225,6 +226,7 @@ private: static bool SummarizeValue(raw_ostream &os, SVal V); static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const; + void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range)const; /// Find the location of the allocation for Sym on the path leading to the /// exploded node N. @@ -710,43 +712,55 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, ReportBadFree(C, ArgVal, ArgExpr->getSourceRange()); return 0; } - - const SymbolicRegion *SR = dyn_cast(R); + + const SymbolicRegion *SrBase = dyn_cast(R->getBaseRegion()); // Various cases could lead to non-symbol values here. // For now, ignore them. - if (!SR) + if (!SrBase) return 0; - SymbolRef Sym = SR->getSymbol(); - const RefState *RS = State->get(Sym); + SymbolRef SymBase = SrBase->getSymbol(); + const RefState *RsBase = State->get(SymBase); SymbolRef PreviousRetStatusSymbol = 0; // Check double free. - if (RS && - (RS->isReleased() || RS->isRelinquished()) && - !didPreviousFreeFail(State, Sym, PreviousRetStatusSymbol)) { + if (RsBase && + (RsBase->isReleased() || RsBase->isRelinquished()) && + !didPreviousFreeFail(State, SymBase, PreviousRetStatusSymbol)) { if (ExplodedNode *N = C.generateSink()) { if (!BT_DoubleFree) BT_DoubleFree.reset( new BugType("Double free", "Memory Error")); - BugReport *R = new BugReport(*BT_DoubleFree, - (RS->isReleased() ? "Attempt to free released memory" : - "Attempt to free non-owned memory"), N); + BugReport *R = new BugReport(*BT_DoubleFree, + (RsBase->isReleased() ? "Attempt to free released memory" + : "Attempt to free non-owned memory"), + N); R->addRange(ArgExpr->getSourceRange()); - R->markInteresting(Sym); + R->markInteresting(SymBase); if (PreviousRetStatusSymbol) R->markInteresting(PreviousRetStatusSymbol); - R->addVisitor(new MallocBugVisitor(Sym)); + R->addVisitor(new MallocBugVisitor(SymBase)); C.emitReport(R); } return 0; } - ReleasedAllocated = (RS != 0); + // Check if the memory location being freed is the actual location + // allocated, or an offset. + RegionOffset Offset = R->getAsOffset(); + if (RsBase && RsBase->isAllocated() && + Offset.isValid() && + !Offset.hasSymbolicOffset() && + Offset.getOffset() != 0) { + ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange()); + return 0; + } + + ReleasedAllocated = (RsBase != 0); // Clean out the info on previous call to free return info. - State = State->remove(Sym); + State = State->remove(SymBase); // Keep track of the return value. If it is NULL, we will know that free // failed. @@ -754,15 +768,17 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, SVal RetVal = C.getSVal(ParentExpr); SymbolRef RetStatusSymbol = RetVal.getAsSymbol(); if (RetStatusSymbol) { - C.getSymbolManager().addSymbolDependency(Sym, RetStatusSymbol); - State = State->set(Sym, RetStatusSymbol); + C.getSymbolManager().addSymbolDependency(SymBase, RetStatusSymbol); + State = State->set(SymBase, RetStatusSymbol); } } // Normal free. - if (Hold) - return State->set(Sym, RefState::getRelinquished(ParentExpr)); - return State->set(Sym, RefState::getReleased(ParentExpr)); + if (Hold) { + return State->set(SymBase, + RefState::getRelinquished(ParentExpr)); + } + return State->set(SymBase, RefState::getReleased(ParentExpr)); } bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) { @@ -891,6 +907,41 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, } } +void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, + SourceRange Range) const { + ExplodedNode *N = C.generateSink(); + if (N == NULL) + return; + + if (!BT_OffsetFree) + BT_OffsetFree.reset(new BugType("Offset free", "Memory Error")); + + SmallString<100> buf; + llvm::raw_svector_ostream os(buf); + + const MemRegion *MR = ArgVal.getAsRegion(); + assert(MR && "Only MemRegion based symbols can have offset free errors"); + + RegionOffset Offset = MR->getAsOffset(); + assert((Offset.isValid() && + !Offset.hasSymbolicOffset() && + Offset.getOffset() != 0) && + "Only symbols with a valid offset can have offset free errors"); + + int offsetBytes = Offset.getOffset() / C.getASTContext().getCharWidth(); + + os << "Argument to free() is offset by " + << offsetBytes + << " " + << ((abs(offsetBytes) > 1) ? "bytes" : "byte") + << " from the start of memory allocated by malloc()"; + + BugReport *R = new BugReport(*BT_OffsetFree, os.str(), N); + R->markInteresting(MR->getBaseRegion()); + R->addRange(Range); + C.emitReport(R); +} + ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE, bool FreesOnFail) const { diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c index 9dc17e6..29f5aa6 100644 --- a/clang/test/Analysis/malloc.c +++ b/clang/test/Analysis/malloc.c @@ -1090,4 +1090,104 @@ void testPassToSystemHeaderFunctionIndirectlyStruct() { fakeSystemHeaderCall(&ss); } // missing leak +int *testOffsetAllocate(size_t size) { + int *memoryBlock = (int *)malloc(size + sizeof(int)); + return &memoryBlock[1]; // no-warning +} + +void testOffsetDeallocate(int *memoryBlock) { + free(&memoryBlock[-1]); // no-warning +} + +void testOffsetOfRegionFreed() { + __int64_t * array = malloc(sizeof(__int64_t)*2); + array += 1; + free(&array[0]); // expected-warning{{Argument to free() is offset by 8 bytes from the start of memory allocated by malloc()}} +} + +void testOffsetOfRegionFreed2() { + __int64_t *p = malloc(sizeof(__int64_t)*2); + p += 1; + free(p); // expected-warning{{Argument to free() is offset by 8 bytes from the start of memory allocated by malloc()}} +} + +void testOffsetOfRegionFreed3() { + char *r = malloc(sizeof(char)); + r = r - 10; + free(r); // expected-warning {{Argument to free() is offset by -10 bytes from the start of memory allocated by malloc()}} +} + +void testOffsetOfRegionFreedAfterFunctionCall() { + int *p = malloc(sizeof(int)*2); + p += 1; + myfoo(p); + free(p); // no-warning +} + +void testFixManipulatedPointerBeforeFree() { + int * array = malloc(sizeof(int)*2); + array += 1; + free(&array[-1]); // no-warning +} + +void testFixManipulatedPointerBeforeFree2() { + char *r = malloc(sizeof(char)); + r = r + 10; + free(r-10); // no-warning +} + +void freeOffsetPointerPassedToFunction() { + __int64_t *p = malloc(sizeof(__int64_t)*2); + p[1] = 0; + p += 1; + myfooint(*p); // not passing the pointer, only a value pointed by pointer + free(p); // expected-warning {{Argument to free() is offset by 8 bytes from the start of memory allocated by malloc()}} +} + +int arbitraryInt(); +void freeUnknownOffsetPointer() { + char *r = malloc(sizeof(char)); + r = r + arbitraryInt(); // unable to reason about what the offset might be + free(r); // no-warning +} + +void testFreeNonMallocPointerWithNoOffset() { + char c; + char *r = &c; + r = r + 10; + free(r-10); // expected-warning {{Argument to free() is the address of the local variable 'c', which is not memory allocated by malloc()}} +} + +void testFreeNonMallocPointerWithOffset() { + char c; + char *r = &c; + free(r+1); // expected-warning {{Argument to free() is the address of the local variable 'c', which is not memory allocated by malloc()}} +} + +void testOffsetZeroDoubleFree() { + int *array = malloc(sizeof(int)*2); + int *p = &array[0]; + free(p); + free(&array[0]); // expected-warning{{Attempt to free released memory}} +} + +void testOffsetPassedToStrlen() { + char * string = malloc(sizeof(char)*10); + string += 1; + int length = strlen(string); // expected-warning {{Memory is never released; potential leak of memory pointed to by 'string'}} +} + +void testOffsetPassedToStrlenThenFree() { + char * string = malloc(sizeof(char)*10); + string += 1; + int length = strlen(string); + free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}} +} + +void testOffsetPassedAsConst() { + char * string = malloc(sizeof(char)*10); + string += 1; + passConstPtr(string); + free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}} +} -- 2.7.4