automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

[XEN PATCH] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

Posted by Federico Serafini 3 weeks, 5 days ago

Update ECLAIR configuration to deviate uses of chk_fld and
alternative_vcall[0-9] macros.

Suggested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl |  8 ++++++++
 docs/misra/deviations.rst                        | 12 ++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index de9ba723fb..eebe779000 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -333,6 +333,14 @@ of the short-circuit evaluation strategy of such logical operators."
 -config=MC3R1.R13.5,reports+={disapplied,"any()"}
 -doc_end
 
+-doc_begin="Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the func parameters without evaluating them."
+-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))))"}
+-doc_end
+
+-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where by code inspection you can see that its usage is correct. The BUILD_BUG_ON checks check that EFI_TIME and struct xenpf_efi_time fields match."
+-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^chk_fld$))))"}
+-doc_end
+
 #
 # Series 14
 #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index eb5ef2bd9d..a97c31d18f 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -279,6 +279,18 @@ Deviations related to MISRA C:2012 Rules:
        the short-circuit evaluation strategy for logical operators.
      - Project-wide deviation; tagged as `disapplied` for ECLAIR.
 
+   * - R13.6
+     - Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the
+       func parameters without evaluating them.
+     - Tagged as `deliberate` for ECLAIR.
+
+   * - R13.6
+     - Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very
+       specific cases where by code inspection you can see that its usage is
+       correct. The BUILD_BUG_ON checks check that EFI_TIME and
+       struct xenpf_efi_time fields match.
+     - Tagged as `deliberate` for ECLAIR.
+
    * - R14.2
      - The severe restrictions imposed by this rule on the use of 'for'
        statements are not counterbalanced by the presumed facilitation of the
-- 
2.34.1

Re: [XEN PATCH] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

Posted by Jan Beulich 3 weeks, 5 days ago

On 04.04.2024 16:21, Federico Serafini wrote:
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -333,6 +333,14 @@ of the short-circuit evaluation strategy of such logical operators."
>  -config=MC3R1.R13.5,reports+={disapplied,"any()"}
>  -doc_end
>  
> +-doc_begin="Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the func parameters without evaluating them."
> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))))"}
> +-doc_end

alternative_vcall(), for now at least, is x86-only. Why blindly deviate it also
for Arm?

> +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where by code inspection you can see that its usage is correct. The BUILD_BUG_ON checks check that EFI_TIME and struct xenpf_efi_time fields match."
> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^chk_fld$))))"}
> +-doc_end

As indicated on earlier occasions - when a macro isn't global, its deviation
better wouldn't be global either. An identically named macro may be introduced
elsewhere, and may not need deviating there.

> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -279,6 +279,18 @@ Deviations related to MISRA C:2012 Rules:
>         the short-circuit evaluation strategy for logical operators.
>       - Project-wide deviation; tagged as `disapplied` for ECLAIR.
>  
> +   * - R13.6
> +     - Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the
> +       func parameters without evaluating them.

Just to mention it: The return type of the function isn't really of interest
there. What's being checked is that the argument types match the corresponding
parameter ones, which otherwise would "naturally" be done by the compiler when
seeing function calls. Might be useful to state no more and no less than what
is really needed.

Further, according to the discussion on the matter: alternative_call() is
quite similar in this regard, just that it uses typeof(). Imo it should be
deviated here as well right away, no matter whether Eclair can actually be
configured to also check typeof() and alignof().

Jan

Re: [XEN PATCH] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

Posted by Federico Serafini 3 weeks, 4 days ago

On 04/04/24 16:34, Jan Beulich wrote:
> On 04.04.2024 16:21, Federico Serafini wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -333,6 +333,14 @@ of the short-circuit evaluation strategy of such logical operators."
>>   -config=MC3R1.R13.5,reports+={disapplied,"any()"}
>>   -doc_end
>>   
>> +-doc_begin="Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the func parameters without evaluating them."
>> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))))"}
>> +-doc_end
> 
> alternative_vcall(), for now at least, is x86-only. Why blindly deviate it also
> for Arm?

I'll reduce the scope to x86 only.

>> +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where by code inspection you can see that its usage is correct. The BUILD_BUG_ON checks check that EFI_TIME and struct xenpf_efi_time fields match."
>> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^chk_fld$))))"}
>> +-doc_end
> 
> As indicated on earlier occasions - when a macro isn't global, its deviation
> better wouldn't be global either. An identically named macro may be introduced
> elsewhere, and may not need deviating there.

Noted.

>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -279,6 +279,18 @@ Deviations related to MISRA C:2012 Rules:
>>          the short-circuit evaluation strategy for logical operators.
>>        - Project-wide deviation; tagged as `disapplied` for ECLAIR.
>>   
>> +   * - R13.6
>> +     - Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the
>> +       func parameters without evaluating them.
> 
> Just to mention it: The return type of the function isn't really of interest
> there. What's being checked is that the argument types match the corresponding
> parameter ones, which otherwise would "naturally" be done by the compiler when
> seeing function calls. Might be useful to state no more and no less than what
> is really needed.

Noted.

-- 
Federico Serafini, M.Sc.

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

Re: [XEN PATCH] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

Posted by Stefano Stabellini 3 weeks, 5 days ago

On Thu, 4 Apr 2024, Jan Beulich wrote:
> Further, according to the discussion on the matter: alternative_call() is
> quite similar in this regard, just that it uses typeof(). Imo it should be
> deviated here as well right away, no matter whether Eclair can actually be
> configured to also check typeof() and alignof().

On this specific point regarding typeof, I agree but it should be done
as a separate rule (not 13.6), to make sure it matches the scanner
results. It looks like the new rule will be called B.MISLEFF.

Re: [XEN PATCH] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

Posted by Jan Beulich 3 weeks, 5 days ago

On 05.04.2024 02:04, Stefano Stabellini wrote:
> On Thu, 4 Apr 2024, Jan Beulich wrote:
>> Further, according to the discussion on the matter: alternative_call() is
>> quite similar in this regard, just that it uses typeof(). Imo it should be
>> deviated here as well right away, no matter whether Eclair can actually be
>> configured to also check typeof() and alignof().
> 
> On this specific point regarding typeof, I agree but it should be done
> as a separate rule (not 13.6), to make sure it matches the scanner
> results. It looks like the new rule will be called B.MISLEFF. 

Fine with me, as long as things stay together (i.e. in a single commit).

Jan

Re: [XEN PATCH] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6

Posted by Federico Serafini 3 weeks, 4 days ago

On 05/04/24 08:06, Jan Beulich wrote:
> On 05.04.2024 02:04, Stefano Stabellini wrote:
>> On Thu, 4 Apr 2024, Jan Beulich wrote:
>>> Further, according to the discussion on the matter: alternative_call() is
>>> quite similar in this regard, just that it uses typeof(). Imo it should be
>>> deviated here as well right away, no matter whether Eclair can actually be
>>> configured to also check typeof() and alignof().
>>
>> On this specific point regarding typeof, I agree but it should be done
>> as a separate rule (not 13.6), to make sure it matches the scanner
>> results. It looks like the new rule will be called B.MISLEFF.
> 
> Fine with me, as long as things stay together (i.e. in a single commit).

Noted.

-- 
Federico Serafini, M.Sc.

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