Menu

MISRA 12.3 FP

2020-09-03
2020-09-06
  • Richard Smith

    Richard Smith - 2020-09-03

    I've finally been able to track down the MISRA 12.3 FP that seems to occur randomly in our code base.
    I've attached a minimum example that will trigger the FP.

    This is a very strange bug as these files must be used EXACTLY with all the whitespace. The line in the header file and the line in the .c file must be on line 60. If you move them off of line 60 (with other code or whitespace) then the FP goes away.
    Tested with HEAD as of today.

    $ cppcheck --addon=misra.py misra_12_3_fp.c 
    Checking misra_12_3_fp.c ...
    misra_12_3_fp.c:60:43: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
        QEQueue_init(&me->deferred_event_queue, me->deferred_events_queue_buf, Q_DIM(me->deferred_events_queue_buf));
    
     
    • Jean-Loic MARTIN

      Good job ! I have the same problem but wasn't able to reproduce it with a minimum amount of code

       
  • Richard Smith

    Richard Smith - 2020-09-03

    Oh and sometimes we also trigger this FP from a structure init. ie

    somestruct_t somestruct =
    {
    .member1 = bla,
    .member2 = bla,
    };

    But I've not been able to reproduce it in a minimal test case.

     
  • Daniel Marjamäki

    Thanks! We should be able to fix that now.

     
  • Daniel Marjamäki

     
  • Georgiy Komarov

    Georgiy Komarov - 2020-09-04

    Thanks for the report!

    Here is PR: https://github.com/danmar/cppcheck/pull/2773

    @whoopsmith could you please try it?

     

    Last edit: Georgiy Komarov 2020-09-04
    • Richard Smith

      Richard Smith - 2020-09-04

      Thank you for the fast turnround.

      I can confirm that this PR resolves this 12.3 FP in the test case and in the specific situations in our codebase. Unfortunatly it does not resolve all of the 12.3 FPs. I'll see if I can narrow down the others. They are very hard to re-create since they seem to be dependent on other parts of the code. So trying to make a test cases makes them go away.

      Would it help if I sent you chunks of the .dump file for the sections that have the errors?

       
      • Georgiy Komarov

        Georgiy Komarov - 2020-09-04

        It is good that we managed to fix this problem at least partially.

        Would it help if I sent you chunks of the .dump file for the sections that have the errors?

        Yes, it might be useful.
        Is it possible to detect certain patterns that lead to false positives? For example, is they occur in the arguments of function calls, in initilizer lists for structs, etc.?

         
        • Richard Smith

          Richard Smith - 2020-09-04

          To get a listing of all the current item I ran cppcheck manuaally using --addon=misra.json so I could get a output of the line instead of just a line number. Pasted below.
          Whis is intersting is by using the --addon= method the number of FPs is vastly reduced. When I run it via our CMake build sytem which uses cppcheck --dump and then python3 msra.py I get over 40 FPs.

          ble_spi.c:417:62: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
          static QState ble_spi_slave_to_master(ao_ble_spi_t * const me, QEvt const * const e);
                                                                       ^
          ble_spi.c:452:52: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
              QTimeEvt_ctorX(&me->dfu_timer, (QActive *) me, LOCAL_BLE_DFU_TIMED_OUT_SIG, 0u);
                                                             ^
          --
          captouch.c:467:52: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
          bool captouch_cal_get_results(captouch_t * const me,
                                                             ^
          --
          led_ui.c:352:54: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
              {&led_pattern_ledR_on,          MS_TO_TICKS(384u)},
                                                               ^
          led_ui.c:357:54: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
              {&led_pattern_all_off,          MS_TO_TICKS(512u)},
                                                               ^
          --
          pmic.c:487:60: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
                          DEBUG_OUT(3u, "%s: LDO3Cfg: %x\n", me->name, (me->shadow_reg)[22]);
                                                                     ^
          --
          whoop_printf.c:696:41: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
                          out((char)va_arg(va, int), buffer, idx++, maxlen);
                                                  ^
          --
          whoop_sleep.c:487:52: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-12.3]
                      size = whoop_snprintf(exceptions_buff, EXCEPTIONS_ASSERT_BUF_END, "pre: %u %u\n", prev_time.epoch_seconds,
                                                             ^
          
           
  • Richard Smith

    Richard Smith - 2020-09-04

    I just realized a test case for one of the FPs. Our printf is a tweaked version of the printf found here:
    https://github.com/mpaland/printf
    If you run cppcheck and misra.py on that printf.c then you will get the same FP as in whoop_printf.c in my post above.

     
    • Georgiy Komarov

      Georgiy Komarov - 2020-09-05

      Thanks!
      It seems, that I fix it. Here is PR: https://github.com/danmar/cppcheck/pull/2778

       
  • Richard Smith

    Richard Smith - 2020-09-05

    Thank you for your work on these FPs. Your change does seem to fix that FP however it does not fix many of the other FPs.

    They seem to be code line dependent and have some dependency on previous state. Meaning that when I run cppcheck on our entire code base (with a list of files as arguments) then I get many of the FPs, but if I run cppcheck on just the single file then they don't show up.

    In a few case they do show up in both the filelist case and in the single case but when I start to remove code from the files they dissapear. In some cases all I'm removing are comment lines.

    Also I get different behavior betweeen running them with --dump and python3 misra.py vs using --addon. It goes both ways. Some of them show up in the --dump case but not in --addon and others show up in --addon but not in --dump.

    I keep trying to produce test cases but I can only get them to occur in our full code base.

    Are there diagnostic things I can add to to misra.py that would help track them down? Like dumping out the state of things when the FP occurs?

     
    • Georgiy Komarov

      Georgiy Komarov - 2020-09-06

      Are there diagnostic things I can add to to misra.py that would help track them down? Like dumping out the state of things when the FP occurs?

      I'm afraid not. We can't get useful debug information without the source code.
      I personally use pdb to look at the local variables of the current state and find out which part of the code led to the error.

       
  • Daniel Marjamäki

    I keep trying to produce test cases but I can only get them to occur in our full code base.

    this sounds very difficult.. number of whitespaces can matter..

    maybe the reduce script can help cppcheck/tools/reduce.py. If you can reproduce the FP with such command: cppcheck --addon=misra.py somefile.c then the reduce script could be executed like : reduce.py --cmd="cppcheck --addon=misra.py somefile.c" --expected="the FP warning" --file=somefile.c. Well typically I think that manually bisecting the code works better, but it is maybe worth a test.

     
  • Daniel Marjamäki

    I have the feeling that it would be a good idea to put more info in Cppcheck tokenlist and update the dump.. and then rewrite this check.

    A flag can be added in a Variable that says if its name token was preceded by a comma.

    int a, b;
    

    So the Variable for a would not say that it was preceded by comma. The Variable for b could say that it's preceded by comma.

    If that info is available in the dump file then the addon check should be rewritten.

     
  • Daniel Marjamäki

    I have added a flag in the Token class..

    int a,b;
    

    That declaration is splitted into:

    int a; int b;
    

    And for the ; token before int b, isSplittedVarDecl is True.

    See:
    https://github.com/danmar/cppcheck/commit/7969bf7ae825aced14d188a223c4658439ad97fa

    So well now the misra checker could be rewritten I think..

     
  • Daniel Marjamäki

    I have rewritten the checker. Can you retry the addon..

     
    • Richard Smith

      Richard Smith - 2020-09-06

      WooHoo! Your rewrite appears to elminate all of our 12.3 FPs for both the --addon and --dump checking methods.
      Thank you!

       
  • Daniel Marjamäki

    I wonder about this code:

    struct X {
        int a;
        int b,c;  // <---
    };
    

    shouldn't the addon write a warning there?

     
    • Richard Smith

      Richard Smith - 2020-09-06

      Yes. I think it should.

       

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.