From 316aa920eb9a690739dae73c450aacf934479d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 29 Oct 2011 19:11:42 +0200 Subject: [PATCH] Fixed #3245 (False positive: Dangerous usage of 'string' (strncpy doesn't always 0-terminate it)) --- lib/checkuninitvar.cpp | 52 ++++++++++++++++++++++++++++++------------ test/testuninitvar.cpp | 17 ++++++++++++++ 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index a03da46cc..23697f33c 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -240,17 +240,22 @@ private: if (mode == 0 && (c->array || (c->pointer && c->alloc))) continue; - // mode 2 : bad usage of pointer. if it's not a pointer then the usage is ok. + // mode 2 : reading array data with mem.. function. It's ok if the + // array is not 0-terminated + if (mode == 2 && c->strncpy_) + continue; + + // mode 3 : bad usage of pointer. if it's not a pointer then the usage is ok. // example: ptr->foo(); - if (mode == 2 && !c->pointer) + if (mode == 3 && !c->pointer) continue; - // mode 3 : using dead pointer is invalid. - if (mode == 3 && (!c->pointer || c->alloc)) + // mode 4 : using dead pointer is invalid. + if (mode == 4 && (!c->pointer || c->alloc)) continue; - // mode 4 : reading uninitialized array or pointer is invalid. - if (mode == 4 && (!c->array && !c->pointer)) + // mode 5 : reading uninitialized array or pointer is invalid. + if (mode == 5 && (!c->array && !c->pointer)) continue; CheckUninitVar *checkUninitVar = dynamic_cast(c->owner); @@ -290,6 +295,15 @@ private: use(checks, tok, 1); } + /** + * Reading array elements with a "mem.." function. It's ok if the array is not 0-terminated. + * @param checks all available checks + * @param tok variable token + */ + static void use_array_mem(std::list &checks, const Token *tok) { + use(checks, tok, 2); + } + /** * Bad pointer usage. If the variable is not a pointer then the usage is ok. * @param checks all available checks @@ -297,7 +311,7 @@ private: * @return if error is found, true is returned */ static bool use_pointer(std::list &checks, const Token *tok) { - return use(checks, tok, 2); + return use(checks, tok, 3); } /** @@ -307,7 +321,7 @@ private: * @return if error is found, true is returned */ static bool use_dead_pointer(std::list &checks, const Token *tok) { - return use(checks, tok, 3); + return use(checks, tok, 4); } /** @@ -318,10 +332,9 @@ private: * @return if error is found, true is returned */ static bool use_array_or_pointer_data(std::list &checks, const Token *tok) { - return use(checks, tok, 4); + return use(checks, tok, 5); } - /** declaring a variable */ void declare(std::list &checks, const Token *vartok, const Token &tok, const bool p, const bool a) const { if (vartok->varId() == 0) @@ -576,7 +589,12 @@ private: std::list var; CheckNullPointer::parseFunctionCall(tok, var, 1); for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) { - use_array(checks, *it); + // is function memset/memcpy/etc? + if (tok.str().compare(0,3,"mem") == 0) + use_array_mem(checks, *it); + else + use_array(checks, *it); + use_dead_pointer(checks, *it); } @@ -839,10 +857,16 @@ private: use_array_or_pointer_data(checks, tok.str() == "!" ? tok.next() : &tok); else if (Token::Match(&tok, "!| %var% (")) { + const Token * const ftok = (tok.str() == "!") ? tok.next() : &tok; std::list var; - CheckNullPointer::parseFunctionCall(tok.str() == "!" ? *tok.next() : tok, var, 1); - for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) - use_array(checks, *it); + CheckNullPointer::parseFunctionCall(*ftok, var, 1); + for (std::list::const_iterator it = var.begin(); it != var.end(); ++it) { + // is function memset/memcpy/etc? + if (ftok->str().compare(0,3,"mem") == 0) + use_array_mem(checks, *it); + else + use_array(checks, *it); + } } else if (Token::Match(&tok, "! %var% )")) { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 5dd56347c..67faa1ca3 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -1330,6 +1330,23 @@ private: " strncat(a, \"world\", 20);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + // #3245 - false positive + { + checkUninitVar("void f() {\n" + " char a[100];\n" + " strncpy(a,p,10);\n" + " memcmp(a,q,10);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + checkUninitVar("void f() {\n" + " char a[100];\n" + " strncpy(a,p,10);\n" + " if (memcmp(a,q,10)==0);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } } // initialization with memset (not 0-terminating string)..