From 2b8b0c44b2c0b6ca48829e58377a430e40ca3955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 9 Aug 2011 18:24:39 +0200 Subject: [PATCH] Fixed #2969 (False positive: assign address of auto-var to function parameter, when function parameter is reassigned later) --- lib/checkautovariables.cpp | 60 +++++++++++++++++++++++++------------- lib/checkautovariables.h | 4 +-- test/testautovariables.cpp | 26 +++++++++++++---- 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 8529f3a94..b19d473b9 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -99,27 +99,35 @@ void CheckAutoVariables::autoVariables() { const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(5)->varId()); if (var && (!var->isClass() || var->type())) - errorAutoVariableAssignment(tok); + errorAutoVariableAssignment(tok, false); } else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) { - const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(1)->varId()); - if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-2), "%type% *")) + // TODO: check if the parameter is only changed temporarily (#2969) + if (_settings->inconclusive) { - const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(6)->varId()); - if (var2 && var2->isLocal() && !var2->isStatic()) - errorAutoVariableAssignment(tok); + const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(1)->varId()); + if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-2), "%type% *")) + { + const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(6)->varId()); + if (var2 && var2->isLocal() && !var2->isStatic()) + errorAutoVariableAssignment(tok, _settings->inconclusive); + } } tok = tok->tokAt(6); } else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) { - const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(1)->varId()); - if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-2), "%type% *")) + // TODO: check if the parameter is only changed temporarily (#2969) + if (_settings->inconclusive) { - const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(5)->varId()); - if (var2 && var2->isLocal() && var2->isArray() && !var2->isStatic()) - errorAutoVariableAssignment(tok); + const Variable * var1 = symbolDatabase->getVariableFromVarId(tok->tokAt(1)->varId()); + if (var1 && var1->isArgument() && Token::Match(var1->nameToken()->tokAt(-2), "%type% *")) + { + const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(5)->varId()); + if (var2 && var2->isLocal() && var2->isArray() && !var2->isStatic()) + errorAutoVariableAssignment(tok, _settings->inconclusive); + } } tok = tok->tokAt(5); } @@ -130,13 +138,13 @@ void CheckAutoVariables::autoVariables() { const Variable * var2 = symbolDatabase->getVariableFromVarId(tok->tokAt(4)->varId()); if (var2 && var2->isLocal() && var2->isArray() && !var2->isStatic()) - errorAutoVariableAssignment(tok); + errorAutoVariableAssignment(tok, false); } tok = tok->tokAt(4); } else if (Token::Match(tok, "[;{}] %var% [ %any% ] = & %var%") && errorAv(tok->tokAt(1), tok->tokAt(7))) { - errorAutoVariableAssignment(tok); + errorAutoVariableAssignment(tok, false); } // Critical return else if (Token::Match(tok, "return & %var% ;") && isAutoVar(tok->tokAt(2)->varId())) @@ -220,14 +228,26 @@ void CheckAutoVariables::errorReturnPointerToLocalArray(const Token *tok) reportError(tok, Severity::error, "returnLocalVariable", "Returning pointer to local array variable"); } -void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok) +void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inconclusive) { - reportError(tok, Severity::error, "autoVariables", - "Assigning address of local auto-variable to a function parameter.\n" - "Dangerous assignment - function parameter takes the address of a local " - "auto-variable. Local auto-variables are reserved from the stack. And the " - "stack is freed when the function ends. So the pointer to a local variable " - "is invalid after the function ends."); + if (!inconclusive) + { + reportError(tok, Severity::error, "autoVariables", + "Assigning address of local auto-variable to a function parameter.\n" + "Dangerous assignment - function parameter takes the address of a local " + "auto-variable. Local auto-variables are reserved from the stack. And the " + "stack is freed when the function ends. So the pointer to a local variable " + "is invalid after the function ends."); + } + else + { + reportInconclusiveError(tok, Severity::error, "autoVariables", + "Inconclusive: Assigning address of local auto-variable to a function parameter.\n" + "Inconclusive: function parameter takes the address of a local auto-variable. " + "Local auto-variables are reserved from the stack. And the stack is freed when " + "the function ends. The address is invalid after the function ends and it " + "might 'leak' from the function through the parameter."); + } } //--------------------------------------------------------------------------- diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 43d9c166c..7f77150b2 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -83,7 +83,7 @@ private: void errorReturnAddressToAutoVariable(const Token *tok); void errorReturnPointerToLocalArray(const Token *tok); - void errorAutoVariableAssignment(const Token *tok); + void errorAutoVariableAssignment(const Token *tok, bool inconclusive); void errorReturnReference(const Token *tok); void errorReturnTempReference(const Token *tok); void errorReturnAutocstr(const Token *tok); @@ -93,7 +93,7 @@ private: void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { CheckAutoVariables c(0,settings,errorLogger); - c.errorAutoVariableAssignment(0); + c.errorAutoVariableAssignment(0, false); c.errorReturnAddressToAutoVariable(0); c.errorReturnPointerToLocalArray(0); c.errorReturnReference(0); diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 13dfcde02..5a7bf0503 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -35,13 +35,13 @@ private: - void check(const char code[]) + void check(const char code[], bool inconclusive=false) { // Clear the error buffer.. errout.str(""); Settings settings; - settings.debugwarnings = true; + settings.inconclusive = inconclusive; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -183,8 +183,15 @@ private: "{\n" " char a;\n" " ab->a = &a;\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str()); + "}", false); + ASSERT_EQUALS("", errout.str()); + + check("void foo(struct AB *ab)\n" + "{\n" + " char a;\n" + " ab->a = &a;\n" + "}", true); + ASSERT_EQUALS("[test.cpp:3]: (error) Inconclusive: Assigning address of local auto-variable to a function parameter.\n", errout.str()); } void testautovar6() // ticket #2931 @@ -193,8 +200,15 @@ private: "{\n" " char a[10];\n" " x->str = a;\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Assigning address of local auto-variable to a function parameter.\n", errout.str()); + "}", false); + ASSERT_EQUALS("", errout.str()); + + check("void foo(struct X *x)\n" + "{\n" + " char a[10];\n" + " x->str = a;\n" + "}", true); + ASSERT_EQUALS("[test.cpp:3]: (error) Inconclusive: Assigning address of local auto-variable to a function parameter.\n", errout.str()); } void testautovar_array1()