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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.09.2025 20:37, Dmytro Prokopchuk1 wrote:
> On 9/25/25 16:25, Jan Beulich wrote:
>> On 25.09.2025 10:04, Dmytro Prokopchuk1 wrote:
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -366,11 +366,22 @@ Deviations related to MISRA C:2012 Rules:
>>>        - Tagged as `safe` for ECLAIR.
>>>   
>>>      * - R11.1
>>> -     - The conversion from a function pointer to unsigned long or (void 
>>> \*) does
>>> +     - The conversion from a function pointer to unsigned long or '(void 
>>> *)' does
>>>          not lose any information, provided that the target type has enough 
>>> bits
>>>          to store it.
>>>        - Tagged as `safe` for ECLAIR.
>>>   
>>> +   * - R11.1
>>> +     - The conversion from unsigned long or '(void *)' to a function 
>>> pointer is
>>> +       safe because it relies on both ABI definitions and compiler 
>>> implementations
>>> +       supported by Xen which define consistent and compatible 
>>> representations
>>> +       (i.e., having the same size and memory layout) for '(void *)', 
>>> unsigned
>>> +       long, and function pointers, enabling safe conversions between 
>>> these types
>>> +       without data loss or corruption. The compile-time assertions 
>>> (BUILD_BUG_ON
>>> +       macro) is integrated into 'xen/common/version.c' to confirm 
>>> conversions
>>> +       compatibility across all target platforms.
>>
>> As you use (and mean) plural, s/is/are/ ? I also think the "The" at the start
>> of the sentence wants dropping.
> Ok.
> 
>>
>> Further, why this very dissimilar wording compared to what's said about
>> conversions _from_ function pointer types?
> Do you mean the following wording should be placed instead (to be 
> similar with previous one)?
> 
> "Conversions from unsigned long or (void *) to a function pointer do not 
> lose any information, provided that the source type has enough bits to 
> restore it."
> 
> And wording about "ABI, compiler..." should be only in commit message?

Perhaps.

>> And then ...
>>
>>> --- a/xen/common/version.c
>>> +++ b/xen/common/version.c
>>> @@ -217,6 +217,17 @@ void __init xen_build_init(void)
>>>   #endif /* CONFIG_X86 */
>>>   }
>>>   #endif /* BUILD_ID */
>>> +
>>> +static void __init __maybe_unused build_assertions(void)
>>> +{
>>> +    /*
>>> +     * To confirm conversion compatibility between unsigned long, (void *)
>>> +     * and function pointers for all supported architectures.
>>> +     */
>>> +    BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
>>> +    BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
>>> +}
>>
>> ... I'm unconvinced checking merely the sizes is sufficient. On architectures
>> involving function descriptors (e.g. ia64) converting in this direction is
>> safe only if earlier on the value was obtained as the result of a conversion
>> in the opposite direction (and all of this within a single component, which
>> of course is guaranteed for Xen).
> As I know mainline Xen doesn't support IA-64 currently (this support was 
> dropped).
> Why we still need to mention about IA-64 here?

Because I needed to use an example I know. Aiui there are other architectures
which use function descriptors (or alike).

> Anyway...
> Yes, this deviation wouldn't work with architectures where the 
> representation of a function involves more than just its address (e.g. 
> IA-64). If not proved that such conversion is symmetric.
> 
> Probably, additional guard may be added below to exclude such 
> architectures (e.g. IA-64):
> 
> static void __init __maybe_unused build_assertions(void)
> {
> #if defined (__IA64__) || defined (__ia64__)
> #error "Conversions to function pointer isn't safe -  architecture uses 
> function descriptors."
> #endif

Well, no, I didn't mean to ask that you add dead code.

>      BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
>      BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
> }
> 
> But if someone really will try to run Xen on such platform, the build 
> will fail.
> 
> Or just mention explicitly that other architectures (e.g., IA-64) might 
> not be safe for such conversions?

My main point really is that once again I wonder how convincing such an
argument would be to assessors, when it's clearly not generic (yet being
worded and the checking coded as if it was).

Jan