Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 10/23/25 13:23, Jan Beulich wrote:
> On 23.10.2025 12:00, Dmytro Prokopchuk1 wrote:
>> On 10/17/25 10:09, Nicola Vetrini wrote:
>>> On 2025-10-15 08:20, Jan Beulich wrote:
>>>> On 14.10.2025 18:16, Dmytro Prokopchuk1 wrote:
>>>>> --- a/xen/common/version.c
>>>>> +++ b/xen/common/version.c
>>>>> @@ -217,6 +217,20 @@ void __init xen_build_init(void)
>>>>>   #endif /* CONFIG_X86 */
>>>>>   }
>>>>>   #endif /* BUILD_ID */
>>>>> +
>>>>> +#if defined(__i386__) || defined(__x86_64__) || defined(__arm__) ||
>>>>> defined(__aarch64__)
>>>>
>>>> Why __i386__? Also (nit): Line too long.
>>
>> Well, I copied this line from Xen codebase,
>> but yeah, __i386__ is outdated now.
>> I'll remove it.
>>
>>>>
>>>> And why this restriction without any comment here or ...
>>>>
>>>>> +static void __init __maybe_unused build_assertions(void)
>>>>> +{
>>>>> +    /*
>>>>> +     * To confirm conversion compatibility between unsigned long,
>>>>> (void *)
>>>>> +     * and function pointers for X86 and ARM architectures only.
>>>>
>>>> ... explanation here? More generally - how would people know to update
>>>> the condition if another port was to be certified?
>>>>
>>>> Finally, with the v3 addition here, is Nicola's R-b really still
>>>> applicable?
>>>>
>>>
>>> I agree with the point you make about i386 (e.g., C-language-
>>> toolchain.rst may be mentioned to provide some context about the
>>> preprocessor guard); that said, my R-by can be retained
>>>
>>>> Jan
>>>>
>>>>> +     */
>>>>> +
>>>>> +    BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
>>>>> +    BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>>   /*
>>>>>    * Local variables:
>>>>>    * mode: C
>>>
>>
>> And probably v4 can have the following wording:
>>
>> /*
>>    * This assertion checks compatibility between 'unsigned long', 'void *',
>>    * and function pointers. This is true for X86 (x86_64) and ARM (arm,
>> aarch64)
>>    * architectures, which is why the check is restricted to these.
>>    *
>>    * For more context on architecture-specific preprocessor guards, see
>>    * docs/misc/C-language-toolchain.rst.
>>    *
>>    * If Xen is ported to a new architecture, verify that this
>> compatibility holds
>>    * before adding its macro to the condition below. If the compatibility
>> does not
>>    * hold, this assertion may need to be revised or removed for that
>> architecture.
>>    */
> 
> Except that this doesn't address my concern. Imo the checks want to be there
> unconditionally, and ports where they're _not_ applicable would then need
> excluding (with suitable commentary and/or alternative checks).
> 
> Jan

Ok, below is the updated logic:

/*
  * This assertion checks compatibility between 'unsigned long', 'void *',
  * and function pointers. This is true for most supported architectures,
  * including X86 (x86_64) and ARM (arm, aarch64).
  *
  * For more context on architecture-specific preprocessor guards, see
  * docs/misc/C-language-toolchain.rst.
  *
  * If porting Xen to a new architecture where this compatibility does 
not hold,
  * exclude that architecture from these checks and provide suitable 
commentary
  * and/or alternative checks as appropriate.
  */
static void __init __maybe_unused build_assertions(void)
{
     /*
      * Exclude architectures where function pointers are larger than 
data pointers:
      * - IA-64: uses 'fat' function pointers (code address + global 
pointer)
      */
#if !defined(__ia64__)
     BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
     BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
#endif
}

Dmytro.