RE: Xen FuSa meeting tomorrow Tue 17 November

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>]

Hi Artem,

This is very interesting.

In regards to comparisons, see the attached results from ResilTech. With
their tool, the top required rules with more than 1000 violations are:
15.6, 21.2, 5.1, 8.4.

I am appending below an rough analysis I did a few months back on the
top violations.

Cheers,

Stefano

---

# Rule 15.6

The body of an iteration-statement (while, for) or a selection-statement
(if, switch) shall be a compound statement.

For example:

    /* non compliant */
    if ( x )
        func();
    
    /* compliant */
    if ( x )
    {
        func();
    }

Where the real issue are cases like this:

    if ( one )
        if ( two )
            foo();
        else
            bar();


## Analysis & Potential Solution

There are many places in the Xen code where we skip the curly brackets
when the statement under 'if' is a one-liner. A solution that was
suggested before was to use GCC automatic checking for these situations:

    -Wmisleading-indentation (C and C++ only)
              Warn when the indentation of the code does not reflect the block
              structure.  Specifically, a warning is issued for "if", "else",
              "while", and "for" clauses with a guarded statement that does not
              use braces, followed by an unguarded statement with the same
              indentation.
   
              In the following example, the call to "bar" is misleadingly
              indented as if it were guarded by the "if" conditional.
   
                        if (some_condition ())
                          foo ();
                          bar ();  /* Gotcha: this is not guarded by the "if".  
*/

              [...]

We could document that -Wmisleading-indentation is required and should
be used as a check against this kind of issues. A patch to the Xen
Makefile to add -Wmisleading-indentation by default would also help. In
this context, we would be using GCC as a static code checker, not as a
compiler (GCC is not a safety certified compiler.)


# Rule 21.2

A reserved identifier or macro name shall not be declared.

Example, non-compliant:

    extern void *memcpy(void *restrict s1, const void *restrict s2, size_t n);

It is non-compliant because we are meant to #include <string.h> to get
the declaration of memcpy.


## Analysis & Potential Solution

In a kernel or a hypervisor it is not possible to #include <string.h>
because the standard library is not available.

Probably it is just a matter of documenting this deviation?


# Rule 5.1

External identifiers shall be distinct.

/* non compliant */
int32_t abc = 0;
int32_t ABC = 0;

/* non compliant: 31 significant chars are common */
int32_t engine_exhaust_gas_temperature_raw;
int32_t engine_exhaust_gas_temperature_scaled;

- in C90 the minimum requirement is that the first 6 characters of
  external identifiers are significant but their case is not required to
  be significant;
- in C99 the minimum requirement is that the first 31 characters of
  external identifiers are significant, with each universal character or
  corresponding extended source character occupying between 6 and 10
  characters. 


## Analysis & Potential Solution

In reality most implementations provide far greater limits. It is common
for external identifiers in C90 to be case-sensitive and for the first
31 characters to be significant.

Should we document that we require a C compiler that supports at least
the first 31 characters to be significant and case-sensitive? Then
configure the MISRA checker accordingly. Would it be good enough?


# Rule 8.4

A compatible declaration shall be visible when an object or function
with external linkage is defined.

Example:

extern int16_t count;
int16_t count = 1; /* compliant */

extern int16_t count = 1; /* non-compliant */



