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?
