walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed #3407 (False positive: (inconclusive) Found duplicate branches for if and else. (inline assembler))

This commit is contained in:
PKEuS 2011-12-13 21:42:38 +01:00 committed by Daniel Marjamäki
parent 34fba9e1ea
commit c9f5117cf5
8 changed files with 105 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
lib/checkother.cpp
View File

@ -2233,37 +2233,6 @@ void CheckOther::incorrectStringBooleanError(const Token *tok, const std::string
// check for duplicate expressions in if statements // check for duplicate expressions in if statements
// if (a) { } else if (a) { } // if (a) { } else if (a) { }
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
static const std::string stringifyTokens(const Token *start, const Token *end)
{
const Token *tok = start;
std::string stringified;
if (tok->isUnsigned())
stringified.append("unsigned ");
else if (tok->isSigned())
stringified.append("signed ");
if (tok->isLong())
stringified.append("long ");
stringified.append(tok->str());
while (tok && tok->next() && tok != end) {
if (tok->isUnsigned())
stringified.append("unsigned ");
else if (tok->isSigned())
stringified.append("signed ");
if (tok->isLong())
stringified.append("long ");
tok = tok->next();
stringified.append(" ");
stringified.append(tok->str());
}
return stringified;
}
static bool expressionHasSideEffects(const Token *first, const Token *last) static bool expressionHasSideEffects(const Token *first, const Token *last)
{ {
@ -2301,7 +2270,7 @@ void CheckOther::checkDuplicateIf()
std::map<std::string, const Token*> expressionMap; std::map<std::string, const Token*> expressionMap;
// get the expression from the token stream // get the expression from the token stream
std::string expression = stringifyTokens(tok->tokAt(2), tok->next()->link()->previous()); std::string expression = tok->tokAt(2)->stringify(tok->next()->link());
// save the expression and its location // save the expression and its location
expressionMap.insert(std::make_pair(expression, tok)); expressionMap.insert(std::make_pair(expression, tok));
@ -2313,7 +2282,7 @@ void CheckOther::checkDuplicateIf()
while (Token::simpleMatch(tok1, "} else if (") && while (Token::simpleMatch(tok1, "} else if (") &&
Token::simpleMatch(tok1->linkAt(3), ") {")) { Token::simpleMatch(tok1->linkAt(3), ") {")) {
// get the expression from the token stream // get the expression from the token stream
expression = stringifyTokens(tok1->tokAt(4), tok1->linkAt(3)->previous()); expression = tok1->tokAt(4)->stringify(tok1->linkAt(3));
// try to look up the expression to check for duplicates // try to look up the expression to check for duplicates
std::map<std::string, const Token *>::iterator it = expressionMap.find(expression); std::map<std::string, const Token *>::iterator it = expressionMap.find(expression);
@ -2356,28 +2325,30 @@ void CheckOther::checkDuplicateBranch()
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
if (!_settings->inconclusive)
return;
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
std::list<Scope>::const_iterator scope; std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) { for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
const Token* const tok = scope->classDef; const Token* tok = scope->classDef;
if ((scope->type != Scope::eIf && scope->type != Scope::eElseIf) || !tok)
continue;
if (tok->str() == "else")
tok = tok->next();
// check all the code in the function for if (..) else // check all the code in the function for if (..) else
if (scope->type == Scope::eIf && tok && tok->next() && if (tok && tok->next() && Token::simpleMatch(tok->next()->link(), ") {") &&
Token::simpleMatch(tok->next()->link(), ") {") &&
Token::simpleMatch(tok->next()->link()->next()->link(), "} else {")) { Token::simpleMatch(tok->next()->link()->next()->link(), "} else {")) {
// save if branch code // save if branch code
std::string branch1 = stringifyTokens(tok->next()->link()->tokAt(2), tok->next()->link()->next()->link()->previous()); std::string branch1 = tok->next()->link()->tokAt(2)->stringify(tok->next()->link()->next()->link());
// find else branch // find else branch
const Token *tok1 = tok->next()->link()->next()->link(); const Token *tok1 = tok->next()->link()->next()->link();
// save else branch code // save else branch code
std::string branch2 = stringifyTokens(tok1->tokAt(3), tok1->linkAt(2)->previous()); std::string branch2 = tok1->tokAt(3)->stringify(tok1->linkAt(2));
// check for duplicates // check for duplicates
if (branch1 == branch2) if (branch1 == branch2)

2
lib/checkuninitvar.cpp
View File

@ -633,7 +633,7 @@ private:
return tok.next()->link(); return tok.next()->link();
} }
if (Token::simpleMatch(&tok, "asm ( )")) { if (Token::Match(&tok, "asm ( %str% )")) {
ExecutionPath::bailOut(checks); ExecutionPath::bailOut(checks);
return &tok; return &tok;
} }

2
lib/checkunusedvar.cpp
View File

@ -697,7 +697,7 @@ void CheckUnusedVar::checkFunctionVariableUsage()
break; break;
} }
if (Token::Match(tok, "[;{}] asm ( ) ;")) { if (Token::Match(tok, "[;{}] asm ( %str% )")) {
variables.clear(); variables.clear();
break; break;
} }

26
lib/token.cpp
View File

@ -889,6 +889,30 @@ void Token::printOut(const char *title, const std::vector<std::string> &fileName
std::cout << stringifyList(true, title, fileNames) << std::endl; std::cout << stringifyList(true, title, fileNames) << std::endl;
} }
std::string Token::stringify(const Token* end) const
{
std::ostringstream ret;
if (isUnsigned())
ret << "unsigned ";
else if (isSigned())
ret << "signed ";
if (isLong())
ret << "long ";
ret << str();
for (const Token *tok = this->next(); tok && tok != end; tok = tok->next()) {
if (tok->isUnsigned())
ret << " unsigned";
else if (tok->isSigned())
ret << " signed";
if (tok->isLong())
ret << " long";
ret << " " << tok->str();
}
return ret.str();
}
std::string Token::stringifyList(bool varid, const char *title) const std::string Token::stringifyList(bool varid, const char *title) const
{ {
const std::vector<std::string> fileNames; const std::vector<std::string> fileNames;
@ -938,5 +962,3 @@ std::string Token::stringifyList(bool varid, const char *title, const std::vecto
return ret.str(); return ret.str();
} }

1
lib/token.h
View File

@ -326,6 +326,7 @@ public:
static void replace(Token *replaceThis, Token *start, Token *end); static void replace(Token *replaceThis, Token *start, Token *end);
/** Stringify a token list (with or without varId) */ /** Stringify a token list (with or without varId) */
std::string stringify(const Token* end) const;
std::string stringifyList(bool varid = false, const char *title = 0) const; std::string stringifyList(bool varid = false, const char *title = 0) const;
std::string stringifyList(bool varid, const char *title, const std::vector<std::string> &fileNames) const; std::string stringifyList(bool varid, const char *title, const std::vector<std::string> &fileNames) const;

47
lib/tokenize.cpp
View File

@ -2008,18 +2008,20 @@ bool Tokenizer::tokenize(std::istream &code,
for (Token *tok = _tokens; tok; tok = tok->next()) { for (Token *tok = _tokens; tok; tok = tok->next()) {
if (Token::simpleMatch(tok, "EXEC SQL")) { if (Token::simpleMatch(tok, "EXEC SQL")) {
// delete all tokens until ";" // delete all tokens until ";"
const Token *end = tok; const Token *end = tok->tokAt(2);
while (end && end->str() != ";") while (end && end->str() != ";")
end = end->next(); end = end->next();
std::string instruction = tok->stringify(end);
Token::eraseTokens(tok, end); Token::eraseTokens(tok, end);
// insert "asm ( ) ;" // insert "asm ( "instruction" ) ;"
tok->str("asm"); tok->str("asm");
// it can happen that 'end' is NULL when wrong code is inserted // it can happen that 'end' is NULL when wrong code is inserted
if (!tok->next()) if (!tok->next())
tok->insertToken(";"); tok->insertToken(";");
tok->insertToken(")"); tok->insertToken(")");
tok->insertToken("\"" + instruction + "\"");
tok->insertToken("("); tok->insertToken("(");
// jump to ';' and continue // jump to ';' and continue
tok = tok->tokAt(3); tok = tok->tokAt(3);
@ -3054,9 +3056,9 @@ void Tokenizer::simplifyTemplatesInstantiate(const Token *tok,
std::list<Token *> &used, std::list<Token *> &used,
std::set<std::string> &expandedtemplates) std::set<std::string> &expandedtemplates)
{ {
// this variable is not used at the moment. the intention was to // this variable is not used at the moment. The intention was to
// allow continuous instantiations until all templates has been expanded // allow continuous instantiations until all templates has been expanded
bool done = false; //bool done = false;
std::vector<const Token *> type; std::vector<const Token *> type;
for (tok = tok->tokAt(2); tok && tok->str() != ">"; tok = tok->next()) { for (tok = tok->tokAt(2); tok && tok->str() != ">"; tok = tok->next()) {
@ -3277,8 +3279,8 @@ void Tokenizer::simplifyTemplatesInstantiate(const Token *tok,
// copy // copy
addtoken(tok3, tok3->linenr(), tok3->fileIndex()); addtoken(tok3, tok3->linenr(), tok3->fileIndex());
if (Token::Match(tok3, "%type% <")) { if (Token::Match(tok3, "%type% <")) {
if (!Token::simpleMatch(tok3, (name + " <").c_str())) //if (!Token::simpleMatch(tok3, (name + " <").c_str()))
done = false; //done = false;
used.push_back(_tokensBack); used.push_back(_tokensBack);
} }
@ -3396,10 +3398,10 @@ void Tokenizer::simplifyTemplates()
simplifyTemplatesUseDefaultArgumentValues(templates, used); simplifyTemplatesUseDefaultArgumentValues(templates, used);
// expand templates // expand templates
bool done = false; //bool done = false;
//while (!done) //while (!done)
{ {
done = true; //done = true;
for (std::list<Token *>::reverse_iterator iter1 = templates.rbegin(); iter1 != templates.rend(); ++iter1) { for (std::list<Token *>::reverse_iterator iter1 = templates.rbegin(); iter1 != templates.rend(); ++iter1) {
simplifyTemplatesInstantiate(*iter1, used, expandedtemplates); simplifyTemplatesInstantiate(*iter1, used, expandedtemplates);
} }
@ -9325,39 +9327,44 @@ void Tokenizer::simplifyAssignmentInFunctionCall()
// Remove __asm.. // Remove __asm..
void Tokenizer::simplifyAsm() void Tokenizer::simplifyAsm()
{ {
std::string instruction;
for (Token *tok = _tokens; tok; tok = tok->next()) { for (Token *tok = _tokens; tok; tok = tok->next()) {
if (Token::Match(tok->next(), "__asm|_asm|asm {") && if (Token::Match(tok, "__asm|_asm|asm {") &&
tok->linkAt(2)->next()) { tok->linkAt(1)->next()) {
Token::eraseTokens(tok, tok->linkAt(2)->next()); instruction = tok->tokAt(2)->stringify(tok->linkAt(1));
Token::eraseTokens(tok, tok->linkAt(1)->next());
} }
else if (Token::Match(tok->next(), "asm|__asm|__asm__ volatile|__volatile__| (")) { else if (Token::Match(tok, "asm|__asm|__asm__ volatile|__volatile__| (")) {
// Goto "(" // Goto "("
Token *partok = tok->tokAt(2); Token *partok = tok->next();
if (partok->str() != "(") if (partok->str() != "(")
partok = partok->next(); partok = partok->next();
instruction = partok->next()->stringify(partok->link());
Token::eraseTokens(tok, partok->link()->next()); Token::eraseTokens(tok, partok->link()->next());
} }
else if (Token::simpleMatch(tok->next(), "__asm")) { else if (Token::simpleMatch(tok, "__asm")) {
const Token *tok2 = tok->next(); const Token *tok2 = tok;
while (tok2 && (tok2->isNumber() || tok2->isName() || tok2->str() == ",")) while (tok2 && (tok2->isNumber() || tok2->isName() || tok2->str() == ","))
tok2 = tok2->next(); tok2 = tok2->next();
if (tok2 && tok2->str() == ";") if (tok2 && tok2->str() == ";") {
instruction = tok->next()->stringify(tok2);
Token::eraseTokens(tok, tok2); Token::eraseTokens(tok, tok2);
else } else
continue; continue;
} }
else else
continue; continue;
// insert "asm ( )" // insert "asm ( "instruction" )"
tok->str("asm");
tok->insertToken(")"); tok->insertToken(")");
tok->insertToken("\"" + instruction + "\"");
tok->insertToken("("); tok->insertToken("(");
tok->insertToken("asm");
Token::createMutualLinks(tok->tokAt(2), tok->tokAt(3)); Token::createMutualLinks(tok->tokAt(1), tok->tokAt(3));
//move the new tokens in the same line as ";" if available //move the new tokens in the same line as ";" if available
tok = tok->tokAt(3); tok = tok->tokAt(3);

26
test/testother.cpp
View File

@ -3717,6 +3717,16 @@ private:
"}"); "}");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (style) Found duplicate branches for if and else.\n", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (style) Found duplicate branches for if and else.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a == 1)\n"
" b = 1;\n"
" else if (a)\n"
" b = 2;\n"
" else\n"
" b = 2;\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:4]: (style) Found duplicate branches for if and else.\n", errout.str());
check("int f(int signed, unsigned char value) {\n" check("int f(int signed, unsigned char value) {\n"
" int ret;\n" " int ret;\n"
" if (signed)\n" " if (signed)\n"
@ -3726,6 +3736,22 @@ private:
" return ret;\n" " return ret;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" if (b)\n"
" __asm__(\"mov ax, bx\");\n"
" else\n"
" __asm__(\"mov bx, bx\");\n"
"}");
ASSERT_EQUALS("", errout.str()); // #3407
check("void f() {\n"
" if (b)\n"
" __asm__(\"mov ax, bx\");\n"
" else\n"
" __asm__(\"mov ax, bx\");\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (style) Found duplicate branches for if and else.\n", errout.str());
} }
void duplicateExpression1() { void duplicateExpression1() {

26
test/testtokenize.cpp
View File

@ -741,19 +741,19 @@ private:
} }
void inlineasm() { void inlineasm() {
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";asm { mov ax,bx };")); ASSERT_EQUALS("; asm ( \"mov ax , bx\" ) ;", tokenizeAndStringify(";asm { mov ax,bx };"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";_asm { mov ax,bx };")); ASSERT_EQUALS("; asm ( \"mov ax , bx\" ) ;", tokenizeAndStringify(";_asm { mov ax,bx };"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";__asm { mov ax,bx };")); ASSERT_EQUALS("; asm ( \"mov ax , bx\" ) ;", tokenizeAndStringify(";__asm { mov ax,bx };"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";__asm__ __volatile__ ( \"mov ax,bx\" );")); ASSERT_EQUALS("; asm ( \"\"mov ax,bx\"\" ) ;", tokenizeAndStringify(";__asm__ __volatile__ ( \"mov ax,bx\" );"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";__asm _emit 12h ;")); ASSERT_EQUALS("; asm ( \"_emit 12h\" ) ;", tokenizeAndStringify(";__asm _emit 12h ;"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";__asm mov a, b ;")); ASSERT_EQUALS("; asm ( \"mov a , b\" ) ;", tokenizeAndStringify(";__asm mov a, b ;"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";asm volatile (\"fnstcw %0\" : \"= m\" (old_cw));")); ASSERT_EQUALS("; asm ( \"\"fnstcw %0\" : \"= m\" ( old_cw )\" ) ;", tokenizeAndStringify(";asm volatile (\"fnstcw %0\" : \"= m\" (old_cw));"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify("; __asm__ (\"fnstcw %0\" : \"= m\" (old_cw));")); ASSERT_EQUALS("; asm ( \"\"fnstcw %0\" : \"= m\" ( old_cw )\" ) ;", tokenizeAndStringify("; __asm__ (\"fnstcw %0\" : \"= m\" (old_cw));"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify("; __asm __volatile__ (\"ddd\") ;")); ASSERT_EQUALS("; asm ( \"\"ddd\"\" ) ;", tokenizeAndStringify("; __asm __volatile__ (\"ddd\") ;"));
ASSERT_EQUALS("; asm ( ) ;", tokenizeAndStringify(";__asm__ volatile ( \"mov ax,bx\" );")); ASSERT_EQUALS("; asm ( \"\"mov ax,bx\"\" ) ;", tokenizeAndStringify(";__asm__ volatile ( \"mov ax,bx\" );"));
// 'asm ( ) ;' should be in the same line // 'asm ( ) ;' should be in the same line
ASSERT_EQUALS(";\n\nasm ( ) ;", tokenizeAndStringify(";\n\n__asm__ volatile ( \"mov ax,bx\" );", true)); ASSERT_EQUALS(";\n\nasm ( \"\"mov ax,bx\"\" ) ;", tokenizeAndStringify(";\n\n__asm__ volatile ( \"mov ax,bx\" );", true));
} }
@ -5797,8 +5797,8 @@ private:
void sql() { void sql() {
// Oracle PRO*C extensions for inline SQL. Just replace the SQL with "asm()" to fix wrong error messages // Oracle PRO*C extensions for inline SQL. Just replace the SQL with "asm()" to fix wrong error messages
// ticket: #1959 // ticket: #1959
ASSERT_EQUALS("asm ( ) ;", tokenizeAndStringify("EXEC SQL SELECT A FROM B;",false)); ASSERT_EQUALS("asm ( \"\"EXEC SQL SELECT A FROM B\"\" ) ;", tokenizeAndStringify("EXEC SQL SELECT A FROM B;",false));
ASSERT_EQUALS("asm ( ) ;", tokenizeAndStringify("EXEC SQL",false)); ASSERT_EQUALS("asm ( \"\"EXEC SQL\"\" ) ;", tokenizeAndStringify("EXEC SQL",false));
} }