From 23d62c03a7d342acec33d43091e70d1b60ec5b0a Mon Sep 17 00:00:00 2001 From: Anton Yartsev Date: Thu, 19 May 2016 23:03:49 +0000 Subject: [PATCH] [analyzer] Fix for PR23790 : constrain return value of strcmp() rather than returning a concrete value. The function strcmp() can return any value, not just {-1,0,1} : "The strcmp(const char *s1, const char *s2) function returns an integer greater than, equal to, or less than zero, accordingly as the string pointed to by s1 is greater than, equal to, or less than the string pointed to by s2." [C11 7.24.4.2p3] https://llvm.org/bugs/show_bug.cgi?id=23790 http://reviews.llvm.org/D16317 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@270154 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/CStringChecker.cpp | 37 ++++--- test/Analysis/string.c | 104 +++++++++++++----- 2 files changed, 96 insertions(+), 45 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index ff4922dac9..e9512977fa 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -1837,6 +1837,8 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val); const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val); bool canComputeResult = false; + SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, + C.blockCount()); if (s1StrLiteral && s2StrLiteral) { StringRef s1StrRef = s1StrLiteral->getString(); @@ -1870,28 +1872,29 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, s2StrRef = s2StrRef.substr(0, s2Term); // Use StringRef's comparison methods to compute the actual result. - int result; + int compareRes = ignoreCase ? s1StrRef.compare_lower(s2StrRef) + : s1StrRef.compare(s2StrRef); - if (ignoreCase) { - // Compare string 1 to string 2 the same way strcasecmp() does. - result = s1StrRef.compare_lower(s2StrRef); - } else { - // Compare string 1 to string 2 the same way strcmp() does. - result = s1StrRef.compare(s2StrRef); + // The strcmp function returns an integer greater than, equal to, or less + // than zero, [c11, p7.24.4.2]. + if (compareRes == 0) { + resultVal = svalBuilder.makeIntVal(compareRes, CE->getType()); + } + else { + DefinedSVal zeroVal = svalBuilder.makeIntVal(0, CE->getType()); + // Constrain strcmp's result range based on the result of StringRef's + // comparison methods. + BinaryOperatorKind op = (compareRes == 1) ? BO_GT : BO_LT; + SVal compareWithZero = + svalBuilder.evalBinOp(state, op, resultVal, zeroVal, + svalBuilder.getConditionType()); + DefinedSVal compareWithZeroVal = compareWithZero.castAs(); + state = state->assume(compareWithZeroVal, true); } - - // Build the SVal of the comparison and bind the return value. - SVal resultVal = svalBuilder.makeIntVal(result, CE->getType()); - state = state->BindExpr(CE, LCtx, resultVal); } } - if (!canComputeResult) { - // Conjure a symbolic value. It's the best we can do. - SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, - C.blockCount()); - state = state->BindExpr(CE, LCtx, resultVal); - } + state = state->BindExpr(CE, LCtx, resultVal); // Record this as a possible path. C.addTransition(state); diff --git a/test/Analysis/string.c b/test/Analysis/string.c index c65d2be1a4..2803362ba4 100644 --- a/test/Analysis/string.c +++ b/test/Analysis/string.c @@ -680,6 +680,18 @@ void strncat_empty() { #define strcmp BUILTIN(strcmp) int strcmp(const char * s1, const char * s2); +void strcmp_check_modelling() { + char *x = "aa"; + char *y = "a"; + clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) <= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strcmp(x, y) > 1); // expected-warning{{UNKNOWN}} + + clang_analyzer_eval(strcmp(y, x) < 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(y, x) >= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strcmp(y, x) < -1); // expected-warning{{UNKNOWN}} +} + void strcmp_constant0() { clang_analyzer_eval(strcmp("123", "123") == 0); // expected-warning{{TRUE}} } @@ -703,13 +715,13 @@ void strcmp_0() { void strcmp_1() { char *x = "234"; char *y = "123"; - clang_analyzer_eval(strcmp(x, y) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}} } void strcmp_2() { char *x = "123"; char *y = "234"; - clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}} } void strcmp_null_0() { @@ -727,25 +739,25 @@ void strcmp_null_1() { void strcmp_diff_length_0() { char *x = "12345"; char *y = "234"; - clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}} } void strcmp_diff_length_1() { char *x = "123"; char *y = "23456"; - clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}} } void strcmp_diff_length_2() { char *x = "12345"; char *y = "123"; - clang_analyzer_eval(strcmp(x, y) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}} } void strcmp_diff_length_3() { char *x = "123"; char *y = "12345"; - clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}} } void strcmp_embedded_null () { @@ -777,6 +789,18 @@ void strcmp_union_function_pointer_cast(union argument a) { #define strncmp BUILTIN(strncmp) int strncmp(const char *s1, const char *s2, size_t n); +void strncmp_check_modelling() { + char *x = "aa"; + char *y = "a"; + clang_analyzer_eval(strncmp(x, y, 2) > 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 2) <= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strncmp(x, y, 2) > 1); // expected-warning{{UNKNOWN}} + + clang_analyzer_eval(strncmp(y, x, 2) < 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(y, x, 2) >= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strncmp(y, x, 2) < -1); // expected-warning{{UNKNOWN}} +} + void strncmp_constant0() { clang_analyzer_eval(strncmp("123", "123", 3) == 0); // expected-warning{{TRUE}} } @@ -800,13 +824,13 @@ void strncmp_0() { void strncmp_1() { char *x = "234"; char *y = "123"; - clang_analyzer_eval(strncmp(x, y, 3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 3) > 0); // expected-warning{{TRUE}} } void strncmp_2() { char *x = "123"; char *y = "234"; - clang_analyzer_eval(strncmp(x, y, 3) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 3) < 0); // expected-warning{{TRUE}} } void strncmp_null_0() { @@ -824,25 +848,25 @@ void strncmp_null_1() { void strncmp_diff_length_0() { char *x = "12345"; char *y = "234"; - clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}} } void strncmp_diff_length_1() { char *x = "123"; char *y = "23456"; - clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}} } void strncmp_diff_length_2() { char *x = "12345"; char *y = "123"; - clang_analyzer_eval(strncmp(x, y, 5) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 5) > 0); // expected-warning{{TRUE}} } void strncmp_diff_length_3() { char *x = "123"; char *y = "12345"; - clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}} } void strncmp_diff_length_4() { @@ -854,13 +878,13 @@ void strncmp_diff_length_4() { void strncmp_diff_length_5() { char *x = "012"; char *y = "12345"; - clang_analyzer_eval(strncmp(x, y, 3) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 3) < 0); // expected-warning{{TRUE}} } void strncmp_diff_length_6() { char *x = "234"; char *y = "12345"; - clang_analyzer_eval(strncmp(x, y, 3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncmp(x, y, 3) > 0); // expected-warning{{TRUE}} } void strncmp_embedded_null () { @@ -874,6 +898,18 @@ void strncmp_embedded_null () { #define strcasecmp BUILTIN(strcasecmp) int strcasecmp(const char *s1, const char *s2); +void strcasecmp_check_modelling() { + char *x = "aa"; + char *y = "a"; + clang_analyzer_eval(strcasecmp(x, y) > 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) <= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strcasecmp(x, y) > 1); // expected-warning{{UNKNOWN}} + + clang_analyzer_eval(strcasecmp(y, x) < 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(y, x) >= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strcasecmp(y, x) < -1); // expected-warning{{UNKNOWN}} +} + void strcasecmp_constant0() { clang_analyzer_eval(strcasecmp("abc", "Abc") == 0); // expected-warning{{TRUE}} } @@ -897,13 +933,13 @@ void strcasecmp_0() { void strcasecmp_1() { char *x = "Bcd"; char *y = "abc"; - clang_analyzer_eval(strcasecmp(x, y) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) > 0); // expected-warning{{TRUE}} } void strcasecmp_2() { char *x = "abc"; char *y = "Bcd"; - clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}} } void strcasecmp_null_0() { @@ -921,25 +957,25 @@ void strcasecmp_null_1() { void strcasecmp_diff_length_0() { char *x = "abcde"; char *y = "aBd"; - clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}} } void strcasecmp_diff_length_1() { char *x = "abc"; char *y = "aBdef"; - clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}} } void strcasecmp_diff_length_2() { char *x = "aBcDe"; char *y = "abc"; - clang_analyzer_eval(strcasecmp(x, y) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) > 0); // expected-warning{{TRUE}} } void strcasecmp_diff_length_3() { char *x = "aBc"; char *y = "abcde"; - clang_analyzer_eval(strcasecmp(x, y) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strcasecmp(x, y) < 0); // expected-warning{{TRUE}} } void strcasecmp_embedded_null () { @@ -953,6 +989,18 @@ void strcasecmp_embedded_null () { #define strncasecmp BUILTIN(strncasecmp) int strncasecmp(const char *s1, const char *s2, size_t n); +void strncasecmp_check_modelling() { + char *x = "aa"; + char *y = "a"; + clang_analyzer_eval(strncasecmp(x, y, 2) > 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 2) <= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strncasecmp(x, y, 2) > 1); // expected-warning{{UNKNOWN}} + + clang_analyzer_eval(strncasecmp(y, x, 2) < 0); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(y, x, 2) >= 0); // expected-warning{{FALSE}} + clang_analyzer_eval(strncasecmp(y, x, 2) < -1); // expected-warning{{UNKNOWN}} +} + void strncasecmp_constant0() { clang_analyzer_eval(strncasecmp("abc", "Abc", 3) == 0); // expected-warning{{TRUE}} } @@ -976,13 +1024,13 @@ void strncasecmp_0() { void strncasecmp_1() { char *x = "Bcd"; char *y = "abc"; - clang_analyzer_eval(strncasecmp(x, y, 3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 3) > 0); // expected-warning{{TRUE}} } void strncasecmp_2() { char *x = "abc"; char *y = "Bcd"; - clang_analyzer_eval(strncasecmp(x, y, 3) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 3) < 0); // expected-warning{{TRUE}} } void strncasecmp_null_0() { @@ -1000,25 +1048,25 @@ void strncasecmp_null_1() { void strncasecmp_diff_length_0() { char *x = "abcde"; char *y = "aBd"; - clang_analyzer_eval(strncasecmp(x, y, 5) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 5) < 0); // expected-warning{{TRUE}} } void strncasecmp_diff_length_1() { char *x = "abc"; char *y = "aBdef"; - clang_analyzer_eval(strncasecmp(x, y, 5) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 5) < 0); // expected-warning{{TRUE}} } void strncasecmp_diff_length_2() { char *x = "aBcDe"; char *y = "abc"; - clang_analyzer_eval(strncasecmp(x, y, 5) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 5) > 0); // expected-warning{{TRUE}} } void strncasecmp_diff_length_3() { char *x = "aBc"; char *y = "abcde"; - clang_analyzer_eval(strncasecmp(x, y, 5) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 5) < 0); // expected-warning{{TRUE}} } void strncasecmp_diff_length_4() { @@ -1030,13 +1078,13 @@ void strncasecmp_diff_length_4() { void strncasecmp_diff_length_5() { char *x = "abcde"; char *y = "aBd"; - clang_analyzer_eval(strncasecmp(x, y, 3) == -1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 3) < 0); // expected-warning{{TRUE}} } void strncasecmp_diff_length_6() { char *x = "aBDe"; char *y = "abc"; - clang_analyzer_eval(strncasecmp(x, y, 3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(strncasecmp(x, y, 3) > 0); // expected-warning{{TRUE}} } void strncasecmp_embedded_null () { -- 2.40.0