r174678 - [analyzer] Report bugs when freeing memory with offset pointer

Anna Zaks ganna at apple.com
Thu Feb 7 15:05:47 PST 2013


Author: zaks
Date: Thu Feb  7 17:05:47 2013
New Revision: 174678

URL: http://llvm.org/viewvc/llvm-project?rev=174678&view=rev
Log:
[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!

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    cfe/trunk/test/Analysis/malloc.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=174678&r1=174677&r2=174678&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Thu Feb  7 17:05:47 2013
@@ -129,6 +129,7 @@ class MallocChecker : public Checker<che
   mutable OwningPtr<BugType> BT_Leak;
   mutable OwningPtr<BugType> BT_UseFree;
   mutable OwningPtr<BugType> BT_BadFree;
+  mutable OwningPtr<BugType> 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::FreeMemAu
     ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
     return 0;
   }
-  
-  const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(R);
+
+  const SymbolicRegion *SrBase = dyn_cast<SymbolicRegion>(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<RegionState>(Sym);
+  SymbolRef SymBase = SrBase->getSymbol();
+  const RefState *RsBase = State->get<RegionState>(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<FreeReturnValue>(Sym);
+  State = State->remove<FreeReturnValue>(SymBase);
 
   // Keep track of the return value. If it is NULL, we will know that free 
   // failed.
@@ -754,15 +768,17 @@ ProgramStateRef MallocChecker::FreeMemAu
     SVal RetVal = C.getSVal(ParentExpr);
     SymbolRef RetStatusSymbol = RetVal.getAsSymbol();
     if (RetStatusSymbol) {
-      C.getSymbolManager().addSymbolDependency(Sym, RetStatusSymbol);
-      State = State->set<FreeReturnValue>(Sym, RetStatusSymbol);
+      C.getSymbolManager().addSymbolDependency(SymBase, RetStatusSymbol);
+      State = State->set<FreeReturnValue>(SymBase, RetStatusSymbol);
     }
   }
 
   // Normal free.
-  if (Hold)
-    return State->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr));
-  return State->set<RegionState>(Sym, RefState::getReleased(ParentExpr));
+  if (Hold) {
+    return State->set<RegionState>(SymBase,
+                                   RefState::getRelinquished(ParentExpr));
+  }
+  return State->set<RegionState>(SymBase, RefState::getReleased(ParentExpr));
 }
 
 bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
@@ -891,6 +907,41 @@ void MallocChecker::ReportBadFree(Checke
   }
 }
 
+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 {

Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=174678&r1=174677&r2=174678&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Thu Feb  7 17:05:47 2013
@@ -1090,4 +1090,104 @@ void testPassToSystemHeaderFunctionIndir
   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()}}
+}
 





More information about the cfe-commits mailing list