diff mbox series

[XEN] automation/eclair: add deviations for MISRA C:2012 Rule 16.3

Message ID 4fa08aaed77d2b68db39ce51beef29820f1ba9b0.1701940034.git.federico.serafini@bugseng.com (mailing list archive)
State Superseded
Headers show
Series [XEN] automation/eclair: add deviations for MISRA C:2012 Rule 16.3 | expand

Commit Message

Federico Serafini Dec. 7, 2023, 9:08 a.m. UTC
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.serafini@bugseng.com>
---
 .../eclair_analysis/ECLAIR/deviations.ecl     | 18 ++++++++++++++
 docs/misra/deviations.rst                     | 24 +++++++++++++++++++
 2 files changed, 42 insertions(+)

Comments

Julien Grall Dec. 7, 2023, 4:31 p.m. UTC | #1
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.serafini@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?

> +        - 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...

> +
> +   * - 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;

Cheers,
Julien Grall Dec. 7, 2023, 4:34 p.m. UTC | #2
Hi again,

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.serafini@bugseng.com>
> ---
>   .../eclair_analysis/ECLAIR/deviations.ecl     | 18 ++++++++++++++
>   docs/misra/deviations.rst                     | 24 +++++++++++++++++++
>   2 files changed, 42 insertions(+)
> 
> 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))"}
> +-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
> +        - BUG_ON
> +
> +   * - 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.

One more thing. This is not explicit which comment should be added. But 
would should deprecate the comment in favor of "fallthrough".

The deviation should have it written down (similar to SAF-1 for rule 8.4).

Cheers,
Stefano Stabellini Dec. 8, 2023, 12:30 a.m. UTC | #3
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.serafini@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;
Federico Serafini Dec. 11, 2023, 10:30 a.m. UTC | #4
On 08/12/23 01:30, Stefano Stabellini wrote:
> 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.serafini@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;

Thank you for your suggestions,
I will formulate a new version of the deviation.
Julien Grall Dec. 11, 2023, 12:36 p.m. UTC | #5
Hi Stefano,

On 08/12/2023 00:30, Stefano Stabellini wrote:
> 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.serafini@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__))

Whoops yes.

> 
> 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.

In general, I really dislike deviate by name because this is assuming 
that the functions name are uniq across all the architecture. For 
instance, fatal_trap() is x86 specific whilst do_unexpected_trap() is 
arm specific.

In this case, the name of the function clearly indicates that they are 
never expected to return. So we are ok.

But I would strongly prefer if we rely on property attribute rather than 
names whenever it is possible.

Cheers,
diff mbox series

Patch

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))"}
+-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
+        - BUG_ON
+
+   * - 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;