diff options
author | Jordan Rose <jordan_rose@apple.com> | 2013-08-19 16:27:34 +0000 |
---|---|---|
committer | Jordan Rose <jordan_rose@apple.com> | 2013-08-19 16:27:34 +0000 |
commit | a728e927c6e58f26b2c8615a8baa761d2f157e4b (patch) | |
tree | 4aaf8bbf2d4b42bfdc26771b6c30e14cff37fd12 | |
parent | 7d0dcd2de023e2667a3f1f14daff9d087fab9bf7 (diff) |
[analyzer] Assume that strings are no longer than SIZE_MAX/4.
This keeps the analyzer from making silly assumptions, like thinking
strlen(foo)+1 could wrap around to 0. This fixes PR16558.
Patch by Karthik Bhat!
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@188680 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 17 | ||||
-rw-r--r-- | test/Analysis/malloc.c | 32 | ||||
-rw-r--r-- | test/Analysis/string.c | 6 | ||||
-rw-r--r-- | www/analyzer/open_projects.html | 5 |
4 files changed, 51 insertions, 9 deletions
diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 932f6316b5..ba1d9b9ff6 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -661,7 +661,7 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C, if (Recorded) return *Recorded; } - + // Otherwise, get a new symbol and update the state. SValBuilder &svalBuilder = C.getSValBuilder(); QualType sizeTy = svalBuilder.getContext().getSizeType(); @@ -669,8 +669,21 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C, MR, Ex, sizeTy, C.blockCount()); - if (!hypothetical) + if (!hypothetical) { + if (Optional<NonLoc> strLn = strLength.getAs<NonLoc>()) { + // In case of unbounded calls strlen etc bound the range to SIZE_MAX/4 + BasicValueFactory &BVF = svalBuilder.getBasicValueFactory(); + const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy); + llvm::APSInt fourInt = APSIntType(maxValInt).getValue(4); + const llvm::APSInt *maxLengthInt = BVF.evalAPSInt(BO_Div, maxValInt, + fourInt); + NonLoc maxLength = svalBuilder.makeIntVal(*maxLengthInt); + SVal evalLength = svalBuilder.evalBinOpNN(state, BO_LE, *strLn, + maxLength, sizeTy); + state = state->assume(evalLength.castAs<DefinedOrUnknownSVal>(), true); + } state = state->set<CStringLength>(MR, strLength); + } return strLength; } diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index 83946c83a8..2e5213e374 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -1216,6 +1216,38 @@ void testReallocEscaped(void **memory) { } } +// PR16558 +void *smallocNoWarn(size_t size) { + if (size == 0) { + return malloc(1); // this branch is never called + } + else { + return malloc(size); + } +} + +char *dupstrNoWarn(const char *s) { + const int len = strlen(s); + char *p = (char*) smallocNoWarn(len + 1); + strcpy(p, s); // no-warning + return p; +} + +void *smallocWarn(size_t size) { + if (size == 2) { + return malloc(1); + } + else { + return malloc(size); + } +} + +char *dupstrWarn(const char *s) { + const int len = strlen(s); + char *p = (char*) smallocWarn(len + 1); + strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}} + return p; +} // ---------------------------------------------------------------------------- // False negatives. diff --git a/test/Analysis/string.c b/test/Analysis/string.c index 6cf52f7a55..9fd3efb5c2 100644 --- a/test/Analysis/string.c +++ b/test/Analysis/string.c @@ -430,11 +430,12 @@ void strcat_unknown_src_length(char *src, int offset) { // length for the "before" strlen, we won't be able to set one for "after". void strcat_too_big(char *dst, char *src) { + // We assume this can never actually happen, so we don't get a warning. if (strlen(dst) != (((size_t)0) - 2)) return; if (strlen(src) != 2) return; - strcat(dst, src); // expected-warning{{This expression will create a string whose length is too big to be represented as a size_t}} + strcat(dst, src); } @@ -653,11 +654,12 @@ void strncat_unknown_limit(float limit) { } void strncat_too_big(char *dst, char *src) { + // We assume this will never actually happen, so we don't get a warning. if (strlen(dst) != (((size_t)0) - 2)) return; if (strlen(src) != 2) return; - strncat(dst, src, 2); // expected-warning{{This expression will create a string whose length is too big to be represented as a size_t}} + strncat(dst, src, 2); } void strncat_zero(char *src) { diff --git a/www/analyzer/open_projects.html b/www/analyzer/open_projects.html index a5f5538662..4a888adbb6 100644 --- a/www/analyzer/open_projects.html +++ b/www/analyzer/open_projects.html @@ -174,11 +174,6 @@ mailing list</a> to notify other members of the community.</p> <i>(Difficulty: Easy)</i></p> </li> - <li>Teach CStringChecker that strings are never longer than, say, <code>SIZE_MAX/4</code> characters.</li> - <p>Though most of CStringChecker's functionality is disabled (due to poor diagnostics for error edge cases), it's still used to model certain operations like <code>strlen</code>, which should give the same result each time it's called on a string. However, assuming that the string length is an arbitrary symbolic value can give strange results -- for example, <code>strlen(str)+1</code> could wrap around to 0. (This is the root cause of <a href="http://llvm.org/bugs/show_bug.cgi?id=16558">PR16558</a>.) In practice, strings are never that long, so picking some large upper bound and recording that in the state would make plenty of sense, and would fix these false positives. - <i>(Difficulty: Easy)</i></p> - </li> - <li>Implement iterators invalidation checker. <p><i>(Difficulty: Easy)</i></p> </li> |