Skip to content

Commit

Permalink
[analyzer] Anti-aliasing: different heap allocations do not alias
Browse files Browse the repository at this point in the history
Add a concept of symbolic memory region belonging to heap memory space.
When comparing symbolic regions allocated on the heap, assume that they
do not alias. 

Use symbolic heap region to suppress a common false positive pattern in
the malloc checker, in code that relies on malloc not returning the
memory aliased to other malloc allocations, stack.

llvm-svn: 158136
  • Loading branch information
AnnaZaks committed Jun 7, 2012
1 parent 5fb2e4d commit 3563fde
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1130,8 +1130,11 @@ class MemRegionManager {
const CXXThisRegion *getCXXThisRegion(QualType thisPointerTy,
const LocationContext *LC);

/// getSymbolicRegion - Retrieve or create a "symbolic" memory region.
const SymbolicRegion* getSymbolicRegion(SymbolRef sym);
/// \brief Retrieve or create a "symbolic" memory region.
const SymbolicRegion* getSymbolicRegion(SymbolRef Sym);

/// \brief Return a unique symbolic region belonging to heap memory space.
const SymbolicRegion *getSymbolicHeapRegion(SymbolRef sym);

const StringRegion *getStringRegion(const StringLiteral* Str);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,12 @@ class SValBuilder {
const LocationContext *LCtx,
QualType type,
unsigned visitCount);
/// \brief Conjure a symbol representing heap allocated memory region.
///
/// Note, the expression should represent a location.
DefinedOrUnknownSVal getConjuredHeapSymbolVal(const Expr *E,
const LocationContext *LCtx,
unsigned Count);

DefinedOrUnknownSVal getDerivedRegionValueSymbolVal(
SymbolRef parentSymbol, const TypedValueRegion *region);
Expand Down
18 changes: 13 additions & 5 deletions clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,19 +477,27 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
const CallExpr *CE,
SVal Size, SVal Init,
ProgramStateRef state) {
// Get the return value.
SVal retVal = state->getSVal(CE, C.getLocationContext());

// Bind the return value to the symbolic value from the heap region.
// TODO: We could rewrite post visit to eval call; 'malloc' does not have
// side effects other than what we model here.
unsigned Count = C.getCurrentBlockCount();
SValBuilder &svalBuilder = C.getSValBuilder();
const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
DefinedSVal RetVal =
cast<DefinedSVal>(svalBuilder.getConjuredHeapSymbolVal(CE, LCtx, Count));
state = state->BindExpr(CE, C.getLocationContext(), RetVal);

// We expect the malloc functions to return a pointer.
if (!isa<Loc>(retVal))
if (!isa<Loc>(RetVal))
return 0;

// Fill the region with the initialization value.
state = state->bindDefault(retVal, Init);
state = state->bindDefault(RetVal, Init);

// Set the region's extent equal to the Size parameter.
const SymbolicRegion *R =
dyn_cast_or_null<SymbolicRegion>(retVal.getAsRegion());
dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion());
if (!R)
return 0;
if (isa<DefinedOrUnknownSVal>(Size)) {
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/StaticAnalyzer/Core/MemRegion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,10 @@ const SymbolicRegion *MemRegionManager::getSymbolicRegion(SymbolRef sym) {
return getSubRegion<SymbolicRegion>(sym, getUnknownRegion());
}

const SymbolicRegion *MemRegionManager::getSymbolicHeapRegion(SymbolRef Sym) {
return getSubRegion<SymbolicRegion>(Sym, getHeapRegion());
}

const FieldRegion*
MemRegionManager::getFieldRegion(const FieldDecl *d,
const MemRegion* superRegion){
Expand Down
12 changes: 12 additions & 0 deletions clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ SValBuilder::getConjuredSymbolVal(const Stmt *stmt,
return nonloc::SymbolVal(sym);
}

DefinedOrUnknownSVal
SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
const LocationContext *LCtx,
unsigned VisitCount) {
QualType T = E->getType();
assert(Loc::isLocType(T));
assert(SymbolManager::canSymbolicate(T));

SymbolRef sym = SymMgr.getConjuredSymbol(E, LCtx, T, VisitCount);
return loc::MemRegionVal(MemMgr.getSymbolicHeapRegion(sym));
}

DefinedSVal SValBuilder::getMetadataSymbolVal(const void *symbolTag,
const MemRegion *region,
const Expr *expr, QualType type,
Expand Down
43 changes: 23 additions & 20 deletions clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,18 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
// regions, though.
return UnknownVal();

// If both values wrap regions, see if they're from different base regions.
const MemSpaceRegion *LeftMS = LeftMR->getMemorySpace();
const MemSpaceRegion *RightMS = RightMR->getMemorySpace();
const MemSpaceRegion *UnknownMS = MemMgr.getUnknownRegion();
const MemRegion *LeftBase = LeftMR->getBaseRegion();
const MemRegion *RightBase = RightMR->getBaseRegion();
if (LeftBase != RightBase &&
!isa<SymbolicRegion>(LeftBase) && !isa<SymbolicRegion>(RightBase)) {

// If the two regions are from different known memory spaces they cannot be
// equal. Also, assume that no symbolic region (whose memory space is
// unknown) is on the stack.
if (LeftMS != RightMS &&
((LeftMS != UnknownMS && RightMS != UnknownMS) ||
(isa<StackSpaceRegion>(LeftMS) || isa<StackSpaceRegion>(RightMS)))) {
switch (op) {
default:
return UnknownVal();
Expand All @@ -688,24 +695,20 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
}
}

// The two regions are from the same base region. See if they're both a
// type of region we know how to compare.
const MemSpaceRegion *LeftMS = LeftBase->getMemorySpace();
const MemSpaceRegion *RightMS = RightBase->getMemorySpace();

// Heuristic: assume that no symbolic region (whose memory space is
// unknown) is on the stack.
// FIXME: we should be able to be more precise once we can do better
// aliasing constraints for symbolic regions, but this is a reasonable,
// albeit unsound, assumption that holds most of the time.
if (isa<StackSpaceRegion>(LeftMS) ^ isa<StackSpaceRegion>(RightMS)) {
// If both values wrap regions, see if they're from different base regions.
// Note, heap base symbolic regions are assumed to not alias with
// each other; for example, we assume that malloc returns different address
// on each invocation.
if (LeftBase != RightBase &&
((!isa<SymbolicRegion>(LeftBase) && !isa<SymbolicRegion>(RightBase)) ||
isa<HeapSpaceRegion>(LeftMS)) ){
switch (op) {
default:
break;
case BO_EQ:
return makeTruthVal(false, resultTy);
case BO_NE:
return makeTruthVal(true, resultTy);
default:
return UnknownVal();
case BO_EQ:
return makeTruthVal(false, resultTy);
case BO_NE:
return makeTruthVal(true, resultTy);
}
}

Expand Down
51 changes: 47 additions & 4 deletions clang/test/Analysis/malloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -839,10 +839,8 @@ int fPtr(unsigned cond, int x) {
return (cond ? mySub : myAdd)(x, x);
}

// ----------------------------------------------------------------------------
// Below are the known false positives.
// Test anti-aliasing.

// TODO: There should be no warning here. This one might be difficult to get rid of.
void dependsOnValueOfPtr(int *g, unsigned f) {
int *p;

Expand All @@ -855,10 +853,55 @@ void dependsOnValueOfPtr(int *g, unsigned f) {
if (p != g)
free(p);
else
return; // expected-warning{{Memory is never released; potential leak}}
return; // no warning
return;
}

int CMPRegionHeapToStack() {
int x = 0;
int *x1 = malloc(8);
int *x2 = &x;
if (x1 == x2)
return 5/x; // expected-warning{{This statement is never executed}}
free(x1);
return x;
}

int CMPRegionHeapToHeap2() {
int x = 0;
int *x1 = malloc(8);
int *x2 = malloc(8);
int *x4 = x1;
int *x5 = x2;
if (x4 == x5)
return 5/x; // expected-warning{{This statement is never executed}}
free(x1);
free(x2);
return x;
}

int CMPRegionHeapToHeap() {
int x = 0;
int *x1 = malloc(8);
int *x4 = x1;
if (x1 == x4) {
free(x1);
return 5/x; // expected-warning{{Division by zero}}
}
return x;// expected-warning{{This statement is never executed}}
}

int HeapAssignment() {
int m = 0;
int *x = malloc(4);
int *y = x;
*x = 5;
if (*x != *y)
return 5/m; // expected-warning{{This statement is never executed}}
free(x);
return 0;
}

// ----------------------------------------------------------------------------
// False negatives.

Expand Down

0 comments on commit 3563fde

Please sign in to comment.