Re: [RFC PATCH] misra: allow conversion from unsigned long to function pointer

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Hello,

Le 13/08/2025 à 20:30, Dmytro Prokopchuk1 a écrit :
> ...
>
> from `vaddr_t' (that is `unsigned long') to `switch_ttbr_fn*' (that is 
> `void(*)(unsigned long)')
>
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
> ---
> This is just a RFC patch.
> The commit message is not important at this stage.
>
> I am seeking comments regarding this case.
>
> Thanks.
> ---
>   automation/eclair_analysis/ECLAIR/deviations.ecl |  8 ++++++++
>   docs/misra/deviations.rst                        | 10 ++++++++++
>   docs/misra/rules.rst                             |  8 +++++++-
>   xen/arch/arm/arm64/mmu/mm.c                      |  2 ++
>   4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index ebce1ceab9..f9fd6076b7 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -365,6 +365,14 @@ constant expressions are required.\""
>   }
>   -doc_end
>
> +-doc_begin="The conversion from unsigned long to a function pointer does not 
> lose any information, provided that the source type has enough bits to 
> restore it."
> +-config=MC3A2.R11.1,casts+={safe,
> +  "from(type(canonical(builtin(unsigned long))))
> +   &&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 3c46a1e47a..27848602f6 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -348,6 +348,16 @@ Deviations related to MISRA C:2012 Rules:
>          to store it.
>        - Tagged as `safe` for ECLAIR.
>
> +   * - R11.1
> +     - The conversion from unsigned long to a function pointer does not lose 
> any
> +       information or violate type safety assumptions if the unsigned long 
> type
> +       is guaranteed to be at least as large as a function pointer. This 
> ensures
> +       that the function pointer address can be fully represented without
> +       truncation or corruption. Macro BUILD_BUG_ON can be integrated into 
> the
> +       build system to confirm that 'sizeof(unsigned long) >= sizeof(void 
> (*)())'

Wouldn't `sizeof(unsigned long) == sizeof(void (*)())` be preferable ?

I assume sizeof(unsigned long) is the size of a CPU word.
Having `sizeof(unsigned long) < sizeof(void (*)())` makes use of
operations like cmpxchg unsuitable on function pointers (because of
object size mismatch).

> +       on all target platforms.
> +     - 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 6812eb7e8a..8b97ecf3f4 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -414,7 +414,13 @@ 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 to
> +       function pointer are permitted if the unsigned long type has a size
> +       and representation sufficient to store the entire function pointer
> +       value without truncation or corruption. 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/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
> index 3e64be6ae6..998d52c162 100644
> --- a/xen/arch/arm/arm64/mmu/mm.c
> +++ b/xen/arch/arm/arm64/mmu/mm.c
> @@ -150,6 +150,7 @@ void __init relocate_and_switch_ttbr(uint64_t ttbr)
>       vaddr_t id_addr = virt_to_maddr(relocate_xen);
>       relocate_xen_fn *fn = (relocate_xen_fn *)id_addr;
>       lpae_t pte;
> +    BUILD_BUG_ON(sizeof(unsigned long) < sizeof(fn));
>
>       /* Enable the identity mapping in the boot page tables */
>       update_identity_mapping(true);
> @@ -178,6 +179,7 @@ void __init switch_ttbr(uint64_t ttbr)
>       vaddr_t id_addr = virt_to_maddr(switch_ttbr_id);
>       switch_ttbr_fn *fn = (switch_ttbr_fn *)id_addr;
>       lpae_t pte;
> +    BUILD_BUG_ON(sizeof(unsigned long) < sizeof(fn));
>
>       /* Enable the identity mapping in the boot page tables */
>       update_identity_mapping(true);



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech