Re: [XEN PATCH 2/2] Address violation of MISRA C Rule 13.1 involving asm side effects.

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 8/9/25 00:40, Nicola Vetrini wrote:
> The rule states: "Initializer lists shall not contain persistent side 
> effects".
> The specific way in which the 'mrs' instruction is used does not lead to
> visible side effects for the surrounding code.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
> Not yet tested on the Xen ECLAIR runner, as the syntax used in the deviation
> is only supported after updating the runner.
> 
> What the tool is reporting is that due to the '=r' constraint and the
> semantics of the instruction, there is the side effect of writing to '_r',
> but this is not observable outside the stmt expr. The deviation ends up being
> a bit too general for my taste, but the restriction on the actual istruction
> should be enough to limit applicability to cases that are arguably safe in
> practice.
> 
> An alternative approach would be represented by stating that side effects in
> 'READ_SYSREG64' are safe, but this is not true in general.
> ---
>   automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index ec0cac797e5f..6b492e38505d 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -437,6 +437,10 @@ write or not"
>   # Series 13
>   #
>   
> +-doc_begin="Consider the asm instruction to read an Arm system register to 
> have no side effects."
> +-asm_properties+={"asm(any())&&child(text, ast_field(value,^mrs\\s+%0.*$))", 
> {no_side_effect}}
> +-doc_end
> +
>   -doc_begin="All developers and reviewers can be safely assumed to be well 
> aware
>   of the short-circuit evaluation strategy of such logical operators."
>   -config=MC3A2.R13.5,reports+={disapplied,"any()"}

I think it's worth to add example of macro expansion in the commit 
description or asm_properties doc:

uint64_t _r; asm volatile("mrs  %0, ""TPIDR_EL2" : "=r" (_r));

This uses the 'mrs' instruction to read from the TPIDR_EL2 register. 
While this read operation accesses a system register, reading itself 
doesn't cause any persistent side effects, as no program state is modified.

Dmytro