All of lore.kernel.org
 help / color / mirror / Atom feed
* [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
@ 2023-12-15  9:26 Federico Serafini
  2023-12-15  9:41 ` Federico Serafini
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Federico Serafini @ 2023-12-15  9:26 UTC (permalink / raw)
  To: xen-devel
  Cc: consulting, Federico Serafini, Simone Ballarin, Doug Goldstein,
	Stefano Stabellini, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Wei Liu

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 that do not give the control back;
  - fallthrough pseudo-keyword;
  - macro BUG();
  - comments.

Update docs/misra/deviations.rst accordingly.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 .../eclair_analysis/ECLAIR/deviations.ecl     | 28 +++++++++++++++++++
 docs/misra/deviations.rst                     | 28 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 683f2bbfe8..e27d840fe4 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -327,6 +327,34 @@ therefore have the same behavior of a boolean"
 -config=MC3R1.R14.4,etypes+={deliberate, "stmt(child(cond,child(expr,ref(^<?domain>?::is_dying$))))","src_type(enum)"}
 -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 ending with a call to a function that does not give
+the control back are safe."
+-config=MC3R1.R16.3,terminals+={safe, "call(property(noreturn))"}
+-doc_end
+
+-doc_begin="Switch clauses ending with pseudo-keyword \"fallthrough\" are
+safe."
+-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/fallthrough;/))))"}
+-doc_end
+
+-doc_begin="Switch clauses ending with failure method \"BUG()\" are safe."
+-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
+-doc_end
+
+-doc_begin="Switch clauses not ending with the break statement are safe if an
+explicit comment indicating the fallthrough intention is present."
+-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
+-doc_end
+
 #
 # Series 20.
 #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index eda3c8100c..d593be81b9 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -276,6 +276,34 @@ Deviations related to MISRA C:2012 Rules:
        therefore have the same behavior of a boolean.
      - Project-wide deviation; tagged as `deliberate` for ECLAIR.
 
+   * - R16.3
+     - Switch clauses ending with continue, goto, return statements are safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R16.3
+     - Switch clauses ending with a call to a function that does not give
+       the control back are safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R16.3
+     - Switch clauses ending with failure method \"BUG()\" are safe.
+     - Tagged as `safe` for ECLAIR.
+
+   * - R16.3
+     - Existing switch clauses not ending with the break statement are safe if
+       an explicit comment indicating the fallthrough intention is present.
+       However, the use of such comments in new code is deprecated:
+       pseudo-keyword "fallthrough" shall be used.
+     - Tagged as `safe` for ECLAIR. The accepted comments are:
+         - /\* fall through \*/
+         - /\* fall through. \*/
+         - /\* fallthrough \*/
+         - /\* fallthrough. \*/
+         - /\* Fall through \*/
+         - /\* Fall through. \*/
+         - /\* Fallthrough \*/
+         - /\* Fallthrough. \*/
+
    * - R20.7
      - Code violating Rule 20.7 is safe when macro parameters are used:
        (1) as function arguments;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-15  9:26 [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3 Federico Serafini
@ 2023-12-15  9:41 ` Federico Serafini
  2023-12-15 21:03 ` Stefano Stabellini
  2023-12-18  7:42 ` Jan Beulich
  2 siblings, 0 replies; 11+ messages in thread
From: Federico Serafini @ 2023-12-15  9:41 UTC (permalink / raw)
  To: xen-devel
  Cc: consulting, Simone Ballarin, Doug Goldstein, Stefano Stabellini,
	Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Wei Liu

On 15/12/23 10:26, 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 that do not give the control back;
>    - fallthrough pseudo-keyword;
>    - macro BUG();
>    - comments.
> 
> Update docs/misra/deviations.rst accordingly.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>   .../eclair_analysis/ECLAIR/deviations.ecl     | 28 +++++++++++++++++++
>   docs/misra/deviations.rst                     | 28 +++++++++++++++++++
>   2 files changed, 56 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 683f2bbfe8..e27d840fe4 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -327,6 +327,34 @@ therefore have the same behavior of a boolean"
>   -config=MC3R1.R14.4,etypes+={deliberate, "stmt(child(cond,child(expr,ref(^<?domain>?::is_dying$))))","src_type(enum)"}
>   -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 ending with a call to a function that does not give
> +the control back are safe."
> +-config=MC3R1.R16.3,terminals+={safe, "call(property(noreturn))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses ending with pseudo-keyword \"fallthrough\" are
> +safe."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/fallthrough;/))))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses ending with failure method \"BUG()\" are safe."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses not ending with the break statement are safe if an
> +explicit comment indicating the fallthrough intention is present."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
> +-doc_end
> +
>   #
>   # Series 20.
>   #
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index eda3c8100c..d593be81b9 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -276,6 +276,34 @@ Deviations related to MISRA C:2012 Rules:
>          therefore have the same behavior of a boolean.
>        - Project-wide deviation; tagged as `deliberate` for ECLAIR.
>   
> +   * - R16.3
> +     - Switch clauses ending with continue, goto, return statements are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Switch clauses ending with a call to a function that does not give
> +       the control back are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Switch clauses ending with failure method \"BUG()\" are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Existing switch clauses not ending with the break statement are safe if
> +       an explicit comment indicating the fallthrough intention is present.
> +       However, the use of such comments in new code is deprecated:
> +       pseudo-keyword "fallthrough" shall be used.
> +     - Tagged as `safe` for ECLAIR. The accepted comments are:
> +         - /\* fall through \*/
> +         - /\* fall through. \*/
> +         - /\* fallthrough \*/
> +         - /\* fallthrough. \*/
> +         - /\* Fall through \*/
> +         - /\* Fall through. \*/
> +         - /\* Fallthrough \*/
> +         - /\* Fallthrough. \*/
> +
>      * - R20.7
>        - Code violating Rule 20.7 is safe when macro parameters are used:
>          (1) as function arguments;

I forgot to mention that this is a V2.
The older version and the discussion can be found at:
https://lists.xenproject.org/archives/html/xen-devel/2023-12/msg00957.html

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-15  9:26 [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3 Federico Serafini
  2023-12-15  9:41 ` Federico Serafini
@ 2023-12-15 21:03 ` Stefano Stabellini
  2023-12-18  7:42 ` Jan Beulich
  2 siblings, 0 replies; 11+ messages in thread
From: Stefano Stabellini @ 2023-12-15 21:03 UTC (permalink / raw)
  To: Federico Serafini
  Cc: xen-devel, consulting, Simone Ballarin, Doug Goldstein,
	Stefano Stabellini, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Wei Liu

On Fri, 15 Dec 2023, 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 that do not give the control back;
>   - fallthrough pseudo-keyword;
>   - macro BUG();
>   - comments.
> 
> Update docs/misra/deviations.rst accordingly.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

This is much sharper and better than before, thanks Federico!


> ---
>  .../eclair_analysis/ECLAIR/deviations.ecl     | 28 +++++++++++++++++++
>  docs/misra/deviations.rst                     | 28 +++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 683f2bbfe8..e27d840fe4 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -327,6 +327,34 @@ therefore have the same behavior of a boolean"
>  -config=MC3R1.R14.4,etypes+={deliberate, "stmt(child(cond,child(expr,ref(^<?domain>?::is_dying$))))","src_type(enum)"}
>  -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 ending with a call to a function that does not give
> +the control back are safe."
> +-config=MC3R1.R16.3,terminals+={safe, "call(property(noreturn))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses ending with pseudo-keyword \"fallthrough\" are
> +safe."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/fallthrough;/))))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses ending with failure method \"BUG()\" are safe."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses not ending with the break statement are safe if an
> +explicit comment indicating the fallthrough intention is present."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
> +-doc_end
> +
>  #
>  # Series 20.
>  #
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index eda3c8100c..d593be81b9 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -276,6 +276,34 @@ Deviations related to MISRA C:2012 Rules:
>         therefore have the same behavior of a boolean.
>       - Project-wide deviation; tagged as `deliberate` for ECLAIR.
>  
> +   * - R16.3
> +     - Switch clauses ending with continue, goto, return statements are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Switch clauses ending with a call to a function that does not give
> +       the control back are safe.

NIT: it might be good to add:

  (noreturn)

to the statement for clarity but it is good enough already


> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Switch clauses ending with failure method \"BUG()\" are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Existing switch clauses not ending with the break statement are safe if
> +       an explicit comment indicating the fallthrough intention is present.
> +       However, the use of such comments in new code is deprecated:
> +       pseudo-keyword "fallthrough" shall be used.

          ^NIT: the pseudo-keyword

both changes could be done on commit

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> +     - Tagged as `safe` for ECLAIR. The accepted comments are:
> +         - /\* fall through \*/
> +         - /\* fall through. \*/
> +         - /\* fallthrough \*/
> +         - /\* fallthrough. \*/
> +         - /\* Fall through \*/
> +         - /\* Fall through. \*/
> +         - /\* Fallthrough \*/
> +         - /\* Fallthrough. \*/
> +
>     * - R20.7
>       - Code violating Rule 20.7 is safe when macro parameters are used:
>         (1) as function arguments;
> -- 
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-15  9:26 [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3 Federico Serafini
  2023-12-15  9:41 ` Federico Serafini
  2023-12-15 21:03 ` Stefano Stabellini
@ 2023-12-18  7:42 ` Jan Beulich
  2023-12-18  8:07   ` Federico Serafini
  2 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2023-12-18  7:42 UTC (permalink / raw)
  To: Federico Serafini
  Cc: consulting, Simone Ballarin, Doug Goldstein, Stefano Stabellini,
	Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel

On 15.12.2023 10:26, Federico Serafini wrote:
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -327,6 +327,34 @@ therefore have the same behavior of a boolean"
>  -config=MC3R1.R14.4,etypes+={deliberate, "stmt(child(cond,child(expr,ref(^<?domain>?::is_dying$))))","src_type(enum)"}
>  -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 ending with a call to a function that does not give
> +the control back are safe."
> +-config=MC3R1.R16.3,terminals+={safe, "call(property(noreturn))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses ending with pseudo-keyword \"fallthrough\" are
> +safe."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/fallthrough;/))))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses ending with failure method \"BUG()\" are safe."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
> +-doc_end
> +
> +-doc_begin="Switch clauses not ending with the break statement are safe if an
> +explicit comment indicating the fallthrough intention is present."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
> +-doc_end
> +
>  #
>  # Series 20.
>  #
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -276,6 +276,34 @@ Deviations related to MISRA C:2012 Rules:
>         therefore have the same behavior of a boolean.
>       - Project-wide deviation; tagged as `deliberate` for ECLAIR.
>  
> +   * - R16.3
> +     - Switch clauses ending with continue, goto, return statements are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Switch clauses ending with a call to a function that does not give
> +       the control back are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Switch clauses ending with failure method \"BUG()\" are safe.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - R16.3
> +     - Existing switch clauses not ending with the break statement are safe if
> +       an explicit comment indicating the fallthrough intention is present.
> +       However, the use of such comments in new code is deprecated:
> +       pseudo-keyword "fallthrough" shall be used.
> +     - Tagged as `safe` for ECLAIR. The accepted comments are:
> +         - /\* fall through \*/
> +         - /\* fall through. \*/
> +         - /\* fallthrough \*/
> +         - /\* fallthrough. \*/
> +         - /\* Fall through \*/
> +         - /\* Fall through. \*/
> +         - /\* Fallthrough \*/
> +         - /\* Fallthrough. \*/

I was puzzled by there being 4 bullet points here, but 5 additions to the
other file. I don't think the wording here is sufficiently unambiguous towards
the use of the pseudo-keyword. If that's to remain a single bullet point, imo
the pseudo-keyword needs mentioning first, and only the talk should be about
comments as an alternative.

Jan


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-18  7:42 ` Jan Beulich
@ 2023-12-18  8:07   ` Federico Serafini
  0 siblings, 0 replies; 11+ messages in thread
From: Federico Serafini @ 2023-12-18  8:07 UTC (permalink / raw)
  To: Jan Beulich
  Cc: consulting, Simone Ballarin, Doug Goldstein, Stefano Stabellini,
	Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel

On 18/12/23 08:42, Jan Beulich wrote:
> On 15.12.2023 10:26, Federico Serafini wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -327,6 +327,34 @@ therefore have the same behavior of a boolean"
>>   -config=MC3R1.R14.4,etypes+={deliberate, "stmt(child(cond,child(expr,ref(^<?domain>?::is_dying$))))","src_type(enum)"}
>>   -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 ending with a call to a function that does not give
>> +the control back are safe."
>> +-config=MC3R1.R16.3,terminals+={safe, "call(property(noreturn))"}
>> +-doc_end
>> +
>> +-doc_begin="Switch clauses ending with pseudo-keyword \"fallthrough\" are
>> +safe."
>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/fallthrough;/))))"}
>> +-doc_end
>> +
>> +-doc_begin="Switch clauses ending with failure method \"BUG()\" are safe."
>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>> +-doc_end
>> +
>> +-doc_begin="Switch clauses not ending with the break statement are safe if an
>> +explicit comment indicating the fallthrough intention is present."
>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
>> +-doc_end
>> +
>>   #
>>   # Series 20.
>>   #
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -276,6 +276,34 @@ Deviations related to MISRA C:2012 Rules:
>>          therefore have the same behavior of a boolean.
>>        - Project-wide deviation; tagged as `deliberate` for ECLAIR.
>>   
>> +   * - R16.3
>> +     - Switch clauses ending with continue, goto, return statements are safe.
>> +     - Tagged as `safe` for ECLAIR.
>> +
>> +   * - R16.3
>> +     - Switch clauses ending with a call to a function that does not give
>> +       the control back are safe.
>> +     - Tagged as `safe` for ECLAIR.
>> +
>> +   * - R16.3
>> +     - Switch clauses ending with failure method \"BUG()\" are safe.
>> +     - Tagged as `safe` for ECLAIR.
>> +
>> +   * - R16.3
>> +     - Existing switch clauses not ending with the break statement are safe if
>> +       an explicit comment indicating the fallthrough intention is present.
>> +       However, the use of such comments in new code is deprecated:
>> +       pseudo-keyword "fallthrough" shall be used.
>> +     - Tagged as `safe` for ECLAIR. The accepted comments are:
>> +         - /\* fall through \*/
>> +         - /\* fall through. \*/
>> +         - /\* fallthrough \*/
>> +         - /\* fallthrough. \*/
>> +         - /\* Fall through \*/
>> +         - /\* Fall through. \*/
>> +         - /\* Fallthrough \*/
>> +         - /\* Fallthrough. \*/
> 
> I was puzzled by there being 4 bullet points here, but 5 additions to the
> other file. I don't think the wording here is sufficiently unambiguous towards
> the use of the pseudo-keyword. If that's to remain a single bullet point, imo
> the pseudo-keyword needs mentioning first, and only the talk should be about
> comments as an alternative.

I'll send a v3 to include Stefano's observations and an
explicit bullet point for pseudo-keyword fallthrough.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-08  0:30   ` Stefano Stabellini
  2023-12-11 10:30     ` Federico Serafini
@ 2023-12-11 12:36     ` Julien Grall
  1 sibling, 0 replies; 11+ messages in thread
From: Julien Grall @ 2023-12-11 12:36 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Federico Serafini, xen-devel, consulting, Simone Ballarin,
	Doug Goldstein, Andrew Cooper, George Dunlap, Jan Beulich,
	Wei Liu

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,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-08  0:30   ` Stefano Stabellini
@ 2023-12-11 10:30     ` Federico Serafini
  2023-12-11 12:36     ` Julien Grall
  1 sibling, 0 replies; 11+ messages in thread
From: Federico Serafini @ 2023-12-11 10:30 UTC (permalink / raw)
  To: Stefano Stabellini, Julien Grall
  Cc: xen-devel, consulting, Simone Ballarin, Doug Goldstein,
	Andrew Cooper, George Dunlap, Jan Beulich, Wei Liu

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.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-07 16:31 ` Julien Grall
@ 2023-12-08  0:30   ` Stefano Stabellini
  2023-12-11 10:30     ` Federico Serafini
  2023-12-11 12:36     ` Julien Grall
  0 siblings, 2 replies; 11+ messages in thread
From: Stefano Stabellini @ 2023-12-08  0:30 UTC (permalink / raw)
  To: Julien Grall
  Cc: Federico Serafini, xen-devel, consulting, Simone Ballarin,
	Doug Goldstein, Stefano Stabellini, Andrew Cooper, George Dunlap,
	Jan Beulich, Wei Liu

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;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-07  9:08 Federico Serafini
  2023-12-07 16:31 ` Julien Grall
@ 2023-12-07 16:34 ` Julien Grall
  1 sibling, 0 replies; 11+ messages in thread
From: Julien Grall @ 2023-12-07 16:34 UTC (permalink / raw)
  To: Federico Serafini, xen-devel
  Cc: consulting, Simone Ballarin, Doug Goldstein, Stefano Stabellini,
	Andrew Cooper, George Dunlap, Jan Beulich, Wei Liu

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,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
  2023-12-07  9:08 Federico Serafini
@ 2023-12-07 16:31 ` Julien Grall
  2023-12-08  0:30   ` Stefano Stabellini
  2023-12-07 16:34 ` Julien Grall
  1 sibling, 1 reply; 11+ messages in thread
From: Julien Grall @ 2023-12-07 16:31 UTC (permalink / raw)
  To: Federico Serafini, xen-devel
  Cc: consulting, Simone Ballarin, Doug Goldstein, Stefano Stabellini,
	Andrew Cooper, George Dunlap, Jan Beulich, Wei Liu

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3
@ 2023-12-07  9:08 Federico Serafini
  2023-12-07 16:31 ` Julien Grall
  2023-12-07 16:34 ` Julien Grall
  0 siblings, 2 replies; 11+ messages in thread
From: Federico Serafini @ 2023-12-07  9:08 UTC (permalink / raw)
  To: xen-devel
  Cc: consulting, Federico Serafini, Simone Ballarin, Doug Goldstein,
	Stefano Stabellini, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Wei Liu

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.
+     - Tagged as `safe` for ECLAIR.
+
    * - R20.7
      - Code violating Rule 20.7 is safe when macro parameters are used:
        (1) as function arguments;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-12-18  8:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-15  9:26 [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.3 Federico Serafini
2023-12-15  9:41 ` Federico Serafini
2023-12-15 21:03 ` Stefano Stabellini
2023-12-18  7:42 ` Jan Beulich
2023-12-18  8:07   ` Federico Serafini
  -- strict thread matches above, loose matches on Subject: below --
2023-12-07  9:08 Federico Serafini
2023-12-07 16:31 ` Julien Grall
2023-12-08  0:30   ` Stefano Stabellini
2023-12-11 10:30     ` Federico Serafini
2023-12-11 12:36     ` Julien Grall
2023-12-07 16:34 ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.