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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-08-13 09:41, Dmytro Prokopchuk1 wrote:
> 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.
Definitely not in the -doc_begin, perhaps in deviations.rst, though in 
reality it is a single case this currently applies to. Reading the 
register is not the reason why this deviation was requested, but the 
write with the "=r" constraint on "_r", as that is the side effect the 
tool is pointing at.
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253