On Thu, 7 Dec 2023, Julien Grall wrote:
> Hi Federico,
> 
> On 07/12/2023 09:08, Federico Serafini wrote:
> > MISRA C:2012 Rule 16.3 states that an unconditional break statement
> > shall terminate every switch-clause.
> > 
> > Update ECLAIR configuration to take into account:
> > - continue, goto, return statements;
> > - functions and macros that do not give the control back;
> > - fallthrough comments and pseudo-keywords.
> > 
> > Update docs/misra/deviations.rst accordingly.
> > 
> > Signed-off-by: Federico Serafini <federico.seraf...@bugseng.com>
> > ---
> >   .../eclair_analysis/ECLAIR/deviations.ecl     | 18 ++++++++++++++
> >   docs/misra/deviations.rst                     | 24 +++++++++++++++++++
> >   2 files changed, 42 insertions(+)
> 
> It would be good that this is depending on to be accepted:
> 
> https://lore.kernel.org/alpine.DEB.2.22.394.2312051859440.110490@ubuntu-linux-20-04-desktop.
> 
> > 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index b0c79741b5..df0b58a010 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -321,6 +321,24 @@ statements are deliberate"
> >   -config=MC3R1.R14.3,statements={deliberate ,
> > "wrapped(any(),node(if_stmt))" }
> >   -doc_end
> >   +#
> > +# Series 16.
> > +#
> > +
> > +-doc_begin="Switch clauses ending with continue, goto, return statements
> > are safe."
> > +-config=MC3R1.R16.3,terminals+={safe,
> > "node(continue_stmt||goto_stmt||return_stmt)"}
> > +-doc_end
> > +
> > +-doc_begin="Switch clauses not ending with the break statement are safe if
> > a function/macro that does not give the control back is present."
> > +-config=MC3R1.R16.3,terminals+={safe,
> > "call(decl(name(__builtin_unreachable||do_unexpected_trap||fatal_trap||machine_halt||machine_restart||maybe_reboot||panic)))"}
> > +-config=MC3R1.R16.3,terminals+={safe,"macro(name(BUG||BUG_ON))"}
> > +-doc_end
> > +
> > +-doc_begin="Switch clauses not ending with the break statement are safe if
> > an explicit comment or pseudo-keyword indicating the fallthrough intention
> > is present."
> > +-config=MC3R1.R16.3,reports+={safe,
> > "any_area(any_loc(any_exp(text(^(?s).*([fF]all[- ]?[tT]hrough|FALL[-
> > ]?THROUGH).*$,0..1))))"}
> > +-config=MC3R1.R16.3,reports+={safe, "any_area(text(^(?s).*([fF]all[-
> > ]?[tT]hrough|FALL[- ]?THROUGH).*$,0..1))"}
> 
> This is not trivial to read. Can you document the exhaustive list of keywords
> you are actually trying to deviate on? Also, did you consider to harmonize to
> only a few?
> 
> > +-doc_end
> > +
> >   #
> >   # Series 20.
> >   #
> > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > index 6e7c4f25b8..fecd2bae8e 100644
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -270,6 +270,30 @@ Deviations related to MISRA C:2012 Rules:
> >          statements are deliberate.
> >        - Project-wide deviation; tagged as `disapplied` for ECLAIR.
> >   +   * - R16.3
> > +     - Switch clauses ending with continue, goto, return statements are
> > safe.
> > +     - Tagged as `safe` for ECLAIR.
> > +
> > +   * - R16.3
> > +     - Switch clauses not ending with the break statement are safe if a
> > +       function/macro that does not give the control back is present.
> > +     - Tagged as `safe` for ECLAIR, such functions/macros are:
> > +        - __builtin_unreachable
> > +        - do_unexpected_trap
> > +        - fatal_trap
> > +        - machine_halt
> > +        - machine_restart
> > +        - maybe_reboot
> > +        - panic
> > +        - BUG
> 
> To me, it seems to be odd to deviate R16.3 by function. Some of those have an
> attribute noreboot. Can this be used?

Just to clarify, I think Julien meant "noreturn" which is defined as
__attribute__((__noreturn__))

I think we need to deviate by function, rather than using SAF, because
we would have to use SAF in every switch rather than at the declaration
of panic and friends. But if we could use noreturn, that would be
awesome.


> > +        - BUG_ON
> 
> BUG_ON() can return if the condition is false. So it doesn't look correct to
> deviate with the argument that the function doesn't give the control back...

+1


> > +
> > +   * - R16.3
> > +     - Switch clauses not ending with the break statement are safe if an
> > +       explicit comment or pseudo-keyword indicating the fallthrough
> > intention
> > +       is present.
> > +     - Tagged as `safe` for ECLAIR.
> > +
> >      * - R20.7
> >        - Code violating Rule 20.7 is safe when macro parameters are used:
> >          (1) as function arguments;

Reply via email to