[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!
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174678 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 9afd8cf..2242b21 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -129,6 +129,7 @@
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 @@
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 @@
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 @@
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::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 {