From 7ce8ae408ab464e743528e14893bda7e9c941c17 Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Wed, 18 Dec 2019 13:05:57 +0300 Subject: [PATCH] misra.py: Fix R12.3 false negative in variables declaration (#2431) * misra.py: Allow executeCheck take multiple args * misra.py: Fix R12.3 FN in variables declaration Make rule 12.3 check detect commas in variables declaration code. * Fix excluded lines * Add support for global variables * Fix FN when linenr from other file was excluded * Add a few more tests * Handle more cases Handle additional cases to check commans in variables declaration including: * multiline variables declaration * functions and structures initialized in the same line * expanding macroses in initialization * Fix FP in global struct initialization * Add another test --- addons/misra.py | 85 ++++++++++++++++++++++++++++++---- addons/test/misra/misra-test.c | 62 ++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 11 deletions(-) diff --git a/addons/misra.py b/addons/misra.py index 6bc7f0ca8..f00d6e043 100755 --- a/addons/misra.py +++ b/addons/misra.py @@ -1349,15 +1349,82 @@ class MisraChecker: if maxval >= sz: self.reportError(token, 12, 2) - def misra_12_3(self, data): + def misra_12_3(self, data, rawTokens, filename): + # We need an additional check for closing braces using in + # initialization lists and function calls, e.g.: + # struct S a = {1, 2, 3}, b, c = foo(1, 2), d; + # ^ ^ + end_tokens_map = {} + for token in data.tokenlist: - if token.str != ',' or token.scope.type == 'Enum' or \ - token.scope.type == 'Class' or token.scope.type == 'Global': + if token.scope.type in ('Enum', 'Class', 'Struct', 'Global'): continue - if token.astParent and token.astParent.str in ['(', ',', '{']: + # Save end tokens from function calls in initialization + if simpleMatch(token, ') ;'): + if (token.isExpandedMacro): + end_tokens_map.setdefault(token.next.linenr, set()) + end_tokens_map[token.linenr].add(token.next.column) + else: + end_tokens_map.setdefault(token.linenr, set()) + end_tokens_map[token.linenr].add(token.column) + if token.str != ',': continue + if token.astParent: + if token.astParent.str in ('(', '{'): + end_token = token.astParent.link + if end_token: + end_tokens_map.setdefault(end_token.linenr, set()) + end_tokens_map[end_token.linenr].add(end_token.column) + continue + elif token.astParent.str == ',': + continue self.reportError(token, 12, 3) + # Cppcheck performs some simplifications in variables declaration code: + # int a, b, c; + # Will be reresented in dump file as: + # int a; int b; int c; + name_tokens_map = {} + for v in data.variables: + if v.isArgument: + continue + nt = v.nameToken + if nt and nt.scope and nt.scope.type not in ('Enum', 'Class', 'Struct'): + name_tokens_map.setdefault(nt.linenr, set()) + name_tokens_map[nt.linenr].add(nt.column) + if not name_tokens_map: + return + + # Select tokens to check + maybe_map = {} + for linenr in set(name_tokens_map.keys()) | set(end_tokens_map.keys()): + maybe_map[linenr] = name_tokens_map.get(linenr, set()) + maybe_map[linenr] |= end_tokens_map.get(linenr, set()) + + # Check variables declaration code in raw tokens to distinguish ';' and + # ',' symbols. + STATE_SKIP = 0 + STATE_CHECK = 1 + state = STATE_SKIP + cur_line = min(maybe_map) + for tok in rawTokens: + if tok.linenr in maybe_map and tok.column in maybe_map[tok.linenr]: + if tok.linenr in end_tokens_map and tok.column in end_tokens_map[tok.linenr]: + if tok.str == ',' or (tok.next and tok.next.str == ','): + self.reportError(tok, 12, 3) + end_tokens_map[tok.linenr].remove(tok.column) + state = STATE_CHECK + cur_line = tok.linenr + if tok.str in ('(', ';', '{'): + state = STATE_SKIP + if tok.linenr > cur_line: + maybe_map.pop(cur_line) + if not maybe_map: + break + cur_line = min(maybe_map) + if state == STATE_CHECK and tok.str == ',': + self.reportError(tok, 12, 3) + def misra_12_4(self, data): if typeBits['INT'] == 16: max_uint = 0xffff @@ -2454,18 +2521,18 @@ class MisraChecker: if not self.settings.quiet: print(*args, **kwargs) - def executeCheck(self, rule_num, check_function, arg): + def executeCheck(self, rule_num, check_function, *args): """Execute check function for a single MISRA rule. :param rule_num: Number of rule in hundreds format :param check_function: Check function to execute - :param arg: Check function argument + :param args: Check function arguments """ if not self.isRuleGloballySuppressed(rule_num): - check_function(arg) + check_function(*args) def parseDump(self, dumpfile): - + filename = '.'.join(dumpfile.split('.')[:-1]) data = cppcheckdata.parsedump(dumpfile) self.dumpfileSuppressions = data.suppressions @@ -2530,7 +2597,7 @@ class MisraChecker: self.executeCheck(1201, self.misra_12_1_sizeof, data.rawTokens) self.executeCheck(1201, self.misra_12_1, cfg) self.executeCheck(1202, self.misra_12_2, cfg) - self.executeCheck(1203, self.misra_12_3, cfg) + self.executeCheck(1203, self.misra_12_3, cfg, data.rawTokens, filename) self.executeCheck(1204, self.misra_12_4, cfg) self.executeCheck(1301, self.misra_13_1, cfg) self.executeCheck(1303, self.misra_13_3, cfg) diff --git a/addons/test/misra/misra-test.c b/addons/test/misra/misra-test.c index 097417a8e..d7e75489e 100644 --- a/addons/test/misra/misra-test.c +++ b/addons/test/misra/misra-test.c @@ -332,9 +332,67 @@ void misra_12_2(u8 x) { a = x << 8; // 12.2 } -void misra_12_3() { +static int misra_12_3_v1 = 0, misra_12_3_v2; // 12.3 +static int misra_12_3_v3, misra_12_3_v4; // 12.3 +enum misra_12_3_e1 { M123A1, M123B1, M123C1 }; +enum misra_12_3_e2 { M123A2 = 3, M123B2 = 4, M123C2 }; +typedef enum misra_12_3_e3 { M123A3 , M123B3, M123C3 } misra_12_3_e3_t; +typedef enum { M123A4 , M123B4, M123C4 } misra_12_3_e4_t; +struct misra_12_3_s1 { int a; int b; int c, d; }; +static struct misra_12_3_s1 misra_12_3_s1_inst = { + 3, + 4, 5, + 6, // no warning +}; +typedef struct misra_12_3_s2 { int a; int b; int c, d; } misra_12_3_s2_t; +typedef struct { int a; int b; int c, d; } misra_12_3_s3_t; +void misra_12_3_fn1(int, int); static int misra_12_3_v5, misra_12_4_v6; // 12.3 +void misra_12_3_fn2(int a, int b) { int d, e; } // 12.3 2.7 +int misra_12_3_fn3(int a, int b) { return a+b;} static int misra_12_3_v5, misra_12_4_v6; // 12.3 +#define MISRA_12_3_FN3_1(A, B) (misra_12_3_fn3(A, B)) +#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) // 20.7 +void misra_12_3(int, int, int); // no warning +void misra_12_3(int a, int b, int c) { // no warning + int a1, a2; // 12.3 + int a3; int a4; // no warning + int a5 = 9, a6; // 12.3 + int a7, a8 = 11; // 12.3 + int a9 = foo(), a10; // 12.3 + int a11 = a = b = c; // 17.8 + + struct s1 {int a, b;}; int a12, a13; // 12.3 + int a14, a15; misra_12_3_fn3(a14, a15); // 12.3 17.7 + ; int a16, a17; // 12.3 + int a18; int a19, a20; // 12.3 + int a21, a22; int a23; // 12.3 + int a24, // 12.3 + a25; + int a26 + , a27; // 12.3 + int a28 + , // 12.3 + a29; + + struct misra_12_3_s2 a30 = {1, 2}, a31; // 12.3 + struct misra_12_3_s2 a32, a33; // 12.3 + struct misra_12_3_s2 a34, a35 = {1, 2}, a36; // 12.3 + + int a37 = MISRA_12_3_FN3_1(a34, a35), a38; // 12.3 + int a39, a40 = MISRA_12_3_FN3_1(a34, a35); // 12.3 + int a41 = MISRA_12_3_FN3_2(a34, a35), a42; // 12.3 + int a43, a44 = MISRA_12_3_FN3_2(a34, a35); // 12.3 + + MISRA_12_3_FN3_2_MSG(fprintf(stderr, "test\n")); // 12.3 + f((1,2),3); // TODO - for (i=0;i<10;i++,j++){} // 12.3 + + for (i=0; i<10; i++, j++){} // 12.3 + for (int i = 0, p = &a1; // 12.3 14.2 + i < 42; + ++i, ++p ) // 12.3 + {} } #define MISRA12_4a 2000000000u