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

Dmytro Prokopchuk1 posted 1 patch 2 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/0e72c83102668dfa6f14c4e8f9839b4a73d30b3d.1760458094.git.dmytro._5Fprokopchuk1@epam.com
There is a newer version of this series
automation/eclair_analysis/ECLAIR/deviations.ecl |  8 ++++++++
docs/misra/deviations.rst                        |  8 +++++++-
docs/misra/rules.rst                             |  7 ++++++-
xen/common/version.c                             | 14 ++++++++++++++
4 files changed, 35 insertions(+), 2 deletions(-)
[PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Dmytro Prokopchuk1 2 weeks, 1 day ago
Rule 11.1 states as following: "Conversions shall not be performed
between a pointer to a function and any other type."

This deviation from Rule 11.1 relies on both ABI definitions and compiler
implementations supported by Xen. The System V x86_64 ABI and the AArch64
ELF ABI 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. Additionally, GCC and Clang, faithfully implement the ABI
specifications, ensuring that the generated machine code conforms to these
guarantees. Developers must note that this behavior is not universal and
depends on platform-specific ABIs and compiler implementations.

Configure Eclair to avoid reporting violations for conversions from
unsigned long or (void *) to a function pointer.

Add a compile-time assertion into the file 'xen/common/version.c' to
confirm this conversion compatibility across X86 and ARM platforms
(assuming this file is common for them).

References:
- System V x86_64 ABI: https://gitlab.com/x86-psABIs/x86-64-ABI/-/jobs/artifacts/master/raw/x86-64-ABI/abi.pdf?job=build
- AArch64 ELF ABI: https://github.com/ARM-software/abi-aa/releases
- GCC: https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html
- Clang: https://clang.llvm.org/docs/CrossCompilation.html

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v3:
- wrote the similar wording as was written for conversions _from_ function pointer types
- limited a compile-time assertions to x86/arm architectures

Link to v2:
https://patchew.org/Xen/b0f269822312a442e87ab02c8deff028b6b040a9.1758787340.git.dmytro._5Fprokopchuk1@epam.com/
---
 automation/eclair_analysis/ECLAIR/deviations.ecl |  8 ++++++++
 docs/misra/deviations.rst                        |  8 +++++++-
 docs/misra/rules.rst                             |  7 ++++++-
 xen/common/version.c                             | 14 ++++++++++++++
 4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 7f3fd35a33..219ba6993b 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -375,6 +375,14 @@ constant expressions are required.\""
 }
 -doc_end
 
+-doc_begin="Conversion from unsigned long or (void *) to a function pointer can restore full information, provided that the source type has enough bits to restore it."
+-config=MC3A2.R11.1,casts+={safe,
+  "from(type(canonical(builtin(unsigned long)||pointer(builtin(void)))))
+   &&to(type(canonical(__function_pointer_types)))
+   &&relation(definitely_preserves_value)"
+}
+-doc_end
+
 -doc_begin="The conversion from a function pointer to a boolean has a well-known semantics that do not lead to unexpected behaviour."
 -config=MC3A2.R11.1,casts+={safe,
   "from(type(canonical(__function_pointer_types)))
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 3271317206..b3431ef24e 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -366,11 +366,17 @@ 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
+     - Conversion from unsigned long or '(void *)' to a function pointer can
+       restore full information, provided that the source type has enough bits
+       to restore it.
+     - Tagged as `safe` for ECLAIR.
+
    * - R11.1
      - The conversion from a function pointer to a boolean has a well-known
        semantics that do not lead to unexpected behaviour.
diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 4388010ec9..4e94251887 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -431,7 +431,12 @@ maintainers if you want to suggest a change.
      - All conversions to integer types are permitted if the destination
        type has enough bits to hold the entire value. Conversions to bool
        and void* are permitted. Conversions from 'void noreturn (*)(...)'
-       to 'void (*)(...)' are permitted.
+       to 'void (*)(...)' are permitted. Conversions from unsigned long or
+       '(void *)' to a function pointer are permitted.
+       Example::
+
+           unsigned long func_addr = (unsigned long)&some_function;
+           void (*restored_func)(void) = (void (*)(void))func_addr;
 
    * - `Rule 11.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_11_02.c>`_
      - Required
diff --git a/xen/common/version.c b/xen/common/version.c
index 553b97ba9b..57a4a74c3d 100644
--- 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__)
+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.
+     */
+
+    BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
+    BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
+}
+#endif
+
 /*
  * Local variables:
  * mode: C
-- 
2.43.0
Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Jan Beulich 2 weeks, 1 day ago
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.

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?

Jan

> +     */
> +
> +    BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
> +    BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
> +}
> +#endif
> +
>  /*
>   * Local variables:
>   * mode: C
Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Nicola Vetrini 1 week, 6 days ago
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.
> 
> 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

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Dmytro Prokopchuk1 1 week ago

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.
  */

BR, Dmytro.
Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Jan Beulich 1 week ago
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

Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Dmytro Prokopchuk1 1 week ago

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.
Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Jan Beulich 1 week ago
On 23.10.2025 15:57, Dmytro Prokopchuk1 wrote:
> 
> 
> 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
> }

I would omit architectures we don't support, though. I gave IA-64 as an
example where things are more complicated (albeit iirc the checks would still
succeed there). However, I didn't expect any trace of it to be added to the
code base (again).

Jan

Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Dmytro Prokopchuk1 6 days, 23 hours ago

On 10/23/25 17:41, Jan Beulich wrote:
> On 23.10.2025 15:57, Dmytro Prokopchuk1 wrote:
>>
>>
>> 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
>> }
> 
> I would omit architectures we don't support, though. I gave IA-64 as an
> example where things are more complicated (albeit iirc the checks would still
> succeed there). However, I didn't expect any trace of it to be added to the
> code base (again).
> 
> Jan

Well, looks like only __powerpc__ matches these criterias.
At least, I see it in 'xen/arch'.

But, this assertion didn't trigger build to fail, when I run CI:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/jobs/11822940884
because PPC64 pointer size is 64-bits (according to the 
C-language-toolchain.rst).

In any case the __powerpc__ is out of scope of certification, so this 
architecture should be excluded.

Dmytro.

Re: [PATCH v3] misra: consider conversion from UL or (void*) to function pointer as safe
Posted by Jan Beulich 6 days, 8 hours ago
On 23.10.2025 18:01, Dmytro Prokopchuk1 wrote:
> On 10/23/25 17:41, Jan Beulich wrote:
>> On 23.10.2025 15:57, Dmytro Prokopchuk1 wrote:
>>> 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
>>>>>>>
>>>>>>>> +     */
>>>>>>>> +
>>>>>>>> +    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).
>>>
>>> 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
>>> }
>>
>> I would omit architectures we don't support, though. I gave IA-64 as an
>> example where things are more complicated (albeit iirc the checks would still
>> succeed there). However, I didn't expect any trace of it to be added to the
>> code base (again).
> 
> Well, looks like only __powerpc__ matches these criterias.
> At least, I see it in 'xen/arch'.
> 
> But, this assertion didn't trigger build to fail, when I run CI:
> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/jobs/11822940884
> because PPC64 pointer size is 64-bits (according to the 
> C-language-toolchain.rst).

Right, because like for ia64 what is being passed around aren't function
pointers, but pointer to the function descriptors.

> In any case the __powerpc__ is out of scope of certification, so this 
> architecture should be excluded.

Not sure here.

Jan