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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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@bugseng.com>
>> ---
>> 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