cppcheck / Discussion / Development: Misra suppressions via .dump file are broken

Richard Smith - 2020-12-02

What was the reasoning for:

commit 1ce78a108604b3987c1fbb3f69df09789da2d3fc Author: Daniel Marjamäki <daniel.marjamaki@gmail.com> Date: Wed Nov 11 20:01:58 2020 +0100 Addons: handle inline suppressions internally in cppcheckdata

The commit message has zero information on why or any sort of justification.

This commit completely breaks MISRA suppressions when processing stand alone via .dump files because it is no longer loading the suppressions that were added into the dump file by cppcheck.

How is stand alone processing of .dump files supposed to work now?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

The reason is that we want that suppressions work for other addons also and not just misra. It is preferable to have a generic infrastructure for all addons.

It works for me when testing it. Test code:

m1.c:

void foo() {
   goto x;
}

No suppression:

$ ../../cppcheck --dump m1.c && python3 ../misra.py m1.c.dump 
Checking m1.c ...
Checking m1.c.dump...
Checking m1.c.dump, config ...
[m1.c:3] (style) misra violation (use --rule-texts=<file> to get proper output) (Undefined) [misra-c2012-15.1]
[m1.c:3] (style) misra violation (use --rule-texts=<file> to get proper output) (Undefined) [misra-c2012-15.2]

MISRA rules violations found:
        Undefined: 2

MISRA rules violated:
        misra-c2012-15.1 (-): 1
        misra-c2012-15.2 (-): 1

Suppressing misra-c2012-15.1

$ ../../cppcheck --dump --suppress=misra-c2012-15.1 m1.c && python3 ../misra.py m1.c.dump 
Checking m1.c ...
Checking m1.c.dump...
Checking m1.c.dump, config ...
[m1.c:3] (style) misra violation (use --rule-texts=<file> to get proper output) (Undefined) [misra-c2012-15.2]

MISRA rules violations found:
        Undefined: 2

MISRA rules violated:
        misra-c2012-15.1 (-): 1
        misra-c2012-15.2 (-): 1

Daniel Marjamäki - 2020-12-02

Also the suppression works the same through cppcheck core:

$ ../../cppcheck --addon=misra --suppress=misra-c2012-15.1 m1.c Checking m1.c ... m1.c:3:4: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-15.2] goto x; // cppcheck-suppress misra-c2012-15.1 ^

I think that is important too.. you should not get different suppressions depending on how you call it.

Last edit: Daniel Marjamäki 2020-12-02
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Look at the output closer. The text of the message was prevented but the summary still shows 2 rules violated and 15.1 is listed in the dump of rules violated.

$ ../../cppcheck --dump --suppress=misra-c2012-15.1 m1.c && python3 ../misra.py m1.c.dump 
Checking m1.c ...
Checking m1.c.dump...
Checking m1.c.dump, config ...
[m1.c:3] (style) misra violation (use --rule-texts=<file> to get proper output) (Undefined) [misra-c2012-15.2]

MISRA rules violations found:
        Undefined: 2

MISRA rules violated:
        misra-c2012-15.1 (-): 1
        misra-c2012-15.2 (-): 1

Also --show-suppressed-rules output is empty.

rsmith@rsmith-xps13:/home/src/cppcheck.git/addons/test$ ../../build/bin/cppcheck --dump --suppress=misra-c2012-15.1 ./misra/m1.c && python3 ../misra.py --show-suppressed-rules ./misra/m1.c.dump
Checking misra/m1.c ...
Checking ./misra/m1.c.dump...
Checking ./misra/m1.c.dump, config ...
[misra/m1.c:3] (style) misra violation (use --rule-texts=<file> to get proper output) (Undefined) [misra-c2012-15.2]

MISRA rules violations found:
        Undefined: 2

MISRA rules violated:
        misra-c2012-15.1 (-): 1
        misra-c2012-15.2 (-): 1
Suppressed Rules List:

Also --show-suppressed-rules output is empty.

rsmith@rsmith-xps13:/home/src/cppcheck.git/addons/test$ ../../build/bin/cppcheck --dump --suppress=misra-c2012-15.1 ./misra/m1.c && python3 ../misra.py --show-suppressed-rules ./misra/m1.c.dump
Checking misra/m1.c ...
Checking ./misra/m1.c.dump...
Checking ./misra/m1.c.dump, config ...
[misra/m1.c:3] (style) misra violation (use --rule-texts=<file> to get proper output) (Undefined) [misra-c2012-15.2]

MISRA rules violations found:
        Undefined: 2

MISRA rules violated:
        misra-c2012-15.1 (-): 1
        misra-c2012-15.2 (-): 1
Suppressed Rules List:

Richard Smith - 2020-12-02

The reason is that we want that suppressions work for other addons also and not just misra. It is preferable to have a generic infrastructure for all addons.

So what is that new infrastructure and how is misra.py supposed to use it?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Daniel Marjamäki - 2020-12-02

Look at the output closer. The text of the message was prevented but the summary still shows 2 rules violated and 15.1 is listed in the dump of rules violated.

it should be fixed.

In the long run I envision that it's better to put the summary in a common place like cppcheckdata. This is not functionality that strongly belongs to the misra addon. But this move will not be rushed.

So what is that new infrastructure and how is misra.py supposed to use it?

For the addons there is not much..
* An addon is not allowed to analyze 2 dumpfiles in parallell. It can analyze 2 dumpfiles in sequence. You can execute 2 addons in parallell if you want..
* Addons should call cppcheckdata.reportError to report errors.

If these 2 rules are followed the the addon should have proper handling of suppressions.

It would feel natural to me if cppcheckdata.reportError collects statistics.

