r194953 - [analyzer] Better modeling of memcpy by the CStringChecker (PR16731).
Anton Yartsev
anton.yartsev at gmail.com
Sun Nov 17 01:18:48 PST 2013
Author: ayartsev
Date: Sun Nov 17 03:18:48 2013
New Revision: 194953
URL: http://llvm.org/viewvc/llvm-project?rev=194953&view=rev
Log:
[analyzer] Better modeling of memcpy by the CStringChecker (PR16731).
New rules of invalidation/escape of the source buffer of memcpy: the source buffer contents is invalidated and escape while the source buffer region itself is neither invalidated, nor escape.
In the current modeling of memcpy the information about allocation state of regions, accessible through the source buffer, is not copied to the destination buffer and we can not track the allocation state of those regions anymore. So we invalidate/escape the source buffer indirect regions in anticipation of their being invalidated for real later. This eliminates false-positive leaks reported by the unix.Malloc and alpha.cplusplus.NewDeleteLeaks checkers for the cases like
char *f() {
void *x = malloc(47);
char *a;
memcpy(&a, &x, sizeof a);
return a;
}
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
cfe/trunk/test/Analysis/malloc.c
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h?rev=194953&r1=194952&r2=194953&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h Sun Nov 17 03:18:48 2013
@@ -337,7 +337,9 @@ class PointerEscape {
for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
E = Escaped.end(); I != E; ++I)
if (!ETraits->hasTrait(*I,
- RegionAndSymbolInvalidationTraits::TK_PreserveContents))
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents) &&
+ !ETraits->hasTrait(*I,
+ RegionAndSymbolInvalidationTraits::TK_SuppressEscape))
RegularEscape.insert(*I);
if (RegularEscape.empty())
@@ -375,7 +377,9 @@ class ConstPointerEscape {
for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
E = Escaped.end(); I != E; ++I)
if (ETraits->hasTrait(*I,
- RegionAndSymbolInvalidationTraits::TK_PreserveContents))
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents) &&
+ !ETraits->hasTrait(*I,
+ RegionAndSymbolInvalidationTraits::TK_SuppressEscape))
ConstEscape.insert(*I);
if (ConstEscape.empty())
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=194953&r1=194952&r2=194953&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Sun Nov 17 03:18:48 2013
@@ -1329,7 +1329,9 @@ public:
/// \brief Describes different invalidation traits.
enum InvalidationKinds {
/// Tells that a region's contents is not changed.
- TK_PreserveContents = 0x1
+ TK_PreserveContents = 0x1,
+ /// Suppress pointer-escaping of a region.
+ TK_SuppressEscape = 0x2
// Do not forget to extend StorageTypeForKinds if number of traits exceed
// the number of bits StorageTypeForKinds can store.
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=194953&r1=194952&r2=194953&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Sun Nov 17 03:18:48 2013
@@ -232,21 +232,21 @@ public:
/// \param IS the set of invalidated symbols.
/// \param Call if non-null, the invalidated regions represent parameters to
/// the call and should be considered directly invalidated.
- /// \param HTraits information about special handling for a particular
+ /// \param ITraits information about special handling for a particular
/// region/symbol.
ProgramStateRef
invalidateRegions(ArrayRef<const MemRegion *> Regions, const Expr *E,
unsigned BlockCount, const LocationContext *LCtx,
bool CausesPointerEscape, InvalidatedSymbols *IS = 0,
const CallEvent *Call = 0,
- RegionAndSymbolInvalidationTraits *HTraits = 0) const;
+ RegionAndSymbolInvalidationTraits *ITraits = 0) const;
ProgramStateRef
invalidateRegions(ArrayRef<SVal> Regions, const Expr *E,
unsigned BlockCount, const LocationContext *LCtx,
bool CausesPointerEscape, InvalidatedSymbols *IS = 0,
const CallEvent *Call = 0,
- RegionAndSymbolInvalidationTraits *HTraits = 0) const;
+ RegionAndSymbolInvalidationTraits *ITraits = 0) const;
/// enterStackFrame - Returns the state for entry to the given stack frame,
/// preserving the current state.
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=194953&r1=194952&r2=194953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Sun Nov 17 03:18:48 2013
@@ -141,8 +141,9 @@ public:
SVal val) const;
static ProgramStateRef InvalidateBuffer(CheckerContext &C,
- ProgramStateRef state,
- const Expr *Ex, SVal V);
+ ProgramStateRef state,
+ const Expr *Ex, SVal V,
+ bool IsSourceBuffer);
static bool SummarizeRegion(raw_ostream &os, ASTContext &Ctx,
const MemRegion *MR);
@@ -809,8 +810,9 @@ const StringLiteral *CStringChecker::get
}
ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C,
- ProgramStateRef state,
- const Expr *E, SVal V) {
+ ProgramStateRef state,
+ const Expr *E, SVal V,
+ bool IsSourceBuffer) {
Optional<Loc> L = V.getAs<Loc>();
if (!L)
return state;
@@ -830,8 +832,20 @@ ProgramStateRef CStringChecker::Invalida
// Invalidate this region.
const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
- return state->invalidateRegions(R, E, C.blockCount(), LCtx,
- /*CausesPointerEscape*/ false);
+
+ bool CausesPointerEscape = false;
+ RegionAndSymbolInvalidationTraits ITraits;
+ // Invalidate and escape only indirect regions accessible through the source
+ // buffer.
+ if (IsSourceBuffer) {
+ ITraits.setTrait(R,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+ ITraits.setTrait(R, RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
+ CausesPointerEscape = true;
+ }
+
+ return state->invalidateRegions(R, E, C.blockCount(), LCtx,
+ CausesPointerEscape, 0, 0, &ITraits);
}
// If we have a non-region value by chance, just remove the binding.
@@ -968,13 +982,20 @@ void CStringChecker::evalCopyCommon(Chec
state = state->BindExpr(CE, LCtx, destVal);
}
- // Invalidate the destination.
+ // Invalidate the destination (regular invalidation without pointer-escaping
+ // the address of the top-level region).
// FIXME: Even if we can't perfectly model the copy, we should see if we
// can use LazyCompoundVals to copy the source values into the destination.
// This would probably remove any existing bindings past the end of the
// copied region, but that's still an improvement over blank invalidation.
- state = InvalidateBuffer(C, state, Dest,
- state->getSVal(Dest, C.getLocationContext()));
+ state = InvalidateBuffer(C, state, Dest, C.getSVal(Dest),
+ /*IsSourceBuffer*/false);
+
+ // Invalidate the source (const-invalidation without const-pointer-escaping
+ // the address of the top-level region).
+ state = InvalidateBuffer(C, state, Source, C.getSVal(Source),
+ /*IsSourceBuffer*/true);
+
C.addTransition(state);
}
}
@@ -1577,13 +1598,19 @@ void CStringChecker::evalStrcpyCommon(Ch
Result = lastElement;
}
- // Invalidate the destination. This must happen before we set the C string
- // length because invalidation will clear the length.
+ // Invalidate the destination (regular invalidation without pointer-escaping
+ // the address of the top-level region). This must happen before we set the
+ // C string length because invalidation will clear the length.
// FIXME: Even if we can't perfectly model the copy, we should see if we
// can use LazyCompoundVals to copy the source values into the destination.
// This would probably remove any existing bindings past the end of the
// string, but that's still an improvement over blank invalidation.
- state = InvalidateBuffer(C, state, Dst, *dstRegVal);
+ state = InvalidateBuffer(C, state, Dst, *dstRegVal,
+ /*IsSourceBuffer*/false);
+
+ // Invalidate the source (const-invalidation without const-pointer-escaping
+ // the address of the top-level region).
+ state = InvalidateBuffer(C, state, srcExpr, srcVal, /*IsSourceBuffer*/true);
// Set the C string length of the destination, if we know it.
if (isBounded && !isAppending) {
@@ -1805,7 +1832,8 @@ void CStringChecker::evalStrsep(CheckerC
// Invalidate the search string, representing the change of one delimiter
// character to NUL.
- State = InvalidateBuffer(C, State, SearchStrPtr, Result);
+ State = InvalidateBuffer(C, State, SearchStrPtr, Result,
+ /*IsSourceBuffer*/false);
// Overwrite the search string pointer. The new value is either an address
// further along in the same string, or NULL if there are no more tokens.
Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator.h?rev=194953&r1=194952&r2=194953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator.h Sun Nov 17 03:18:48 2013
@@ -32,6 +32,7 @@ typedef __typeof(sizeof(int)) size_t;
size_t strlen(const char *);
char *strcpy(char *restrict, const char *restrict);
+void *memcpy(void *dst, const void *src, size_t n);
typedef unsigned long __darwin_pthread_key_t;
typedef __darwin_pthread_key_t pthread_key_t;
Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=194953&r1=194952&r2=194953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Sun Nov 17 03:18:48 2013
@@ -627,8 +627,49 @@ void doNotInvalidateWhenPassedToSystemCa
char *p = malloc(12);
strlen(p);
strcpy(p, s);
+ strcpy(s, p);
+ strcpy(p, p);
+ memcpy(p, s, 1);
+ memcpy(s, p, 1);
+ memcpy(p, p, 1);
} // expected-warning {{leak}}
+// Treat source buffer contents as escaped.
+void escapeSourceContents(char *s) {
+ char *p = malloc(12);
+ memcpy(s, &p, 12); // no warning
+
+ void *p1 = malloc(7);
+ char *a;
+ memcpy(&a, &p1, sizeof a);
+ // FIXME: No warning due to limitations imposed by current modelling of
+ // 'memcpy' (regions metadata is not copied).
+
+ int *ptrs[2];
+ int *allocated = (int *)malloc(4);
+ memcpy(&ptrs[0], &allocated, sizeof(int *));
+ // FIXME: No warning due to limitations imposed by current modelling of
+ // 'memcpy' (regions metadata is not copied).
+}
+
+void invalidateDestinationContents() {
+ int *null = 0;
+ int *p = (int *)malloc(4);
+ memcpy(&p, &null, sizeof(int *));
+
+ int *ptrs1[2]; // expected-warning {{Potential leak of memory pointed to by}}
+ ptrs1[0] = (int *)malloc(4);
+ memcpy(ptrs1, &null, sizeof(int *));
+
+ int *ptrs2[2]; // expected-warning {{Potential memory leak}}
+ ptrs2[0] = (int *)malloc(4);
+ memcpy(&ptrs2[1], &null, sizeof(int *));
+
+ int *ptrs3[2]; // expected-warning {{Potential memory leak}}
+ ptrs3[0] = (int *)malloc(4);
+ memcpy(&ptrs3[0], &null, sizeof(int *));
+} // expected-warning {{Potential memory leak}}
+
// Rely on the CString checker evaluation of the strcpy API to convey that the result of strcpy is equal to p.
void symbolLostWithStrcpy(char *s) {
char *p = malloc(12);
More information about the cfe-commits
mailing list