From bc34f0239ddd1044f6aa90ad68b4ad3858b075cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 18 Dec 2018 14:36:49 +0100 Subject: [PATCH] Disable the subfunction value flow analysis. It does not work well and needs to be rewritten. There are false positives. --- lib/valueflow.cpp | 3 ++- test/testbufferoverrun.cpp | 4 ++-- test/testnullpointer.cpp | 17 ++++++++--------- test/testother.cpp | 2 +- test/teststring.cpp | 2 +- test/testtype.cpp | 4 ++-- test/testvalueflow.cpp | 29 ++++++++--------------------- 7 files changed, 24 insertions(+), 37 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index cbd724915..20d3898c8 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4152,7 +4152,8 @@ static void valueFlowSubFunction(TokenList *tokenlist, ErrorLogger *errorLogger, // passed values are not "known".. changeKnownToPossible(argvalues); - valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues); + // FIXME: We need to rewrite the valueflow analysis of function calls. This does not work well. + //valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues); } } } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index eb1e0a381..759232e29 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -368,7 +368,7 @@ private: " str[i] = 0;\n" "}\n" "void b() { a(16); }"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'str[16]' accessed at index 16, which is out of bounds.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array 'str[16]' accessed at index 16, which is out of bounds.\n", "", errout.str()); } @@ -4130,7 +4130,7 @@ private: " int a[sz];\n" "}\n" "void x() { f(-100); }"); - ASSERT_EQUALS("[test.cpp:2]: (error) Declaration of array 'a' with negative size is undefined behaviour\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Declaration of array 'a' with negative size is undefined behaviour\n", "", errout.str()); // don't warn for constant sizes -> this is a compiler error so this is used for static assertions for instance check("int x, y;\n" diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 01fcfeffb..f5166129b 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -1345,9 +1345,8 @@ private: " int i = s ? s->value + 1 \n" " : s->value - 1; // <-- null ptr dereference \n" " return i;\n" - "}\n" - "int main(){f(0);}\n", true); - ASSERT_EQUALS("[test.cpp:4]: (warning) Possible null pointer dereference: s\n", errout.str()); + "}\n"); + TODO_ASSERT_EQUALS("[test.cpp:4]: (warning) Possible null pointer dereference: s\n", "", errout.str()); } void nullpointer30() { // #6392 @@ -2596,7 +2595,7 @@ private: " p = s - 20;\n" "}\n" "void bar() { foo(0); }\n"); - ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", "", errout.str()); check("void foo(char *s) {\n" " if (!s) {}\n" @@ -2608,7 +2607,7 @@ private: " s -= 20;\n" "}\n" "void bar() { foo(0); }\n"); - ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", "", errout.str()); check("void foo(char *s) {\n" " if (!s) {}\n" @@ -2628,7 +2627,7 @@ private: " char * p = s + 20;\n" "}\n" "void bar() { foo(0); }\n"); - ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", "", errout.str()); check("void foo(char *s) {\n" " if (!s) {}\n" @@ -2640,7 +2639,7 @@ private: " char * p = 20 + s;\n" "}\n" "void bar() { foo(0); }\n"); - ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", "", errout.str()); check("void foo(char *s) {\n" " if (!s) {}\n" @@ -2652,7 +2651,7 @@ private: " s += 20;\n" "}\n" "void bar() { foo(0); }\n"); - ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", "", errout.str()); check("void foo(char *s) {\n" " if (!s) {}\n" @@ -2708,7 +2707,7 @@ private: "int main() {\n" " call(4,0);\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (error) Null pointer dereference: p\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (error) Null pointer dereference: p\n", "", errout.str()); ctu("void dostuff(int *x, int *y) {\n" " if (!var)\n" diff --git a/test/testother.cpp b/test/testother.cpp index 032d263f0..a29fa89cd 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -560,7 +560,7 @@ private: " f1(123,y);\n" " if (y>0){}\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Either the condition 'y>0' is redundant or there is division by zero at line 1.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Either the condition 'y>0' is redundant or there is division by zero at line 1.\n", "", errout.str()); // avoid false positives when variable is changed after division check("void f() {\n" diff --git a/test/teststring.cpp b/test/teststring.cpp index 5b79a7460..5460b22ec 100644 --- a/test/teststring.cpp +++ b/test/teststring.cpp @@ -98,7 +98,7 @@ private: " char* s = \"Y\";\n" " foo_FP1(s);\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (error) Modifying string literal \"Y\" directly or indirectly is undefined behaviour.\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (error) Modifying string literal \"Y\" directly or indirectly is undefined behaviour.\n", "", errout.str()); check("void foo_FP1(char *p) {\n" " p[1] = 'B';\n" diff --git a/test/testtype.cpp b/test/testtype.cpp index faecaa2c0..0695ec433 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -194,13 +194,13 @@ private: " return x * y;\n" "}\n" "void f2() { f1(-4,4); }"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", "", errout.str()); check("unsigned int f1(int x) {" // x has no signedness, but it can have the value -1 so assume it's signed " return x * 5U;\n" "}\n" "void f2() { f1(-4); }"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", "", errout.str()); check("unsigned int f1(int x) {" // #6168: FP for inner calculation " return 5U * (1234 - x);\n" // <- signed subtraction, x is not sign converted diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 3aa9485d4..14ef41534 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -90,8 +90,9 @@ private: TEST_CASE(valueFlowSwitchVariable); TEST_CASE(valueFlowForLoop); - TEST_CASE(valueFlowSubFunction); - TEST_CASE(valueFlowSubFunctionLibrary); + // TODO value flow in sub function + //TEST_CASE(valueFlowSubFunction); + //TEST_CASE(valueFlowSubFunctionLibrary); TEST_CASE(valueFlowFunctionReturn); TEST_CASE(valueFlowFunctionDefaultParameter); @@ -316,7 +317,7 @@ private: "}\n" "\n" "void test() { dostuff(\"abc\"); }"; - ASSERT_EQUALS(true, testValueOfX(code, 2, "\"abc\"", ValueFlow::Value::TOK)); + TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 2, "\"abc\"", ValueFlow::Value::TOK)); } void valueFlowPointerAlias() { @@ -687,14 +688,6 @@ private: ASSERT_EQUALS(false, testValueOfX(code, 3U, 0)); // function call => calculation - code = "void f(int x) {\n" - " a = x + 8;\n" - "}\n" - "void callf() {\n" - " f(7);\n" - "}"; - ASSERT_EQUALS(15, valueOfTok(code, "+").intvalue); - code = "void f(int x, int y) {\n" " a = x + y;\n" "}\n" @@ -929,9 +922,10 @@ private: " int x = 3;\n" " f1(x+1);\n" "}\n"; - ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n" - "6,Calling function 'f1', 1st argument 'x+1' value is 4\n", - getErrorPathForX(code, 2U)); + TODO_ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n" + "6,Calling function 'f1', 1st argument 'x+1' value is 4\n", + "", + getErrorPathForX(code, 2U)); code = "void f(int a) {\n" " int x;\n" @@ -3133,13 +3127,6 @@ private: "}"; ASSERT(isNotKnownValues(code, ">")); - // function - code = "int f(int x) { return x + 1; }\n" // <- possible value - "void a() { f(12); }"; - value = valueOfTok(code, "+"); - ASSERT_EQUALS(13, value.intvalue); - ASSERT(value.isPossible()); - // known and possible value code = "void f() {\n" " int x = 1;\n"