On 8/14/25 23:43, Nicola Vetrini wrote:
> On 2025-08-14 10:36, Jan Beulich wrote:
>> On 13.08.2025 20:27, Dmytro Prokopchuk1 wrote:
>>> ...
>>>
>>> from `vaddr_t' (that is `unsigned long') to `switch_ttbr_fn*' (that
>>> is `void(*)(unsigned long)')
>>>
>>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopch...@epam.com>
>>> ---
>>> 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
>>> +
>
> This check is not quite targeted at this situation, as the behaviour of
> different compilers is a bit of a grey area (even GCC, though that works
> in practice). The relation is mostly aimed at testing whether the
> pointer are represented using the same number of bits as unsigned long
> (which happens to be the case fortunately).
Hi Nicola.
Well, we're telling Eclair the conversion types from() and to(), but can
Eclair determine their sizes (in bits) for particular architecture?
I mean, is it possible to avoid this "sizeof(unsigned long) ==
sizeof(void (*)())" in source code using only Eclair configs?
Dmytro.
>
>>> -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 (*)())'
>>> + on all target platforms.
>>
>> If sizeof(unsigned long) > sizeof(void (*)()), there is loss of
>> information.
>> Unless (not said here) the unsigned long value itself is the result of
>> converting a function pointer to unsigned long. Whether all of that
>> together
>> can be properly expressed to Eclair I don't know. Hence, as Teddy already
>> suggested, == may want specifying instead.
>>
>
> +1; it might be worth to add both the eclair config and the
> BUILD_BUG_ON, noting that neither is sufficient on its own: unless the
> compiler guarantees not to fiddle with the value is unaltered when cast
> back and forth all checks on the number of bits are moot.
>
>>> --- 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);
>>
>> BUILD_BUG_ON() is a statement, not a declaration, and hence wants
>> grouping
>> as such. Question is whether we indeed want to sprinkle such checks all
>> over the code base. (I expect the two cases here aren't all we have.)
>>
>
> +1 as well. I would expect such check to live e.g. in compiler.h or any
> similarly general header, since this is a widespread and largely arch-
> neutral property that Xen wants to be always true I believe.
>