Last edit: Daniel Marjamäki 2020-12-02

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Richard Smith - 2020-12-02
  
  Look at the output closer. The text of the message was prevented but the summary still shows 2 rules violated and 15.1 is listed in the dump of rules violated.
  
  it should be fixed
  
  Nod. How?
  
  Also --show-suppressed-rules output is empty.
  
  This is an important diagnostic tool that we use to figure out why suppression may not be working. How can this diagnostic be re-implemented such that it works? I do not see a similar cppcheck option.
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Daniel Marjamäki - 2020-12-03
    
    not sure.. I think some "quick hack" would be fine right now so it will work in the next release:
    * either cppcheckdata.reportError will return False when the warning is suppressed. Then Misra addon must collect the statistics.
    * cppcheckdata.reportError can collect some statistics about what messages are diagnosed/suppressed.
    
    I'd say the most straightforward /simple solution is best if we want something working now.. I want to release cppcheck-2.3 on saturday.
    
    We can add a flag isSuppressed in the class Suppression. Then it is possible for an addon to iterate suppressions and report those that was unused.
    
    For the long term I would suggest that we collect the statistics about reported/suppressed messages etc in a central place so all addons can easily write corresponding reports.
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
    - Richard Smith - 2020-12-03
      
      I've taken a look at this and cppcheckdata is only instantiated scope of the dump file being processed so it won't work for a summary of suppressions. It will need to be managed by something that stays in scope.
      
      We can add a flag isSuppressed in the class Suppression. Then it is possible for an addon to iterate suppressions and report those that was unused.
      
      The --show-suppressed-rules just showed all the suppression not the ones that were actually used so the flag isn't necessary.
      
      I propose that for now misra.py keep a list of the suppressions it finds so that --show-suppressed-rules can be fixed and that the MISRA summary information be removed since its broken now and with --addon its not available.
      
      If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

What do you think about this then.. it seems to work for --show-suppressed-rules as far as I see.. however please note it is just intended as a rough but working proof of concept.

diff --git a/addons/cppcheckdata.py b/addons/cppcheckdata.py
index a27e5c394..92f594820 100755
--- a/addons/cppcheckdata.py
+++ b/addons/cppcheckdata.py
@@ -16,6 +16,7 @@ from fnmatch import fnmatch
 EXIT_CODE = 0

 current_dumpfile_suppressions = []
+error_stats = { 'output': {}, 'suppressed': {} }

 class Directive:
     """
@@ -1134,11 +1135,20 @@ def reportError(location, severity, message, addon, errorId, extra=''):
                 'extra': extra}
         sys.stdout.write(json.dumps(msg) + '\n')
     else:
+        def add_error_stat(what, error_id):
+            global error_stats
+            if error_id not in error_stats[what]:
+                error_stats[what][error_id] = 1
+            else:
+                error_stats[what][error_id] += 1
+
         if is_suppressed(location, message, '%s-%s' % (addon, errorId)):
+            add_error_stat('suppressed', '%s-%s' % (addon, errorId))
             return
         loc = '[%s:%i]' % (location.file, location.linenr)
         if len(extra) > 0:
             message += ' (' + extra + ')'
         sys.stderr.write('%s (%s) %s [%s-%s]\n' % (loc, severity, message, addon, errorId))
+        add_error_stat('output', '%s-%s' % (addon, errorId))
         global EXIT_CODE
         EXIT_CODE = 1
diff --git a/addons/misra.py b/addons/misra.py
index 5055a8222..54d88c9de 100755
--- a/addons/misra.py
+++ b/addons/misra.py
@@ -3584,7 +3584,9 @@ def main():
                 print("\t%15s (%s): %d" % (misra_id, severity, rules_violated[misra_id]))

     if args.show_suppressed_rules:
-        checker.showSuppressedRules()
+        for error_id, count in cppcheckdata.error_stats['suppressed'].items():
+            print("suppressed: %s: %i" % (error_id, count))
+        #checker.showSuppressedRules()


 if __name__ == '__main__':

Last edit: Daniel Marjamäki 2020-12-03

Daniel Marjamäki - 2020-12-03

If you create some pull request that solves this now .. then I will appreciate it. it doesn't have to be a fully centralized solution at this point.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Richard Smith - 2020-12-04

That patch only shows suppressions if they actually created a violation. The previous behavior was to list all suppressions defined for all files regardless if a violation was triggered. The point of that option was to be be able to check if the suppression you create is actually getting picked up by the checker.

IMO you should revert that change and restore the previous behavior until a migration plan for implementing it in cppcheck is created while preserving the existing behavior.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Daniel Marjamäki - 2020-12-04

well.. I am scared that this will not be implemented properly then..

I can revert my commit.. and then after the release I revert back to current behavior.. and then there is some time to make it proper..

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Daniel Marjamäki - 2020-12-04

I only need to temporarily revert 1ce78a108604b3987c1fbb3f69df09789da2d3fc and then it will work as you want?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Richard Smith - 2020-12-04

I think so let me test and see.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Richard Smith - 2020-12-04
  
  Yes. That appears to restore the previous behavior.
  
  👍
  1
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Misra suppressions via .dump file are broken

Static source code analysis tool for C and C++ code

Forums

Help

Misra suppressions via .dump file are broken

Misra suppressions via .dump file are broken

Static source code analysis tool for C and C++ code

Forums

Help

Misra suppressions via .dump file are broken document.SUBSCRIPTION_OPTIONS = { "thing": "topic", "subscribed": false, "url": "subscribe", "icon": { "css": "fa fa-envelope-o" } };

Misra suppressions via .dump file are broken