[cfe-commits] r133408 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CStringChecker.cpp test/Analysis/string.c
Jordy Rose
jediknil at belkadan.com
Sun Jun 19 20:49:16 PDT 2011
Author: jrose
Date: Sun Jun 19 22:49:16 2011
New Revision: 133408
URL: http://llvm.org/viewvc/llvm-project?rev=133408&view=rev
Log:
[analyzer] Re-enable checking for strncpy, along with a new validation of the size argument. strncat is not yet up-to-date, but I'm leaving it enabled for now (there shouldn't be any false positives, at least...)
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/string.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=133408&r1=133407&r2=133408&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Sun Jun 19 22:49:16 2011
@@ -123,12 +123,15 @@
const Expr *FirstBuf,
const Expr *SecondBuf,
const char *firstMessage = NULL,
- const char *secondMessage = NULL) const;
+ const char *secondMessage = NULL,
+ bool WarnAboutSize = false) const;
const GRState *CheckBufferAccess(CheckerContext &C, const GRState *state,
const Expr *Size, const Expr *Buf,
- const char *message = NULL) const {
+ const char *message = NULL,
+ bool WarnAboutSize = false) const {
// This is a convenience override.
- return CheckBufferAccess(C, state, Size, Buf, NULL, message, NULL);
+ return CheckBufferAccess(C, state, Size, Buf, NULL, message, NULL,
+ WarnAboutSize);
}
const GRState *CheckOverlap(CheckerContext &C, const GRState *state,
const Expr *Size, const Expr *First,
@@ -290,7 +293,8 @@
const Expr *FirstBuf,
const Expr *SecondBuf,
const char *firstMessage,
- const char *secondMessage) const {
+ const char *secondMessage,
+ bool WarnAboutSize) const {
// If a previous check has failed, propagate the failure.
if (!state)
return NULL;
@@ -323,9 +327,11 @@
// Check that the first buffer is sufficiently long.
SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
if (Loc *BufLoc = dyn_cast<Loc>(&BufStart)) {
+ const Expr *warningExpr = (WarnAboutSize ? Size : FirstBuf);
+
SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc,
LastOffset, PtrTy);
- state = CheckLocation(C, state, FirstBuf, BufEnd, firstMessage);
+ state = CheckLocation(C, state, warningExpr, BufEnd, firstMessage);
// If the buffer isn't large enough, abort.
if (!state)
@@ -341,9 +347,11 @@
BufStart = svalBuilder.evalCast(BufVal, PtrTy, SecondBuf->getType());
if (Loc *BufLoc = dyn_cast<Loc>(&BufStart)) {
+ const Expr *warningExpr = (WarnAboutSize ? Size : SecondBuf);
+
SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc,
LastOffset, PtrTy);
- state = CheckLocation(C, state, SecondBuf, BufEnd, secondMessage);
+ state = CheckLocation(C, state, warningExpr, BufEnd, secondMessage);
}
}
@@ -1251,6 +1259,18 @@
}
}
+ // We still want to know if the bound is known to be too large.
+ const char * const warningMsg =
+ "Size argument is greater than the length of the destination buffer";
+ state = CheckBufferAccess(C, state, lenExpr, Dst, warningMsg,
+ /* WarnAboutSize = */ true);
+ // FIXME: It'd be nice for this not to be a hard error, so we can warn
+ // about more than one thing, but the multiple calls to CheckLocation
+ // result in the same ExplodedNode, which means we don't keep emitting
+ // bug reports.
+ if (!state)
+ return;
+
// If we couldn't pin down the copy length, at least bound it.
if (amountCopied.isUnknown()) {
// Try to get a "hypothetical" string length symbol, which we can later
@@ -1399,8 +1419,11 @@
// string, but that's still an improvement over blank invalidation.
state = InvalidateBuffer(C, state, Dst, *dstRegVal);
- // Set the C string length of the destination.
- state = setCStringLength(state, dstRegVal->getRegion(), finalStrLength);
+ // Set the C string length of the destination, if we know it.
+ if (!isBounded || (amountCopied == strLength))
+ state = setCStringLength(state, dstRegVal->getRegion(), finalStrLength);
+ else
+ state = setCStringLength(state, dstRegVal->getRegion(), UnknownVal());
}
assert(state);
@@ -1589,7 +1612,7 @@
.Cases("memcmp", "bcmp", &CStringChecker::evalMemcmp)
.Cases("memmove", "__memmove_chk", &CStringChecker::evalMemmove)
.Cases("strcpy", "__strcpy_chk", &CStringChecker::evalStrcpy)
- //.Cases("strncpy", "__strncpy_chk", &CStringChecker::evalStrncpy)
+ .Cases("strncpy", "__strncpy_chk", &CStringChecker::evalStrncpy)
.Cases("stpcpy", "__stpcpy_chk", &CStringChecker::evalStpcpy)
.Cases("strcat", "__strcat_chk", &CStringChecker::evalStrcat)
.Cases("strncat", "__strncat_chk", &CStringChecker::evalStrncat)
Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=133408&r1=133407&r2=133408&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Sun Jun 19 22:49:16 2011
@@ -457,6 +457,105 @@
//===----------------------------------------------------------------------===
+// strncpy()
+//===----------------------------------------------------------------------===
+
+#ifdef VARIANT
+
+#define __strncpy_chk BUILTIN(__strncpy_chk)
+char *__strncpy_chk(char *restrict s1, const char *restrict s2, size_t n, size_t destlen);
+
+#define strncpy(a,b,n) __strncpy_chk(a,b,n,(size_t)-1)
+
+#else /* VARIANT */
+
+#define strncpy BUILTIN(strncpy)
+char *strncpy(char *restrict s1, const char *restrict s2, size_t n);
+
+#endif /* VARIANT */
+
+
+void strncpy_null_dst(char *x) {
+ strncpy(NULL, x, 5); // expected-warning{{Null pointer argument in call to string copy function}}
+}
+
+void strncpy_null_src(char *x) {
+ strncpy(x, NULL, 5); // expected-warning{{Null pointer argument in call to string copy function}}
+}
+
+void strncpy_fn(char *x) {
+ strncpy(x, (char*)&strcpy_fn, 5); // expected-warning{{Argument to string copy function is the address of the function 'strcpy_fn', which is not a null-terminated string}}
+}
+
+void strncpy_effects(char *x, char *y) {
+ char a = x[0];
+
+ if (strncpy(x, y, 5) != x)
+ (void)*(char*)0; // no-warning
+
+ if (strlen(x) != strlen(y))
+ (void)*(char*)0; // expected-warning{{null}}
+
+ if (a != x[0])
+ (void)*(char*)0; // expected-warning{{null}}
+}
+
+void strncpy_overflow(char *y) {
+ char x[4];
+ if (strlen(y) == 4)
+ strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void strncpy_no_overflow(char *y) {
+ char x[4];
+ if (strlen(y) == 3)
+ strncpy(x, y, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void strncpy_no_overflow2(char *y, int n) {
+ if (n <= 4)
+ return;
+
+ char x[4];
+ if (strlen(y) == 3)
+ strncpy(x, y, n); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void strncpy_truncate(char *y) {
+ char x[4];
+ if (strlen(y) == 4)
+ strncpy(x, y, 3); // no-warning
+}
+
+void strncpy_no_truncate(char *y) {
+ char x[4];
+ if (strlen(y) == 3)
+ strncpy(x, y, 3); // no-warning
+}
+
+void strncpy_exactly_matching_buffer(char *y) {
+ char x[4];
+ strncpy(x, y, 4); // no-warning
+
+ // strncpy does not null-terminate, so we have no idea what the strlen is
+ // after this.
+ if (strlen(x) > 4)
+ (void)*(int*)0; // expected-warning{{null}}
+}
+
+void strncpy_exactly_matching_buffer2(char *y) {
+ if (strlen(y) >= 4)
+ return;
+
+ char x[4];
+ strncpy(x, y, 4); // no-warning
+
+ // This time, we know that y fits in x anyway.
+ if (strlen(x) > 3)
+ (void)*(int*)0; // no-warning
+}
+
+//===----------------------------------------------------------------------===
// strncat()
//===----------------------------------------------------------------------===
More information about the cfe-commits
mailing list