[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Top MISRA violations


  • To: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
  • Date: Thu, 23 Apr 2020 09:52:37 -0700 (PDT)
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.60.83) smtp.rcpttodomain=arm.com smtp.mailfrom=xilinx.com; dmarc=bestguesspass action=none header.from=xilinx.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oQ6Z/coVpUUZZIgvQB5hGc48LXNbwv1+P/B9E8EB5cw=; b=hueZkiEYUDPJnfQBog9KKUfzvkakgT7G32tp8MMjM1JyaJeniIWFdortVrMK9Q+lIzGeNUMvqvGslYhtF8aweKZlhwPqnzUu0fy0ihR9Nj5FBFxnvKtJPA7geAcchjmOoCan59Um2Lt7pZcx/3YXTHBhDEsLI1D72G3EcYpOZu55nnArpe+t9fzUjnt/PgZYRcslERo/PdEdigHGthQAX7kBD6azYQmQiVY4bHAIHoxlPCq+U4lSlRRzGZv5bHe6EEnszt/G4k8p+t3/gmDapjVPUs8jLrw6TBDHlKpCH63uMu9DYH8bsiQGn5jUSGLiY9I4hl/+bJe/Q0OzE781uw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Zma+dUUoM8SvgUxoJFFHKM2hvZXNivHB/sY1T/EPJyV2iJaded2x1zSOrc7u4wy5t5w5EUwRKqRxSHgP2vEHdt5cm0065doDcBwVwFoHYuJKtgVZVLjn7gSaGExEjJSiF1QizrTttPi8HTDK7mYB4WeMAvDwb5jV1YIAyCH6E0+YxNFHnopFUPp18GnstFmqMEh1tLtPd33fpy37YhwV1hUgAp2y5RhF0itwDk0ZGbW4t96egMQXN9dexnKrlycqepgbP0Qvty6sJ9lbq4WUPMsiwsKNb8mJgLvWgm2stc2HfYFCPQKESDpf+pPAHhcyD6xJnrNaCwEiOtkmyo8cDw==
  • Authentication-results: spf=pass (sender IP is 149.199.60.83) smtp.mailfrom=xilinx.com; arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=bestguesspass action=none header.from=xilinx.com;
  • Cc: "fusa-sig@xxxxxxxxxxxxxxxxxxxx" <fusa-sig@xxxxxxxxxxxxxxxxxxxx>, nd <nd@xxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>, Julien Grall <julien.grall.oss@xxxxxxxxx>
  • Delivery-date: Thu, 23 Apr 2020 16:52:50 +0000
  • List-id: This is a discussion list for members of the Xen Project FuSa SIG <fusa-sig.lists.xenproject.org>

On Thu, 23 Apr 2020, Bertrand Marquis wrote:
>       On 23 Apr 2020, at 01:52, Stefano Stabellini 
> <stefano.stabellini@xxxxxxxxxx> wrote:
> 
> On Wed, 22 Apr 2020, Julien Grall wrote:
>       Hi,
> 
>       On Wed, 22 Apr 2020 at 21:50, Stefano Stabellini
>       <stefano.stabellini@xxxxxxxxxx> wrote:
> 
>             Hi all,
> 
>             Francesco kindly shared his spreadsheet with the analysis of the 
> MISRA
>             violations, see attached. I am sharing it here to the list as all 
> the
>             members of fuse-sig have access to MISRA. I also want to take this
>             opportunity to thank Francesco and his team for their work.
> 
> 
>             Looking at the spreadsheet, the top required rules with more than 
> 1000
>             violations are: 15.6, 21.2, 5.1, 8.4.
> 
> 
>             # 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.)
> 
> 
>       A quick look at the man [1] suggests that -Wall already include
>       -Wmisleading-indentation on recent GCC. Note that is has been known to
>       bring some false positive in older version of GCC. So if we force on
>       compiler that doesn't include it in -Wall, then we may end up to have
>       more nuisance than needed.
> 
>       How about documenting the version of GCC to use?
> 
> 
> Yep, from my understanding that would be good enough. (I'd love for
> Antonio and other experts to confirm.)
> 
> 
> From my experience -Wall is neither or to much but more undefined as it 
> changing with the versions of compiler.
> So in the certification processes I went through the only solution is to 
> explicitly list all of them so that it can be reviewed.

What you wrote makes sense.

Also, I think Julien meant that we should use -Wmisleading-indentation
only with GCC versions that include -Wmisleading-indentation in -Wall
(due to quality of the implemantation in GCC). Not necessarely use -Wall
instead of -Wmisleading-indentation.

So in this case, we could document something like:

  use GCC from version X to version Y with -Wmisleading-indentation
  to check for Rule 15.6


> It would suggest to put all warning in one make include file with a comment 
> for each of them and classify them with:
> - mandatory
> - recommended
> - other
> 
> Mandatory should always be activated, recommended could be selectable by user 
> and activated during internal tests.
> Other could be a place holder for new flags or proposals.
> 
> It should be also possible to filter some of them depending on the compiler 
> and version of compiler used, the architecture or configuration
> 
> There is some work the first time but once done that should not be to much 
> overhead to maintain those.
 
Looks like a good and solid way of doing it

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.