Skip to content

Commit

Permalink
[analyzer] Let TK_PreserveContents span across the whole base region.
Browse files Browse the repository at this point in the history
If an address of a field is passed through a const pointer,
the whole structure's base region should receive the
TK_PreserveContents trait and avoid invalidation.

Additionally, include a few FIXME tests shown up during testing.

Differential Revision: http://reviews.llvm.org/D19057

llvm-svn: 267413
  • Loading branch information
haoNoQ committed Apr 25, 2016
1 parent dd21523 commit 70247e6
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 2 deletions.
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
Expand Up @@ -920,7 +920,7 @@ ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C,
// Invalidate and escape only indirect regions accessible through the source
// buffer.
if (IsSourceBuffer) {
ITraits.setTrait(R,
ITraits.setTrait(R->getBaseRegion(),
RegionAndSymbolInvalidationTraits::TK_PreserveContents);
ITraits.setTrait(R, RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
CausesPointerEscape = true;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Core/CallEvent.cpp
Expand Up @@ -177,7 +177,7 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
// below for efficiency.
if (PreserveArgs.count(Idx))
if (const MemRegion *MR = getArgSVal(Idx).getAsRegion())
ETraits.setTrait(MR->StripCasts(),
ETraits.setTrait(MR->getBaseRegion(),
RegionAndSymbolInvalidationTraits::TK_PreserveContents);
// TODO: Factor this out + handle the lower level const pointers.

Expand Down
47 changes: 47 additions & 0 deletions clang/test/Analysis/call-invalidation.cpp
Expand Up @@ -118,3 +118,50 @@ void testPureConst() {
}


struct PlainStruct {
int x, y;
mutable int z;
};

PlainStruct glob;

void useAnything(void *);
void useAnythingConst(const void *);

void testInvalidationThroughBaseRegionPointer() {
PlainStruct s1;
s1.x = 1;
s1.z = 1;
clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
// Not only passing a structure pointer through const pointer parameter,
// but also passing a field pointer through const pointer parameter
// should preserve the contents of the structure.
useAnythingConst(&(s1.y));
clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
// FIXME: Should say "UNKNOWN", because it is not uncommon to
// modify a mutable member variable through const pointer.
clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
useAnything(&(s1.y));
clang_analyzer_eval(s1.x == 1); // expected-warning{{UNKNOWN}}
}


void useFirstConstSecondNonConst(const void *x, void *y);
void useFirstNonConstSecondConst(void *x, const void *y);

void testMixedConstNonConstCalls() {
PlainStruct s2;
s2.x = 1;
useFirstConstSecondNonConst(&(s2.x), &(s2.y));
clang_analyzer_eval(s2.x == 1); // expected-warning{{UNKNOWN}}
s2.x = 1;
useFirstNonConstSecondConst(&(s2.x), &(s2.y));
clang_analyzer_eval(s2.x == 1); // expected-warning{{UNKNOWN}}
s2.y = 1;
useFirstConstSecondNonConst(&(s2.x), &(s2.y));
clang_analyzer_eval(s2.y == 1); // expected-warning{{UNKNOWN}}
s2.y = 1;
useFirstNonConstSecondConst(&(s2.x), &(s2.y));
clang_analyzer_eval(s2.y == 1); // expected-warning{{UNKNOWN}}
}
19 changes: 19 additions & 0 deletions clang/test/Analysis/malloc.c
Expand Up @@ -1750,6 +1750,19 @@ void testEscapeThroughSystemCallTakingVoidPointer3(fake_rb_tree_t *rbt) {
fake_rb_tree_insert_node(rbt, data); // no warning
}

struct IntAndPtr {
int x;
int *p;
};

void constEscape(const void *ptr);

void testConstEscapeThroughAnotherField() {
struct IntAndPtr s;
s.p = malloc(sizeof(int));
constEscape(&(s.x)); // could free s->p!
} // no-warning

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

Expand All @@ -1769,3 +1782,9 @@ void testPassToSystemHeaderFunctionIndirectly() {
// FIXME: This is a leak: if we think a system function won't free p, it
// won't free (p-1) either.
}

void testMallocIntoMalloc() {
StructWithPtr *s = malloc(sizeof(StructWithPtr));
s->memP = malloc(sizeof(int));
free(s);
} // FIXME: should warn here

0 comments on commit 70247e6

Please sign in to comment.