From 2e56928834aa043c6c80b5acdf0144aa01519c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 13 Jan 2013 12:02:10 +0100 Subject: [PATCH] Fixed #4482 (add test for UB due to usage of NULL in variadic functions) --- lib/checkother.cpp | 82 +++++++++++++++++++++++++++++++++++++ lib/checkother.h | 9 +++- lib/tokenize.cpp | 12 +++--- test/testother.cpp | 8 ++++ test/testsimplifytokens.cpp | 2 +- test/testtokenize.cpp | 7 ++++ 6 files changed, 112 insertions(+), 8 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a2fbc3740..db79da181 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3748,3 +3748,85 @@ void CheckOther::oppositeInnerConditionError(const Token *tok) { reportError(tok, Severity::warning, "oppositeInnerCondition", "Opposite conditions in nested 'if' blocks lead to a dead code block.", true); } + + +void CheckOther::checkVarFuncNullUB() +{ + if (!_settings->isEnabled("portability")) + return; + + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + const std::size_t functions = symbolDatabase->functionScopes.size(); + for (std::size_t i = 0; i < functions; ++i) { + const Scope * scope = symbolDatabase->functionScopes[i]; + for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + // Is NULL passed to a function? + if (Token::Match(tok,"[(,] NULL [,)]")) { + // Locate function name in this function call. + const Token *ftok = tok; + while (ftok && ftok->str() != "(") { + if (ftok->str() == ")") + ftok = ftok->link(); + ftok = ftok->previous(); + } + ftok = ftok ? ftok->previous() : NULL; + if (ftok && ftok->isName()) { + // If this is a variadic function then report error + const Function *f = symbolDatabase->findFunctionByName(ftok->str(), scope); + if (f) { + const Token *tok2 = f->argDef; + tok2 = tok2 ? tok2->link() : NULL; // goto ')' + if (Token::simpleMatch(tok2->tokAt(-3), ". . .")) + varFuncNullUBError(tok); + } + } + } + } + } +} + +void CheckOther::varFuncNullUBError(const Token *tok) +{ + reportError(tok, + Severity::portability, + "varFuncNullUB", + "Passing NULL to a function with variable number of arguments leads to undefined behaviour on some platforms.\n" + "Passing NULL to a function with variable number of arguments leads to undefined behaviour on some platforms.\n" + "The behaviour is undefined when NULL is #defined as 0, sizeof(int)!=sizeof(void*) and the function expects a pointer. Otherwise the behaviour is defined.\n" + "See section section 7.1.4 and section 7.15.1.1 in the C standard. Section 7.1.4 explains that the function call is UB. Section 7.15.1.1 explains that the va_arg macro has UB.\n" + "To reproduce you might be able to use this little code example. Try it on a platform where sizeof(int)!=sizeof(void*), for instance on a x86_64 machine. If ERROR is written by the program on the screen it means that 0 is not converted to a NULL pointer. Changing the 0 to (void*)0 will fix the program.\n" + "#include \n" + "#include \n" + "\n" + "void f(char *s, ...) {\n" + " va_list ap;\n" + " va_start(ap,s);\n" + " for (;;) {\n" + " char *p = va_arg(ap,char*);\n" + " printf(\"%018p, %s\n\", p, (long)p & 255 ? p : \"\");\n" + " if(!p) break;\n" + " }\n" + " va_end(ap);\n" + "}\n" + "\n" + "void g() {\n" + " char *s2 = \"x\";\n" + " char *s3 = \"ERROR\";\n" + "\n" + " // changing 0 to 0L makes the error go away on x86_64\n" + " f(\"first\", s2, s2, s2, s2, s2, 0, s3, (char*)0);\n" + "}\n" + "\n" + "void h() {\n" + " int i;\n" + " volatile unsigned char a[1000];\n" + " for (i = 0; i &constFunctions, diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index ca5932d39..945e17bb5 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -2131,14 +2131,14 @@ void Tokenizer::simplifyFileAndLineMacro() void Tokenizer::simplifyNull() { for (Token *tok = list.front(); tok; tok = tok->next()) { - if (tok->str() == "NULL" || tok->str() == "__null" || - tok->str() == "'\\0'" || tok->str() == "'\\x0'") { + if (tok->str() == "NULL" && !Token::Match(tok->previous(), "[(,] NULL [,)]")) tok->str("0"); - } else if (tok->isNumber() && - MathLib::isInt(tok->str()) && - MathLib::toLongNumber(tok->str()) == 0) { + else if (tok->str() == "__null" || tok->str() == "'\\0'" || tok->str() == "'\\x0'") + tok->str("0"); + else if (tok->isNumber() && + MathLib::isInt(tok->str()) && + MathLib::toLongNumber(tok->str()) == 0) tok->str("0"); - } } // nullptr.. diff --git a/test/testother.cpp b/test/testother.cpp index 7978f3352..5e923853c 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -188,6 +188,8 @@ private: TEST_CASE(redundantVarAssignment); TEST_CASE(redundantMemWrite); + + TEST_CASE(varFuncNullUB); } void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) { @@ -6799,6 +6801,12 @@ private: "}"); ASSERT_EQUALS("", errout.str()); } + + void varFuncNullUB() { // #4482 + check("void a(...);\n" + "void b() { a(NULL); }"); + ASSERT_EQUALS("[test.cpp:2]: (portability) Passing NULL to a function with variable number of arguments leads to undefined behaviour on some platforms.\n", errout.str()); + } }; REGISTER_TEST(TestOther) diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index 8f884715e..a61ec8c16 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -3407,7 +3407,7 @@ private: ASSERT_EQUALS("void f ( int i ) { goto label ; { label : ; " + *it + " ; } }",tok("void f (int i) { goto label; switch(i) { label: " + *it + "; } }")); //ticket #3148 ASSERT_EQUALS("void f ( ) { MACRO ( " + *it + " ) }",tok("void f() { MACRO(" + *it + ") }")); - ASSERT_EQUALS("void f ( ) { MACRO ( " + *it + " ; , 0 ) }",tok("void f() { MACRO(" + *it + ";, NULL) }")); + ASSERT_EQUALS("void f ( ) { MACRO ( " + *it + " ; , NULL ) }",tok("void f() { MACRO(" + *it + ";, NULL) }")); ASSERT_EQUALS("void f ( ) { MACRO ( bar1 , " + *it + " ) }",tok("void f() { MACRO(bar1, " + *it + ") }")); ASSERT_EQUALS("void f ( ) { MACRO ( " + *it + " ; bar2 , foo ) }",tok("void f() { MACRO(" + *it + "; bar2, foo) }")); } diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 7d2bfe48c..0cad5d602 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -448,6 +448,8 @@ private: TEST_CASE(simplifyOperatorName5); TEST_CASE(simplifyOperatorName6); // ticket #3194 + TEST_CASE(simplifyNull); + TEST_CASE(simplifyNullArray); // Some simple cleanups of unhandled macros in the global scope @@ -7207,6 +7209,11 @@ private: ASSERT_EQUALS(result2, tokenizeAndStringify(code2,false)); } + void simplifyNull() { + ASSERT_EQUALS("if ( p == 0 )", tokenizeAndStringify("if (p==NULL)")); + ASSERT_EQUALS("f ( NULL ) ;", tokenizeAndStringify("f(NULL);")); + } + void simplifyNullArray() { ASSERT_EQUALS("* ( foo . bar [ 5 ] ) = x ;", tokenizeAndStringify("0[foo.bar[5]] = x;")); }