Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

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

On 2025-06-05 09:17, Jan Beulich wrote:
> On 05.06.2025 01:49, victorm.lira@amd.com wrote:
>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> 
>> Function `reboot_machine' does not return, but lacks the `noreturn' 
>> attribute,
>> therefore causing a violation of MISRA C Rule 2.1: "A project shall 
>> not contain
>> unreachable code".
> 
> Is this (uniformly) true? Looking at ...
> 
>> --- a/xen/common/keyhandler.c
>> +++ b/xen/common/keyhandler.c
>> @@ -251,7 +251,7 @@ static void cf_check dump_hwdom_registers(unsigned 
>> char key)
>>      }
>>  }
>> 
>> -static void cf_check reboot_machine(unsigned char key, bool unused)
>> +static void noreturn cf_check reboot_machine(unsigned char key, bool 
>> unused)
>>  {
>>      printk("'%c' pressed -> rebooting machine\n", key);
>>      machine_restart(0);
> 
> ... generated code here, I can see that the compiler is perfectly able 
> to
> leverage the noreturn that machine_restart() has, resulting in no
> unreachable code to be generated. That is - neither in source nor in
> binary there is any unreachable code. Therefore I'm having a hard time
> seeing what the violation is here.
> 
> That said, I certainly don't mind the addition of the (seemingly) 
> missing
> attribute. Otoh I wonder whether an attribute the removal of which has 
> no
> effect wouldn't count as "dead code" or alike, violating some other 
> rule.
> 

Inlining does not play a role in this case. Here reboot_machine() is 
marked as a violation because machine_restart() is noreturn and there is 
no other path upon which reboot_machine() may return, hence any function 
calling reboot_machine() could have portions that are inadvertently 
unreachable (as in never executed due to divergence) by not having the 
annotation. That said, in such trivial cases compilers are typically 
able to derive the property automatically, but they are not obliged to 
and, more importantly, the behavior may even differ with the same 
compiler using different optimization levels.

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253