[cfe-commits] r132955 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CStringChecker.cpp test/Analysis/string-fail.c test/Analysis/string.c
Jordy Rose
jediknil at belkadan.com
Mon Jun 13 18:15:32 PDT 2011
Author: jrose
Date: Mon Jun 13 20:15:31 2011
New Revision: 132955
URL: http://llvm.org/viewvc/llvm-project?rev=132955&view=rev
Log:
[analyzer] Fix modeling of strnlen to be more conservative. Move tests we can't properly model (yet?) to string-fail.c.
Added:
cfe/trunk/test/Analysis/string-fail.c
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=132955&r1=132954&r2=132955&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Mon Jun 13 20:15:31 2011
@@ -903,10 +903,35 @@
void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
bool IsStrnlen) const {
const GRState *state = C.getState();
+
+ if (IsStrnlen) {
+ const Expr *maxlenExpr = CE->getArg(1);
+ SVal maxlenVal = state->getSVal(maxlenExpr);
+
+ const GRState *stateZeroSize, *stateNonZeroSize;
+ llvm::tie(stateZeroSize, stateNonZeroSize) =
+ assumeZero(C, state, maxlenVal, maxlenExpr->getType());
+
+ // If the size can be zero, the result will be 0 in that case, and we don't
+ // have to check the string itself.
+ if (stateZeroSize) {
+ SVal zero = C.getSValBuilder().makeZeroVal(CE->getType());
+ stateZeroSize = stateZeroSize->BindExpr(CE, zero);
+ C.addTransition(stateZeroSize);
+ }
+
+ // If the size is GUARANTEED to be zero, we're done!
+ if (!stateNonZeroSize)
+ return;
+
+ // Otherwise, record the assumption that the size is nonzero.
+ state = stateNonZeroSize;
+ }
+
+ // Check that the string argument is non-null.
const Expr *Arg = CE->getArg(0);
SVal ArgVal = state->getSVal(Arg);
- // Check that the argument is non-null.
state = checkNonNull(C, state, Arg, ArgVal);
if (state) {
@@ -917,41 +942,82 @@
if (strLength.isUndef())
return;
+ DefinedOrUnknownSVal result = UnknownVal();
+
// If the check is for strnlen() then bind the return value to no more than
// the maxlen value.
if (IsStrnlen) {
+ QualType cmpTy = C.getSValBuilder().getContext().IntTy;
+
+ // It's a little unfortunate to be getting this again,
+ // but it's not that expensive...
const Expr *maxlenExpr = CE->getArg(1);
SVal maxlenVal = state->getSVal(maxlenExpr);
-
+
NonLoc *strLengthNL = dyn_cast<NonLoc>(&strLength);
NonLoc *maxlenValNL = dyn_cast<NonLoc>(&maxlenVal);
- QualType cmpTy = C.getSValBuilder().getContext().IntTy;
- const GRState *stateTrue, *stateFalse;
-
- // Check if the strLength is greater than or equal to the maxlen
- llvm::tie(stateTrue, stateFalse) =
- state->assume(cast<DefinedOrUnknownSVal>
- (C.getSValBuilder().evalBinOpNN(state, BO_GE,
- *strLengthNL, *maxlenValNL,
- cmpTy)));
-
- // If the strLength is greater than or equal to the maxlen, set strLength
- // to maxlen
- if (stateTrue && !stateFalse) {
- strLength = maxlenVal;
+ if (strLengthNL && maxlenValNL) {
+ const GRState *stateStringTooLong, *stateStringNotTooLong;
+
+ // Check if the strLength is greater than the maxlen.
+ llvm::tie(stateStringTooLong, stateStringNotTooLong) =
+ state->assume(cast<DefinedOrUnknownSVal>
+ (C.getSValBuilder().evalBinOpNN(state, BO_GT,
+ *strLengthNL,
+ *maxlenValNL,
+ cmpTy)));
+
+ if (stateStringTooLong && !stateStringNotTooLong) {
+ // If the string is longer than maxlen, return maxlen.
+ result = *maxlenValNL;
+ } else if (stateStringNotTooLong && !stateStringTooLong) {
+ // If the string is shorter than maxlen, return its length.
+ result = *strLengthNL;
+ }
+ }
+
+ if (result.isUnknown()) {
+ // If we don't have enough information for a comparison, there's
+ // no guarantee the full string length will actually be returned.
+ // All we know is the return value is the min of the string length
+ // and the limit. This is better than nothing.
+ unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
+ result = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+ NonLoc *resultNL = cast<NonLoc>(&result);
+
+ if (strLengthNL) {
+ state = state->assume(cast<DefinedOrUnknownSVal>
+ (C.getSValBuilder().evalBinOpNN(state, BO_LE,
+ *resultNL,
+ *strLengthNL,
+ cmpTy)), true);
+ }
+
+ if (maxlenValNL) {
+ state = state->assume(cast<DefinedOrUnknownSVal>
+ (C.getSValBuilder().evalBinOpNN(state, BO_LE,
+ *resultNL,
+ *maxlenValNL,
+ cmpTy)), true);
+ }
}
- }
- // If getCStringLength couldn't figure out the length, conjure a return
- // value, so it can be used in constraints, at least.
- if (strLength.isUnknown()) {
- unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
- strLength = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+ } else {
+ // This is a plain strlen(), not strnlen().
+ result = cast<DefinedOrUnknownSVal>(strLength);
+
+ // If we don't know the length of the string, conjure a return
+ // value, so it can be used in constraints, at least.
+ if (result.isUnknown()) {
+ unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
+ result = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+ }
}
// Bind the return value.
- state = state->BindExpr(CE, strLength);
+ assert(!result.isUnknown() && "Should have conjured a value by now");
+ state = state->BindExpr(CE, result);
C.addTransition(state);
}
}
Added: cfe/trunk/test/Analysis/string-fail.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string-fail.c?rev=132955&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/string-fail.c (added)
+++ cfe/trunk/test/Analysis/string-fail.c Mon Jun 13 20:15:31 2011
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,cplusplus.experimental.CString,deadcode.experimental.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,cplusplus.experimental.CString,deadcode.experimental.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s
+// XFAIL: *
+
+// This file is for tests that may eventually go into string.c, or may be
+// deleted outright. At one point these tests passed, but only because we
+// weren't correctly modelling the behavior of the relevant string functions.
+// The tests aren't incorrect, but require the analyzer to be smarter about
+// conjured values than it currently is.
+
+//===----------------------------------------------------------------------===
+// Declarations
+//===----------------------------------------------------------------------===
+
+// Some functions are so similar to each other that they follow the same code
+// path, such as memcpy and __memcpy_chk, or memcmp and bcmp. If VARIANT is
+// defined, make sure to use the variants instead to make sure they are still
+// checked by the analyzer.
+
+// Some functions are implemented as builtins. These should be #defined as
+// BUILTIN(f), which will prepend "__builtin_" if USE_BUILTINS is defined.
+
+// Functions that have variants and are also available as builtins should be
+// declared carefully! See memcpy() for an example.
+
+#ifdef USE_BUILTINS
+# define BUILTIN(f) __builtin_ ## f
+#else /* USE_BUILTINS */
+# define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+#define NULL 0
+typedef typeof(sizeof(int)) size_t;
+
+
+//===----------------------------------------------------------------------===
+// strnlen()
+//===----------------------------------------------------------------------===
+
+#define strnlen BUILTIN(strnlen)
+size_t strnlen(const char *s, size_t maxlen);
+
+void strnlen_liveness(const char *x) {
+ if (strnlen(x, 10) < 5)
+ return;
+ if (strnlen(x, 10) < 5)
+ (void)*(char*)0; // no-warning
+}
+
+void strnlen_subregion() {
+ struct two_stringsn { char a[2], b[2]; };
+ extern void use_two_stringsn(struct two_stringsn *);
+
+ struct two_stringsn z;
+ use_two_stringsn(&z);
+
+ size_t a = strnlen(z.a, 10);
+ z.b[0] = 5;
+ size_t b = strnlen(z.a, 10);
+ if (a == 0 && b != 0)
+ (void)*(char*)0; // expected-warning{{never executed}}
+
+ use_two_stringsn(&z);
+
+ size_t c = strnlen(z.a, 10);
+ if (a == 0 && c != 0)
+ (void)*(char*)0; // expected-warning{{null}}
+}
+
+extern void use_stringn(char *);
+void strnlen_argument(char *x) {
+ size_t a = strnlen(x, 10);
+ size_t b = strnlen(x, 10);
+ if (a == 0 && b != 0)
+ (void)*(char*)0; // expected-warning{{never executed}}
+
+ use_stringn(x);
+
+ size_t c = strnlen(x, 10);
+ if (a == 0 && c != 0)
+ (void)*(char*)0; // expected-warning{{null}}
+}
+
+extern char global_strn[];
+void strnlen_global() {
+ size_t a = strnlen(global_strn, 10);
+ size_t b = strnlen(global_strn, 10);
+ if (a == 0 && b != 0)
+ (void)*(char*)0; // expected-warning{{never executed}}
+
+ // Call a function with unknown effects, which should invalidate globals.
+ use_stringn(0);
+
+ size_t c = strnlen(global_strn, 10);
+ if (a == 0 && c != 0)
+ (void)*(char*)0; // expected-warning{{null}}
+}
+
+void strnlen_indirect(char *x) {
+ size_t a = strnlen(x, 10);
+ char *p = x;
+ char **p2 = &p;
+ size_t b = strnlen(x, 10);
+ if (a == 0 && b != 0)
+ (void)*(char*)0; // expected-warning{{never executed}}
+
+ extern void use_stringn_ptr(char*const*);
+ use_stringn_ptr(p2);
+
+ size_t c = strnlen(x, 10);
+ if (a == 0 && c != 0)
+ (void)*(char*)0; // expected-warning{{null}}
+}
Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=132955&r1=132954&r2=132955&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Mon Jun 13 20:15:31 2011
@@ -199,76 +199,59 @@
return strnlen((char*)&&label, 3); // expected-warning{{Argument to byte string function is the address of the label 'label', which is not a null-terminated string}}
}
-void strnlen_subregion() {
- struct two_stringsn { char a[2], b[2]; };
- extern void use_two_stringsn(struct two_stringsn *);
-
- struct two_stringsn z;
- use_two_stringsn(&z);
-
- size_t a = strnlen(z.a, 10);
- z.b[0] = 5;
- size_t b = strnlen(z.a, 10);
- if (a == 0 && b != 0)
- (void)*(char*)0; // expected-warning{{never executed}}
+void strnlen_zero() {
+ if (strnlen("abc", 0) != 0)
+ (void)*(char*)0; // no-warning
+ if (strnlen(NULL, 0) != 0) // no-warning
+ (void)*(char*)0; // no-warning
+}
+
+size_t strnlen_compound_literal() {
+ // This used to crash because we don't model the string lengths of
+ // compound literals.
+ return strnlen((char[]) { 'a', 'b', 0 }, 1);
+}
- use_two_stringsn(&z);
+size_t strnlen_unknown_limit(float f) {
+ // This used to crash because we don't model the integer values of floats.
+ return strnlen("abc", (int)f);
+}
- size_t c = strnlen(z.a, 10);
- if (a == 0 && c != 0)
+void strnlen_is_not_strlen(char *x) {
+ if (strnlen(x, 10) != strlen(x))
(void)*(char*)0; // expected-warning{{null}}
}
-extern void use_stringn(char *);
-void strnlen_argument(char *x) {
- size_t a = strnlen(x, 10);
- size_t b = strnlen(x, 10);
- if (a == 0 && b != 0)
+void strnlen_at_limit(char *x) {
+ size_t len = strnlen(x, 10);
+ if (len > 10)
(void)*(char*)0; // expected-warning{{never executed}}
-
- use_stringn(x);
-
- size_t c = strnlen(x, 10);
- if (a == 0 && c != 0)
- (void)*(char*)0; // expected-warning{{null}}
+ if (len == 10)
+ (void)*(char*)0; // expected-warning{{null}}
}
-extern char global_strn[];
-void strnlen_global() {
- size_t a = strnlen(global_strn, 10);
- size_t b = strnlen(global_strn, 10);
- if (a == 0 && b != 0)
+void strnlen_less_than_limit(char *x) {
+ size_t len = strnlen(x, 10);
+ if (len > 10)
(void)*(char*)0; // expected-warning{{never executed}}
-
- // Call a function with unknown effects, which should invalidate globals.
- use_stringn(0);
-
- size_t c = strnlen(global_str, 10);
- if (a == 0 && c != 0)
- (void)*(char*)0; // expected-warning{{null}}
+ if (len < 10)
+ (void)*(char*)0; // expected-warning{{null}}
}
-void strnlen_indirect(char *x) {
- size_t a = strnlen(x, 10);
- char *p = x;
- char **p2 = &p;
- size_t b = strnlen(x, 10);
- if (a == 0 && b != 0)
+void strnlen_at_actual(size_t limit) {
+ size_t len = strnlen("abc", limit);
+ if (len > 3)
(void)*(char*)0; // expected-warning{{never executed}}
-
- extern void use_stringn_ptr(char*const*);
- use_stringn_ptr(p2);
-
- size_t c = strnlen(x, 10);
- if (a == 0 && c != 0)
+ if (len == 3)
(void)*(char*)0; // expected-warning{{null}}
}
-void strnlen_liveness(const char *x) {
- if (strnlen(x, 10) < 5)
- return;
- if (strnlen(x, 10) < 5)
- (void)*(char*)0; // no-warning
+void strnlen_less_than_actual(size_t limit) {
+ size_t len = strnlen("abc", limit);
+ if (len > 3)
+ (void)*(char*)0; // expected-warning{{never executed}}
+ if (len < 3)
+ (void)*(char*)0; // expected-warning{{null}}
}
//===----------------------------------------------------------------------===
More information about the cfe-commits
mailing list