[XEN] automation/eclair: configure Rule 13.6 and custom service B.UNEVALEFF

Commit Message

Federico Serafini June 22, 2024, 10:48 a.m. UTC

Rule 13.6 states that "The operand of the `sizeof' operator shall not
contain any expression which has potential side effects".

Define service B.UNEVALEFF as an extension of Rule 13.6 to
check for unevalued side effects also for typeof and alignof operators.

Update ECLAIR configuration to deviate uses of macro chk_fld for
Rule 13.6 and alternative_v?call[0-9] for both Rule 13.6 and
B.UNEVALEFF.

Add service B.UNEVALEFF to the accepted.ecl guidelines and check
"violations" in the weekly analysis.

Signed-off-by: Federico Serafini <[e-mail]>
Signed-off-by: Nicola Vetrini <[e-mail]>
---
 automation/eclair_analysis/ECLAIR/B.UNEVALEFF.ecl    | 10 ++++++++++
 .../eclair_analysis/ECLAIR/accepted_guidelines.sh    |  2 ++
 automation/eclair_analysis/ECLAIR/analysis.ecl       |  1 +
 automation/eclair_analysis/ECLAIR/deviations.ecl     | 10 ++++++++++
 docs/misra/deviations.rst                            | 12 ++++++++++++
 5 files changed, 35 insertions(+)
 create mode 100644 automation/eclair_analysis/ECLAIR/B.UNEVALEFF.ecl

Comments

Andrew Cooper June 22, 2024, 12:22 p.m. UTC | #1

On 22/06/2024 11:48 am, Federico Serafini wrote:
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index e2653f77eb..6bdfe7c883 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -328,6 +328,16 @@ of the short-circuit evaluation strategy of such logical operators."
>  -config=MC3R1.R13.5,reports+={disapplied,"any()"}
>  -doc_end
>  
> +-doc_begin="Macros alternative_v?call[0-9] use sizeof and typeof to check that th argument types match the corresponding parameter ones."

Typo "th" => "the".  Can be fixed on commit.

> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))&&file(^xen/arch/x86.*$)))"}
> +-config=B.UNEVALEFF,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_v?call[0-9]$))&&file(^xen/arch/x86.*$)))"}

There will be expansions of these macros outside of arch/x86/. 
drivers/iommu/ as an example.

Is the file() clause targetting the source of the macro (in which it
probably wants making more specific to alternative_call.h), or the
expansions of the macro (in which case it probably wants dropping
entirely) ?

> +-doc_end
> +
> +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where the usage is correct and can be checked by code inspection.
> +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$))&&file(^xen/common/efi/runtime\\.c$)))"}
> +-doc_end

It's just occurred to me.  Anything, no matter how complicated, inside a
BUILD_BUG_ON() is clearly a compile time thing so has no relevant side
effects.

Is it possible to generally exclude any sizeof/alignof/typeof inside a
BUILD_BUG_ON()?  That would be better than identifying every user locally.

~Andrew

Federico Serafini June 24, 2024, 7:20 a.m. UTC | #2

On 22/06/24 14:22, Andrew Cooper wrote:
> On 22/06/2024 11:48 am, Federico Serafini wrote:
>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> index e2653f77eb..6bdfe7c883 100644
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -328,6 +328,16 @@ of the short-circuit evaluation strategy of such logical operators."
>>   -config=MC3R1.R13.5,reports+={disapplied,"any()"}
>>   -doc_end
>>   
>> +-doc_begin="Macros alternative_v?call[0-9] use sizeof and typeof to check that th argument types match the corresponding parameter ones."
> 
> Typo "th" => "the".  Can be fixed on commit.
> 
>> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))&&file(^xen/arch/x86.*$)))"}
>> +-config=B.UNEVALEFF,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_v?call[0-9]$))&&file(^xen/arch/x86.*$)))"}
> 
> There will be expansions of these macros outside of arch/x86/.
> drivers/iommu/ as an example.
> 
> Is the file() clause targetting the source of the macro (in which it
> probably wants making more specific to alternative_call.h), or the
> expansions of the macro (in which case it probably wants dropping
> entirely) ?

It is targetting the source of the macro,
we can make it more specific.


