Skip to content

Commit

Permalink
[analyzer] Different handling of alloca().
Browse files Browse the repository at this point in the history 
+ separate bug report for "Free alloca()" error to be able to customize checkers responsible for this error.
+ Muted "Free alloca()" error for NewDelete checker that is not responsible for c-allocated memory, turned on for unix.MismatchedDeallocator checker.
+ RefState for alloca() - to be able to detect usage of zero-allocated memory by upcoming ZeroAllocDereference checker.
+ AF_Alloca family to handle alloca() consistently - keep proper family in RefState, handle 'alloca' by getCheckIfTracked() facility, etc.
+ extra tests.

llvm-svn: 229850
  • Loading branch information
@ayartsev7
ayartsev7 committed Feb 19, 2015
1 parent f0f0d27 commit 5b5c7ce
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 41 deletions.
108 changes: 78 additions & 30 deletions clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ enum AllocationFamily {
AF_Malloc,
AF_CXXNew,
AF_CXXNewArray,
AF_IfNameIndex
AF_IfNameIndex,
AF_Alloca
};

class RefState {
Expand Down Expand Up @@ -160,10 +161,11 @@ class MallocChecker : public Checker<check::DeadSymbols,
{
public:
MallocChecker()
: II_malloc(nullptr), II_free(nullptr), II_realloc(nullptr),
II_calloc(nullptr), II_valloc(nullptr), II_reallocf(nullptr),
II_strndup(nullptr), II_strdup(nullptr), II_kmalloc(nullptr),
II_if_nameindex(nullptr), II_if_freenameindex(nullptr) {}
: II_alloca(nullptr), II_alloca_builtin(nullptr), II_malloc(nullptr),
II_free(nullptr), II_realloc(nullptr), II_calloc(nullptr),
II_valloc(nullptr), II_reallocf(nullptr), II_strndup(nullptr),
II_strdup(nullptr), II_kmalloc(nullptr), II_if_nameindex(nullptr),
II_if_freenameindex(nullptr) {}

/// In pessimistic mode, the checker assumes that it does not know which
/// functions might free the memory.
Expand Down Expand Up @@ -217,11 +219,13 @@ class MallocChecker : public Checker<check::DeadSymbols,
mutable std::unique_ptr<BugType> BT_Leak[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_UseFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_BadFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_FreeAlloca[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
mutable IdentifierInfo *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_alloca_builtin, *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 Optional<uint64_t> KernelZeroFlagVal;

void initIdentifierInfo(ASTContext &C) const;
Expand Down Expand Up @@ -343,6 +347,8 @@ class MallocChecker : public Checker<check::DeadSymbols,
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
const Expr *DeallocExpr) const;
void ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
SourceRange Range) const;
void ReportMismatchedDealloc(CheckerContext &C, SourceRange Range,
const Expr *DeallocExpr, const RefState *RS,
SymbolRef Sym, bool OwnershipTransferred) const;
Expand Down Expand Up @@ -501,6 +507,8 @@ class StopTrackingCallback : public SymbolVisitor {
void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const {
if (II_malloc)
return;
II_alloca = &Ctx.Idents.get("alloca");
II_alloca_builtin = &Ctx.Idents.get("__builtin_alloca");
II_malloc = &Ctx.Idents.get("malloc");
II_free = &Ctx.Idents.get("free");
II_realloc = &Ctx.Idents.get("realloc");
Expand All @@ -521,6 +529,9 @@ bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {
if (isCMemFunction(FD, C, AF_IfNameIndex, MemoryOperationKind::MOK_Any))
return true;

if (isCMemFunction(FD, C, AF_Alloca, MemoryOperationKind::MOK_Any))
return true;

if (isStandardNewDelete(FD, C))
return true;

Expand Down Expand Up @@ -564,6 +575,11 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD,
if (FunI == II_if_nameindex)
return true;
}

if (Family == AF_Alloca && CheckAlloc) {
if (FunI == II_alloca || FunI == II_alloca_builtin)
return true;
}
}

if (Family != AF_Malloc)
Expand Down Expand Up @@ -747,8 +763,10 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
State = MallocUpdateRefState(C, CE, State);
} else if (FunI == II_strndup) {
State = MallocUpdateRefState(C, CE, State);
}
else if (isStandardNewDelete(FD, C.getASTContext())) {
} else if (FunI == II_alloca || FunI == II_alloca_builtin) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
} else if (isStandardNewDelete(FD, C.getASTContext())) {
// Process direct calls to operator new/new[]/delete/delete[] functions
// as distinct from new/new[]/delete/delete[] expressions that are
// processed by the checkPostStmt callbacks for CXXNewExpr and
Expand Down Expand Up @@ -1096,6 +1114,9 @@ AllocationFamily MallocChecker::getAllocationFamily(CheckerContext &C,
if (isCMemFunction(FD, Ctx, AF_IfNameIndex, MemoryOperationKind::MOK_Any))
return AF_IfNameIndex;

if (isCMemFunction(FD, Ctx, AF_Alloca, MemoryOperationKind::MOK_Any))
return AF_Alloca;

return AF_None;
}

Expand Down Expand Up @@ -1160,6 +1181,7 @@ void MallocChecker::printExpectedAllocName(raw_ostream &os, CheckerContext &C,
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
case AF_IfNameIndex: os << "'if_nameindex()'"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("not a deallocation expression");
}
}
Expand All @@ -1171,7 +1193,8 @@ void MallocChecker::printExpectedDeallocName(raw_ostream &os,
case AF_CXXNew: os << "'delete'"; return;
case AF_CXXNewArray: os << "'delete[]'"; return;
case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
case AF_None: llvm_unreachable("suspicious AF_None argument");
case AF_Alloca:
case AF_None: llvm_unreachable("suspicious argument");
}
}

