On 10.12.2025 01:57, Stefano Stabellini wrote: > On Tue, 28 Oct 2025, Jan Beulich wrote: >> 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 <[email protected]> >>> Reviewed-by: Nicola Vetrini <[email protected]> >>> --- >>> 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._5fprokopch...@epam.com/ >>> --- >>> .../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. > > Could the patch be committed without the #if !defined(__powerpc__) / > #endif chunk considering that according to Dmytro it should pass the > pipeline anyway?
And with the comment also dropped (or adjusted accordingly). Jan
