From 6366a577f981f5c0d809bedbb2e16a4342ad0fe2 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Sat, 19 Dec 2020 09:55:19 +0200 Subject: [PATCH] feat(unused_var): analyze global variables inside function body (#2944) --- lib/checkunusedvar.cpp | 24 ++++++-- test/testunusedvar.cpp | 130 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 146 insertions(+), 8 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index e20b87d2c..7c41a9798 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1609,17 +1609,33 @@ bool CheckUnusedVar::isFunctionWithoutSideEffects(const Function& func, const To } bool sideEffectReturnFound = false; + std::set pointersToGlobals; for (Token* bodyToken = func.functionScope->bodyStart->next(); bodyToken != func.functionScope->bodyEnd; - bodyToken = bodyToken->next()) { + bodyToken = bodyToken->next()) + { + // check variable inside function body const Variable* bodyVariable = bodyToken->variable(); if (bodyVariable) { - // check variable for side-effects if (!isVariableWithoutSideEffects(*bodyVariable)) { return false; } // check if global variable is changed - if (bodyVariable->isGlobal()) { - return false; // TODO: analyze global variable usage + if (bodyVariable->isGlobal() || (pointersToGlobals.find(bodyVariable) != pointersToGlobals.end()) ) { + if (bodyVariable->isPointer() || bodyVariable->isArray()) { + return false; // TODO: Update astutils.cpp:1544 isVariableChanged() and remove this. Unhandled case: `*(global_arr + 1) = new_val` + } + const int depth = 20; + if (isVariableChanged(bodyToken, depth, mSettings, mTokenizer->isCPP())) { + return false; + } + // check if pointer to global variable assigned to another variable (another_var = &global_var) + if (Token::simpleMatch(bodyToken->tokAt(-1), "&") && Token::simpleMatch(bodyToken->tokAt(-2), "=")) { + const Token* assigned_var_token = bodyToken->tokAt(-3); + if (assigned_var_token && assigned_var_token->variable()) + { + pointersToGlobals.insert(assigned_var_token->variable()); + } + } } } diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index af2eb6782..dd240c5b1 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -615,7 +615,7 @@ private: "void f() {\n" " C c;\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:11]: (style) Unused variable: c\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:11]: (style) Unused variable: c\n", errout.str()); // changing global variable in return functionVariableUsage( @@ -698,6 +698,23 @@ private: "}"); ASSERT_EQUALS("", errout.str()); + // global variable use in function body without change + functionVariableUsage( + "int global = 1;\n" + "int func() {\n" + " int x = global + 1;\n" + " return x;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + ASSERT_EQUALS("[test.cpp:12]: (style) Unused variable: c\n", errout.str()); + // changing global array variable in function body functionVariableUsage( "int x[] = {0, 1, 3};\n" @@ -715,6 +732,38 @@ private: "}"); ASSERT_EQUALS("", errout.str()); + functionVariableUsage( + "int x[] = {0, 1, 3};\n" + "int func() {\n" + " *x = 2;\n" + " return 1;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage( + "int x[] = {0, 1, 3};\n" + "int func() {\n" + " *(x + 1) = 2;\n" + " return 1;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + // changing local variable functionVariableUsage( "int func() {\n" @@ -985,7 +1034,7 @@ private: functionVariableUsage( "int global = 1;\n" "int func() {\n" - " int *p = &global;\n" + " int* p = &global;\n" " *p = 0;\n" " return 1;\n" "}\n" @@ -999,11 +1048,30 @@ private: "}"); ASSERT_EQUALS("", errout.str()); - // global struct variable + // global variable assigning to local pointer, but not modifying + functionVariableUsage( + "int global = 1;\n" + "int func() {\n" + " int* p = &global;\n" + " (void) p;\n" + " return 1;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + // TODO: see TODO for global vars under CheckUnusedVar::isFunctionWithoutSideEffects() + TODO_ASSERT_EQUALS("[test.cpp:13]: (style) Unused variable: c\n", "", errout.str()); + + // global struct variable modification functionVariableUsage( "struct S { int x; } s;\n" "int func() {\n" - " s.x = 1;;\n" + " s.x = 1;\n" " return 1;\n" "}\n" "class C {\n" @@ -1015,6 +1083,60 @@ private: " C c;\n" "}"); ASSERT_EQUALS("", errout.str()); + + // global struct variable without modification + functionVariableUsage( + "struct S { int x; } s;\n" + "int func() {\n" + " int y = s.x + 1;\n" + " return y;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + ASSERT_EQUALS("[test.cpp:12]: (style) Unused variable: c\n", errout.str()); + + // global pointer to struct variable modification + functionVariableUsage( + "struct S { int x; };\n" + "struct S* s = new(struct S);\n" + "int func() {\n" + " s->x = 1;\n" + " return 1;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // global pointer to struct variable without modification + functionVariableUsage( + "struct S { int x; };\n" + "struct S* s = new(struct S);\n" + "int func() {\n" + " int y = s->x + 1;\n" + " return y;\n" + "}\n" + "class C {\n" + "public:\n" + " C() : x(func()) {}\n" + " int x;\n" + "};\n" + "void f() {\n" + " C c;\n" + "}"); + // TODO: see TODO for global vars under CheckUnusedVar::isFunctionWithoutSideEffects() + TODO_ASSERT_EQUALS("[test.cpp:13]: (style) Unused variable: c\n", "", errout.str()); } // #5355 - False positive: Variable is not assigned a value.