From b89f5fbeffd91f6207eece9a0deeb0453df4e8d3 Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Sun, 27 Jun 2021 11:51:32 +0300 Subject: [PATCH] misra: Fix 8.2 false positives (#3309) * misra: Fix 8.2 false positives Fix false positives in rule 8.2 that occurred in cases when we have a function definition and declaration in the same file. For example, the following code generated false positives before this commit: ``` void f(uint8_t * const x); void f(uint8_t * const x) { (void)x; } ``` We need to distinguish the declaration and the definition, so the dump file generation routine was extended to keep token where the definition of the function. The analysis in the addon also been improved. Closes Trac issue: https://trac.cppcheck.net/ticket/10219 --- addons/cppcheckdata.py | 7 ++- addons/misra.py | 85 +++++++++++++++++++++++++++------- addons/test/misra/misra-test.c | 9 +++- lib/symboldatabase.cpp | 5 +- 4 files changed, 85 insertions(+), 21 deletions(-) diff --git a/addons/cppcheckdata.py b/addons/cppcheckdata.py index aaddd31ff..86a35b2b1 100755 --- a/addons/cppcheckdata.py +++ b/addons/cppcheckdata.py @@ -415,6 +415,7 @@ class Function: Attributes argument Argument list + token Token in function implementation tokenDef Token in function definition isVirtual Is this function is virtual isImplicitlyVirtual Is this function is virtual this in the base classes @@ -424,6 +425,8 @@ class Function: Id = None argument = None argumentId = None + token = None + tokenId = None tokenDef = None tokenDefId = None name = None @@ -435,6 +438,7 @@ class Function: def __init__(self, element, nestedIn): self.Id = element.get('id') + self.tokenId = element.get('token') self.tokenDefId = element.get('tokenDef') self.name = element.get('name') self.type = element.get('type') @@ -450,7 +454,7 @@ class Function: self.argumentId = {} def __repr__(self): - attrs = ["Id", "tokenDefId", "name", "type", "isVirtual", + attrs = ["Id", "tokenId", "tokenDefId", "name", "type", "isVirtual", "isImplicitlyVirtual", "isStatic", "argumentId"] return "{}({})".format( "Function", @@ -460,6 +464,7 @@ class Function: def setId(self, IdMap): for argnr, argid in self.argumentId.items(): self.argument[argnr] = IdMap[argid] + self.token = IdMap.get(self.tokenId, None) self.tokenDef = IdMap[self.tokenDefId] diff --git a/addons/misra.py b/addons/misra.py index 600d56c06..cd696c094 100755 --- a/addons/misra.py +++ b/addons/misra.py @@ -1476,21 +1476,10 @@ class MisraChecker: if rawToken is None: break following.append(rawToken) - return following - # Check arguments in function declaration - for func in data.functions: - - startCall = func.tokenDef.next - if startCall is None or startCall.str != '(': - continue - - endCall = startCall.link - if endCall is None or endCall.str != ')': - continue - - # Zero arguments should be in form ( void ) + # Zero arguments should be in form ( void ) + def checkZeroArguments(func, startCall, endCall): if (len(func.argument) == 0): voidArg = startCall.next while voidArg is not endCall: @@ -1500,23 +1489,85 @@ class MisraChecker: if not voidArg.str == 'void': self.reportError(func.tokenDef, 8, 2) + def checkDeclarationArgumentsViolations(func, startCall, endCall): + # Collect the tokens for the arguments in function definition + argNameTokens = set() + for arg in func.argument: + argument = func.argument[arg] + typeStartToken = argument.typeStartToken + if typeStartToken is None: + continue + nameToken = argument.nameToken + if nameToken is None: + continue + argNameTokens.add(nameToken) + + # Check if we have the same number of variables in both the + # declaration and the definition. + # + # TODO: We actually need to check if the names of the arguments are + # the same. But we can't do this because we have no links to + # variables in the arguments in function defintion in the dump file. + foundVariables = 0 + while startCall and startCall != endCall: + if startCall.varId: + foundVariables += 1 + startCall = startCall.next + + if len(argNameTokens) != foundVariables: + self.reportError(func.tokenDef, 8, 2) + + def checkDefinitionArgumentsViolations(func, startCall, endCall): for arg in func.argument: argument = func.argument[arg] typeStartToken = argument.typeStartToken if typeStartToken is None: continue - nameToken = argument.nameToken # Arguments should have a name unless variable length arg + nameToken = argument.nameToken if nameToken is None and typeStartToken.str != '...': self.reportError(typeStartToken, 8, 2) # Type declaration on next line (old style declaration list) is not allowed - if ((typeStartToken.file == endCall.file) and - ((typeStartToken.linenr > endCall.linenr) or - (typeStartToken.column > endCall.column))): + if ((typeStartToken.linenr > endCall.linenr) or + (typeStartToken.column > endCall.column)): self.reportError(typeStartToken, 8, 2) + # Check arguments in function declaration + for func in data.functions: + + # Check arguments in function definition + tokenImpl = func.token + if tokenImpl: + startCall = tokenImpl.next + if startCall is None or startCall.str != '(': + continue + endCall = startCall.link + if endCall is None or endCall.str != ')': + continue + checkZeroArguments(func, startCall, endCall) + checkDefinitionArgumentsViolations(func, startCall, endCall) + + # Check arguments in function declaration + tokenDef = func.tokenDef + if tokenDef: + startCall = func.tokenDef.next + if startCall is None or startCall.str != '(': + continue + endCall = startCall.link + if endCall is None or endCall.str != ')': + continue + checkZeroArguments(func, startCall, endCall) + if tokenImpl: + checkDeclarationArgumentsViolations(func, startCall, endCall) + else: + # When there is no function definition, we should execute + # its checks for the declaration token. The point is that without + # a known definition we have no Function.argument list required + # for declaration check. + checkDefinitionArgumentsViolations(func, startCall, endCall) + # Check arguments in pointer declarations for var in data.variables: if not var.isPointer: diff --git a/addons/test/misra/misra-test.c b/addons/test/misra/misra-test.c index 5b552e6ec..c27ed23b3 100644 --- a/addons/test/misra/misra-test.c +++ b/addons/test/misra/misra-test.c @@ -319,6 +319,11 @@ static int misra_8_2_k ( // void); static int misra_8_2_l ( // 8.2 ); +void misra_8_2_m(uint8_t * const x); +void misra_8_2_m(uint8_t * const x) +{ +(void)x; +} int16_t ( *misra_8_2_p_a ) (); // 8.2 int16_t ( *misra_8_2_p_b ) (void); int16_t ( *misra_8_2_p_c ) (int); @@ -732,8 +737,8 @@ extern uint32_t misra_12_3_fn7(const uint32_t * const, const uint8_t); // 8.2 #define MISRA_12_3_FN3_2(A, B) (misra_12_3_fn3(A, \ B)) #define MISRA_12_3_FN3_2_MSG(x) x, fflush(stderr) -void misra_12_3(int, int, int); // no warning -void misra_12_3(int a, int b, int c) { // 8.2 +void misra_12_3(int, int, int); // 8.2 +void misra_12_3(int a, int b, int c) { int a1, a2; // 12.3 int a3; int a4; // no warning int a5 = 9, a6; // 12.3 diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 4ede9290a..98a2493ad 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -3679,7 +3679,10 @@ void SymbolDatabase::printXml(std::ostream &out) const if (!scope->functionList.empty()) { out << " " << std::endl; for (std::list::const_iterator function = scope->functionList.begin(); function != scope->functionList.end(); ++function) { - out << " tokenDef << "\" name=\"" << ErrorLogger::toxml(function->name()) << '\"'; + out << " token + << "\" tokenDef=\"" << function->tokenDef + << "\" name=\"" << ErrorLogger::toxml(function->name()) << '\"'; out << " type=\"" << (function->type == Function::eConstructor? "Constructor" : function->type == Function::eCopyConstructor ? "CopyConstructor" : function->type == Function::eMoveConstructor ? "MoveConstructor" :