Skip to content

Conversation

@jubnzv
Copy link
Contributor

@jubnzv jubnzv commented Sep 24, 2019

This commit will fix issues with MISRA rule 4.1 caused by improper escape sequences handling.

References:

  1. https://trac.cppcheck.net/ticket/9370
  2. https://sourceforge.net/p/cppcheck/discussion/development/thread/7274ed3842/?limit=25#799a

@jubnzv jubnzv changed the title Fix Rule 4.1 misra.py: Fix Rule 4.1 Sep 24, 2019
@jubnzv
Copy link
Contributor Author

jubnzv commented Sep 25, 2019

@matzeschmid @whoopsmith could you please review these changes before merging? This solves the problems on your codebase?

@matzeschmid
Copy link
Contributor

If you add a max. sequence length check in isOctalEscapeSequence() then the following non-compliant examples will be catched.

int c41_12        = '\12323';  // 4.1
int c41_13        = '\1232\3'; // 4.1
def isOctalEscapeSequence(symbols):
    """Checks that given symbols are valid octal escape sequence.
    Reference: n1570 6.4.4.4"""
    if len(symbols) < 2 or len(symbols) > 4 or symbols[0] != '\\':
        return False
    return all([s in string.octdigits for s in symbols[1:]])

@matzeschmid
Copy link
Contributor

I removed the restriction to just check assignments to see how the fix works for non-assignment test cases. After that it generates false positives because the escape sequence split adds a '\' to each sub-token. Thus the line const char *s41_3 = "\x41" "g"; generates false positive because split result for token g is \g .
There is also a false positive for (void)printf("%i", '\141'); because %i gets \%i .

Sequence split line in script.
for sequence in ['\\' + t for t in token.str[1:-1].split('\\')]:

addons/misra.py Outdated
def isHexEscapeSequence(symbols):
"""Checks that given symbols are valid hex escape sequence.
Reference: n1570 6.4.4.4"""
if len(symbols) < 3 or symbols[0] != '\\' or symbols[1] != 'x':
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could compress this condition I think:

if len(symbols) < 3 or symbols[:2] != '\\x':

@whoopsmith
Copy link
Contributor

Doesn't work for me at all. It throws 4.1 errors all over the place for things that are not octal sequences. Like below:

static const char * mfg_pass = "pass";

@jubnzv
Copy link
Contributor Author

jubnzv commented Sep 25, 2019

Thanks for the thorough review.
It seems there are more pitfalls, I'll fix it.

@matzeschmid
Copy link
Contributor

Thanks for the reasonable review.
It seems there are more pitfalls, I'll fix it.

I think the wrong sequence split described above generates most of the false positives.

@whoopsmith
Copy link
Contributor

I ran the latest revision and all the false positives are gone. I haven't tested if a valid error gets caught yet.

@danmar danmar merged commit 846f356 into danmar:master Sep 28, 2019
jubnzv added a commit to jubnzv/cppcheck that referenced this pull request Nov 13, 2019
* misra.py: Use standard string module

* misra.py: Fixup R4.1

References:
* https://trac.cppcheck.net/ticket/9370
* https://sourceforge.net/p/cppcheck/discussion/development/thread/7274ed3842/?limit=25#799a

* Add more examples from @matzeschmid PR

* Add more out-of-bounds tests

* Fix R4.1.

* Compress hex condition

* Add more test cases

* Fix python3 zip import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants