...

from `vaddr_t' (that is `unsigned long') to `switch_ttbr_fn*' (that is `void(*)(unsigned long)')

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@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
+
 -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.
+     - 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);
-- 
2.43.0

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_prokopchuk1@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
> +
>  -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.

> --- 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.)

Jan

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_prokopchuk1@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).

>>  -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.

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

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_prokopchuk1@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.
> 

On 2025-08-18 12:16, Dmytro Prokopchuk1 wrote:
> 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_prokopchuk1@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.
> 

Unfortunately no. ECLAIR knowns the number of bytes used to represent 
pointer and unsigned long, but what it cannot tell is whether the bits 
are preserved after being converted. What we can do, as it was done 
here, is provide a written justification that this is indeed the case 
for the toolchain we care about (GCC in the specific case). I suggest 
having both the config and the assertion to be extra sure that the 
assumption is never broken (despite being very unlikely).

>> 
>>>>  -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.
>> 

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

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@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
> +
>   -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