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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.10.2025 19:51, Dmytro Prokopchuk1 wrote:
> 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@xxxxxxxx>
> Reviewed-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
> Changes in v4:
> - the build assertions for the X86 and ARM are enabled by default 
> (unconditionally)
> - re-wrote description for the build_assertions() function
> - excluded PowerPC architecture (not in scope of certification) from the 
> check and wrote apropriate explanation
> 
> Link to v3:
> https://patchew.org/Xen/0e72c83102668dfa6f14c4e8f9839b4a73d30b3d.1760458094.git.dmytro._5Fprokopchuk1@xxxxxxxx/
> ---
>  .../eclair_analysis/ECLAIR/deviations.ecl     |  8 ++++++
>  docs/misra/deviations.rst                     |  8 +++++-
>  docs/misra/rules.rst                          |  7 +++++-
>  xen/common/version.c                          | 25 +++++++++++++++++++
>  4 files changed, 46 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..674bd72b31 100644
> --- a/xen/common/version.c
> +++ b/xen/common/version.c
> @@ -217,6 +217,31 @@ void __init xen_build_init(void)
>  #endif /* CONFIG_X86 */
>  }
>  #endif /* BUILD_ID */
> +
> +/*
> + * 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:
> +     * - PowerPC: uses function descriptors (code address + TOC pointer).
> +     */

Yes, it uses function descriptors (aiui they consist of three longs, not just
two though), but "function pointers are larger than data pointers" is still
wrong there, which is why (as you indicated before) ...

> +#if !defined(__powerpc__)
> +    BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void (*)(void)));
> +    BUILD_BUG_ON(sizeof(void *) != sizeof(void (*)(void)));
> +#endif

... these checks still will not cause build breakage even if enabled for PPC.
At least aiui (what is being passed around are pointers to function
descriptors). IOW I don't view it as justified to exclude PPC here, at least
not yet.

Jan