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

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

On 2025-09-26 08:46, Jan Beulich wrote:
> 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

Well, it is true that the intended scope of those deviations is for the 
architectures and compilers that are subject to the analysis, because 
adding a new architecture or compiler to the mix would mean that all the 
assumptions need to be re-evaluated for that compiler/arch (this is an 
IDB in the first place, so it is unlikely that a general statement can 
be made). Perhaps the BUILD_BUG_ON should be limited to these 
arch-es/compilers, so that there is little doubt about the intended 
motivation of the check.

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