On Mon, 30 Nov 2020, Artem Mygaiev wrote:
> Hello all
> 
> I have played with cppcheck MISRA-2012 implementation a little, please see 
> some preliminary results below
> 
> Tools:
>  * https://github.com/danmar/cppcheck/releases/tag/2.2 built with default 
> settings
>  * Linaro's gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu
> 
> Experimented with current Xen staging tree & Stefano's tiny64_defconfigxen
> 
> Commands to check only xen folder excluding userspace tools and kconfig:
> $ export 
> PATH=$PATH:~/downloads/gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu/bin/
> $ XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-none-linux-gnu- make 
> tiny64_defconfigxen
> $ bear --exclude xen/tools make xen -j$NUMCPUS XEN_TARGET_ARCH=arm64 
> CROSS_COMPILE=aarch64-none-linux-gnu-
> $ ../cppcheck/cppcheck --include=xen/include/generated/autoconf.h 
> --project=compile_commands.json --dump
> $ find -name "*.dump" -print0 | xargs -0 python ../cppcheck/addons/misra.py
> 
> Results summary:
> MISRA rules violations found:
>         Undefined: 6134
> 
> MISRA rules violated:
>         misra-c2012-2.7 (-): 10
>         misra-c2012-3.1 (-): 42
>         misra-c2012-5.5 (-): 4
>         misra-c2012-7.3 (-): 52
>         misra-c2012-8.11 (-): 20
>         misra-c2012-9.5 (-): 4
>         misra-c2012-10.1 (-): 47
>         misra-c2012-10.4 (-): 195
>         misra-c2012-10.6 (-): 1
>         misra-c2012-11.3 (-): 10
>         misra-c2012-11.4 (-): 26
>         misra-c2012-11.5 (-): 131
>         misra-c2012-11.6 (-): 8
>         misra-c2012-11.8 (-): 7
>         misra-c2012-12.1 (-): 36
>         misra-c2012-12.3 (-): 83
>         misra-c2012-13.3 (-): 11
>         misra-c2012-13.4 (-): 56
>         misra-c2012-13.5 (-): 13
>         misra-c2012-14.2 (-): 2
>         misra-c2012-14.4 (-): 96
>         misra-c2012-15.1 (-): 7
>         misra-c2012-15.5 (-): 215
>         misra-c2012-15.6 (-): 4405
>         misra-c2012-15.7 (-): 6
>         misra-c2012-16.3 (-): 132
>         misra-c2012-16.4 (-): 2
>         misra-c2012-17.2 (-): 3
>         misra-c2012-17.7 (-): 28
>         misra-c2012-17.8 (-): 81
>         misra-c2012-18.4 (-): 85
>         misra-c2012-18.7 (-): 1
>         misra-c2012-19.2 (-): 46
>         misra-c2012-20.1 (-): 4
>         misra-c2012-20.5 (-): 5
>         misra-c2012-20.7 (-): 56
>         misra-c2012-20.10 (-): 22
>         misra-c2012-21.1 (-): 180
>         misra-c2012-21.9 (-): 2
> 
> Would be interesting to compare with other tools' results.
> 
> Best regards,
>  -- Artem
> 
> -----Original Message-----
> From: Norbert Manthey <nmanthey@xxxxxxxxx> 
> Sent: Friday, 20 November, 2020 10:46
> To: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Stefano Stabellini 
> <stefano.stabellini@xxxxxxxxxx>
> Cc: Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>; Julien Grall <julien@xxxxxxx>; 
> David Ward <david.ward@xxxxxxxxxxxxxxx>; Francesco Brancati 
> <francesco.brancati@xxxxxxxxxxxxx>; pserwa@xxxxxxxxx; 
> mszczepankiewicz@xxxxxxxxx; fusa-sig@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: Xen FuSa meeting tomorrow Tue 17 November
> 
> Hi all,
> 
> On 11/20/20 9:33 AM, Bertrand Marquis wrote:
> > CAUTION: This email originated from outside of the organization. Do not 
> > click links or open attachments unless you can confirm the sender and know 
> > the content is safe.
> >
> >
> >
> > Hi Stefano,
> >
> >> On 19 Nov 2020, at 20:20, Stefano Stabellini 
> >> <stefano.stabellini@xxxxxxxxxx> wrote:
> >>
> >> Hi Artem, Francesco, and all,
> >>
> >> I think we should come up with a shortlist of potential candidates. 
> >> As we discussed during the last two calls, aside from price, the 
> >> important parameters are:
> >>
> >> - completeness of the MISRAC checks
> >> - ability of being called automatically as part of the CI-loop
> >>    - availability of a remote API
> >>    - OR a license that allows for headless invocations
> >> - ability to take as input a list of deviations maintained together 
> >> with  the source code (so that can we have different deviations for 
> >> each Xen
> >>  branch)
> >>
> >> Does this set of criteria seem reasonable?
> > I think an important one is the ability to share the results of the tool.
> > If the results are protected by some kind of license we will end up 
> > having to fix all problems without an opportunity to share the tool 
> > results on the mailing list for example if the CI loop can (and I 
> > think it is critical that it does) be executed on pushes to the mailing 
> > list before they are merged in staging.
> 
> We can run the Coverity analysis locally, so that would fix the "run on 
> mailing list contributions" requirement. Next, we process the output of the 
> command, and extract relevant information (introduced defects, their trace, 
> fixed/dropped defects, ...) and format that into notifications of the 
> relevant style (we have scripts for Coverity similarly to what we did for 
> Infer, Fortify, CBMC, CppCheck, ... in 
> https://urldefense.com/v3/__https://github.com/awslabs/one-line-scan__;!!GF_29dbcQIUBPA!iW5BfqLOXh9Q0qkIBYwp4F8hOzmxxvo6yHFb3V2Oq2vt_8DWXmMFnibdKsRMxx7uTw$
>  [github[.]com]).
> 
> I am not a license expert, so I cannot tell what would be required to be 
> allowed to share the results of email contributions back. This requirement 
> should certainly be brought up early to select an appropriate tool.
> 
> Best,
> Norbert
> 
> >
> > Cheers
> > Bertrand
> >
> >>
> >> On Thu, 19 Nov 2020, Artem Mygaiev wrote:
> >>> Hello all
> >>>
> >>> Using Coverity for MISRA was considered some time ago at the very first 
> >>> days of FuSa SIG. Coverity indeed supports MISRA but there's a catch: 
> >>> Open Source version of Coverity (Coverity Scan) does not support MISRA.
> >>>
> >>> BR,
> >>> -- Artem
> >>>
> >>> -----Original Message-----
> >>> From: Fusa-sig <fusa-sig-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> >>> Julien Grall
> >>> Sent: четверг, 19 ноября 2020 г. 12:27
> >>> To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> >>> Cc: David Ward <david.ward@xxxxxxxxxxxxxxx>; Francesco Brancati 
> >>> <francesco.brancati@xxxxxxxxxxxxx>; pserwa@xxxxxxxxx; 
> >>> mszczepankiewicz@xxxxxxxxx; fusa-sig@xxxxxxxxxxxxxxxxxxxx; Manthey, 
> >>> Norbert <nmanthey@xxxxxxxxx>
> >>> Subject: Re: Xen FuSa meeting tomorrow Tue 17 November
> >>>
> >>> (+Norbert)
> >>>
> >>> On 18/11/2020 19:15, Julien Grall wrote:
> >>>>>> So you have enough data from coverity to provide some meaningful 
> >>>>>> information.
> >>>>>> :) I don't know whether this is based on just parsing e-mails or 
> >>>>>> tools coverity may provide in a paid version.
> >>>>> Excellent. Yeah, that is the kind of level of integration that we need.
> >>>> I have asked the person in charge if he could provide more details 
> >>>> how this was setup and the complexity to get it working. I will let 
> >>>> you know when I have an answer.
> >>> I have CCed Norbert who worked on the integration of coverity. @Stefano, 
> >>> feel free to ask more details on the setup.

Attachment: CodecheckResultXEN0.5.xlsx
Description: MS-Excel 2007 spreadsheet