> 
>> +-doc_end
>> +
>> +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where the usage is correct and can be checked by code inspection.
>> +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$))&&file(^xen/common/efi/runtime\\.c$)))"}
>> +-doc_end
> 
> It's just occurred to me.  Anything, no matter how complicated, inside a
> BUILD_BUG_ON() is clearly a compile time thing so has no relevant side
> effects.
> 
> Is it possible to generally exclude any sizeof/alignof/typeof inside a
> BUILD_BUG_ON()?  That would be better than identifying every user locally.

Sure.

I will send a v2 with your observations.

Patch

diff --git a/automation/eclair_analysis/ECLAIR/B.UNEVALEFF.ecl b/automation/eclair_analysis/ECLAIR/B.UNEVALEFF.ecl
new file mode 100644
index 0000000000..92d8db8986
--- /dev/null
+++ b/automation/eclair_analysis/ECLAIR/B.UNEVALEFF.ecl
@@ -0,0 +1,10 @@ 
+-clone_service=MC3R1.R13.6,B.UNEVALEFF
+
+-config=B.UNEVALEFF,summary="The operand of the `alignof' and `typeof'  operators shall not contain any expression which has potential side effects"
+-config=B.UNEVALEFF,stmt_child_matcher=
+{"stmt(node(utrait_expr)&&operator(alignof))", expr, 0, "stmt(any())", {}},
+{"stmt(node(utrait_type)&&operator(alignof))", type, 0, "stmt(any())", {}},
+{"stmt(node(utrait_expr)&&operator(preferred_alignof))", expr, 0, "stmt(any())", {}},
+{"stmt(node(utrait_type)&&operator(preferred_alignof))", type, 0, "stmt(any())", {}},
+{"type(node(typeof_expr))", expr, 0, "stmt(any())", {}},
+{"type(node(typeof_type))", type, 0, "stmt(any())", {}}
diff --git a/automation/eclair_analysis/ECLAIR/accepted_guidelines.sh b/automation/eclair_analysis/ECLAIR/accepted_guidelines.sh
index b308bd4cda..368135122c 100755
--- a/automation/eclair_analysis/ECLAIR/accepted_guidelines.sh
+++ b/automation/eclair_analysis/ECLAIR/accepted_guidelines.sh
@@ -11,3 +11,5 @@  accepted_rst=$1
 
 grep -Eo "\`(Dir|Rule) [0-9]+\.[0-9]+" ${accepted_rst} \
      | sed -e 's/`Rule /MC3R1.R/' -e  's/`Dir /MC3R1.D/' -e 's/.*/-enable=&/' > ${script_dir}/accepted.ecl
+
+echo "-enable=B.UNEVALEFF" >> ${script_dir}/accepted.ecl
diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl b/automation/eclair_analysis/ECLAIR/analysis.ecl
index 9134e59617..df0b551812 100644
--- a/automation/eclair_analysis/ECLAIR/analysis.ecl
+++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
@@ -52,6 +52,7 @@  their Standard Library equivalents."
 -eval_file=adopted.ecl
 -eval_file=out_of_scope.ecl
 
+-eval_file=B.UNEVALEFF.ecl
 -eval_file=deviations.ecl
 -eval_file=call_properties.ecl
 -eval_file=tagging.ecl
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index e2653f77eb..6bdfe7c883 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -328,6 +328,16 @@  of the short-circuit evaluation strategy of such logical operators."
 -config=MC3R1.R13.5,reports+={disapplied,"any()"}
 -doc_end
 
+-doc_begin="Macros alternative_v?call[0-9] use sizeof and typeof to check that th argument types match the corresponding parameter ones."
+-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))&&file(^xen/arch/x86.*$)))"}
+-config=B.UNEVALEFF,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_v?call[0-9]$))&&file(^xen/arch/x86.*$)))"}
+-doc_end
+
+-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where the usage is correct and can be checked by code inspection.
+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$))&&file(^xen/common/efi/runtime\\.c$)))"}
+-doc_end
+
 #
 # Series 14
 #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 36959aa44a..122e779f30 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
+     - On x86, macros alternative_vcall[0-9] use sizeof to type-check the
+       aguments of \"func\" 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