Expand Down Expand Up @@ -1225,8 +1248,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,

const MemSpaceRegion *MS = R->getMemorySpace();

// Parameters, locals, statics, globals, and memory returned by alloca()
// shouldn't be freed.
// Parameters, locals, statics and globals shouldn't be freed.
if (!(isa<UnknownSpaceRegion>(MS) || isa<HeapSpaceRegion>(MS))) {
// FIXME: at the time this code was written, malloc() regions were
// represented by conjured symbols, which are all in UnknownSpaceRegion.
Expand All @@ -1252,6 +1274,12 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,

if (RsBase) {

// Memory returned by alloca() shouldn't be freed.
if (RsBase->getAllocationFamily() == AF_Alloca) {
ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
return nullptr;
}

// Check for double free first.
if ((RsBase->isReleased() || RsBase->isRelinquished()) &&
!didPreviousFreeFail(State, SymBase, PreviousRetStatusSymbol)) {
Expand Down Expand Up @@ -1327,7 +1355,8 @@ MallocChecker::getCheckIfTracked(MallocChecker::CheckKind CK,

switch (Family) {
case AF_Malloc:
case AF_IfNameIndex: {
case AF_IfNameIndex:
case AF_Alloca: {
// C checkers.
if (CK == CK_MallocOptimistic ||
CK == CK_MallocPessimistic) {
Expand Down Expand Up @@ -1502,23 +1531,19 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
while (const ElementRegion *ER = dyn_cast_or_null<ElementRegion>(MR))
MR = ER->getSuperRegion();

if (MR && isa<AllocaRegion>(MR))
os << "Memory allocated by alloca() should not be deallocated";
else {
os << "Argument to ";
if (!printAllocDeallocName(os, C, DeallocExpr))
os << "deallocator";

os << " is ";
bool Summarized = MR ? SummarizeRegion(os, MR)
: SummarizeValue(os, ArgVal);
if (Summarized)
os << ", which is not memory allocated by ";
else
os << "not memory allocated by ";
os << "Argument to ";
if (!printAllocDeallocName(os, C, DeallocExpr))
os << "deallocator";

printExpectedAllocName(os, C, DeallocExpr);
}
os << " is ";
bool Summarized = MR ? SummarizeRegion(os, MR)
: SummarizeValue(os, ArgVal);
if (Summarized)
os << ", which is not memory allocated by ";
else
os << "not memory allocated by ";

printExpectedAllocName(os, C, DeallocExpr);

BugReport *R = new BugReport(*BT_BadFree[*CheckKind], os.str(), N);
R->markInteresting(MR);
Expand All @@ -1527,6 +1552,29 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
}
}

void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
SourceRange Range) const {

auto CheckKind = getCheckIfTracked(MakeVecFromCK(CK_MallocOptimistic,
CK_MallocPessimistic,
CK_MismatchedDeallocatorChecker),
AF_Alloca);
if (!CheckKind.hasValue())
return;

if (ExplodedNode *N = C.generateSink()) {
if (!BT_FreeAlloca[*CheckKind])
BT_FreeAlloca[*CheckKind].reset(
new BugType(CheckNames[*CheckKind], "Free alloca()", "Memory Error"));

BugReport *R = new BugReport(*BT_FreeAlloca[*CheckKind],
"Memory allocated by alloca() should not be deallocated", N);
R->markInteresting(ArgVal.getAsRegion());
R->addRange(Range);
C.emitReport(R);
}
}

void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
SourceRange Range,
const Expr *DeallocExpr,
Expand Down
5 changes: 5 additions & 0 deletions clang/test/Analysis/MismatchedDeallocator-checker-test.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ void testMalloc8() {
operator delete[](p); // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not operator delete[]}}
}

void testAlloca() {
int *p = (int *)__builtin_alloca(sizeof(int));
delete p; // expected-warning{{Memory allocated by alloca() should not be deallocated}}
}

//--------------- test new family
void testNew1() {
int *p = new int;
Expand Down
5 changes: 0 additions & 5 deletions clang/test/Analysis/NewDelete-checker-test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,6 @@ void testUseThisAfterDelete() {
c->f(0); // expected-warning{{Use of memory after it is freed}}
}

void testDeleteAlloca() {
int *p = (int *)__builtin_alloca(sizeof(int));
delete p; // expected-warning{{Memory allocated by alloca() should not be deallocated}}
}

void testDoubleDelete() {
int *p = new int;
delete p;
Expand Down
11 changes: 9 additions & 2 deletions clang/test/Analysis/NewDelete-intersections.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

typedef __typeof__(sizeof(int)) size_t;
extern "C" void *malloc(size_t);
extern "C" void *alloca(size_t);
extern "C" void free(void *);

//----------------------------------------------------------------------------
Expand All @@ -29,11 +30,17 @@ void testMallocFreeNoWarn() {
int *p4 = (int *)malloc(sizeof(int));
free(p4);
int j = *p4; // no warn

int *p5 = (int *)alloca(sizeof(int));
free(p5); // no warn
}

void testDeleteMalloced() {
int *p = (int *)malloc(sizeof(int));
delete p; // no warn
int *p1 = (int *)malloc(sizeof(int));
delete p1; // no warn

int *p2 = (int *)__builtin_alloca(sizeof(int));
delete p2; // no warn
}

//----- Test free standard new
Expand Down
15 changes: 11 additions & 4 deletions clang/test/Analysis/free.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify %s
// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-checker=core,alpha.unix.MallocWithAnnotations -fblocks -verify %s
typedef __typeof(sizeof(int)) size_t;
void free(void *);
void *alloca(size_t);

void t1 () {
int a[] = { 1 };
Expand Down Expand Up @@ -49,24 +51,29 @@ void t10 () {
}

void t11 () {
char *p = (char*)__builtin_alloca(2);
char *p = (char*)alloca(2);
free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
}

void t12 () {
char *p = (char*)__builtin_alloca(2);
free(p); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
}

void t13 () {
free(^{return;}); // expected-warning {{Argument to free() is a block, which is not memory allocated by malloc()}}
}

void t13 (char a) {
void t14 (char a) {
free(&a); // expected-warning {{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}}
}

static int someGlobal[2];
void t14 () {
void t15 () {
free(someGlobal); // expected-warning {{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}}
}

void t15 (char **x, int offset) {
void t16 (char **x, int offset) {
// Unknown value
free(x[offset]); // no-warning
}

0 comments on commit 5b5c7ce

Please sign in to comment.