Rule 11.1 states as following: "Conversions shall not be performed
between a pointer to a function and any other type".

In "xen/common/bug.c", in order to get additional debug information,
pointer "bug_fn_t *fn" in the data section is converted to a function
pointer, which is then used to get such information. This specific
conversion has been reviewed and found to have no undefined behaviour
associated to it, therefore it can be exempted from compliance.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
As this patch introduces a deviation for service MC3A2.R11.1, it
depends on the following patch and shall not be applied prior to its
application.
https://lore.kernel.org/xen-devel/cf13be4779f15620e94b99b3b91f9cb040319989.1733826952.git.alessandro.zucchelli@bugseng.com/T/#u
---
 docs/misra/safe.json | 8 ++++++++
 xen/common/bug.c     | 1 +
 2 files changed, 9 insertions(+)

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 684346386e..d80fb3a48f 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -92,6 +92,14 @@
         },
         {
             "id": "SAF-11-safe",
+            "analyser": {
+                "eclair": "MC3A2.R11.1"
+            },
+            "name": "Rule 11.1: conversion for debugging purposes",
+            "text": "conversion of selected pointers to function pointers for debugging purposes are safe."
+        },
+        {
+            "id": "SAF-12-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/common/bug.c b/xen/common/bug.c
index 75cb35fcfa..2d08bb3d41 100644
--- a/xen/common/bug.c
+++ b/xen/common/bug.c
@@ -44,6 +44,7 @@ int do_bug_frame(const struct cpu_user_regs *regs, unsigned long pc)
 
     if ( id == BUGFRAME_run_fn )
     {
+        /* SAF-11-safe conversion for debugging purposes */
         bug_fn_t *fn = bug_ptr(bug);
 
         fn(regs);
-- 
2.43.0

On 11.12.2024 12:05, Alessandro Zucchelli wrote:
> Rule 11.1 states as following: "Conversions shall not be performed
> between a pointer to a function and any other type".
> 
> In "xen/common/bug.c", in order to get additional debug information,
> pointer "bug_fn_t *fn" in the data section is converted to a function
> pointer, which is then used to get such information.

If the pointer converted pointed into the data section, it would fault
upon being used to call what it points to, for the lack of execute
permissions there.

The change itself looks okay to me, but the description imo needs
updating, to be as precise as possible.

Jan

On Wed, 11 Dec 2024, Jan Beulich wrote:
> On 11.12.2024 12:05, Alessandro Zucchelli wrote:
> > Rule 11.1 states as following: "Conversions shall not be performed
> > between a pointer to a function and any other type".
> > 
> > In "xen/common/bug.c", in order to get additional debug information,
> > pointer "bug_fn_t *fn" in the data section is converted to a function
> > pointer, which is then used to get such information.
> 
> If the pointer converted pointed into the data section, it would fault
> upon being used to call what it points to, for the lack of execute
> permissions there.
> 
> The change itself looks okay to me, but the description imo needs
> updating, to be as precise as possible.


What about:

In "xen/common/bug.c", in order to get additional debug information,
pointer "bug_fn_t *fn" is converted to a function pointer, which is then
used to get such information.

?

On 12.12.2024 03:29, Stefano Stabellini wrote:
> On Wed, 11 Dec 2024, Jan Beulich wrote:
>> On 11.12.2024 12:05, Alessandro Zucchelli wrote:
>>> Rule 11.1 states as following: "Conversions shall not be performed
>>> between a pointer to a function and any other type".
>>>
>>> In "xen/common/bug.c", in order to get additional debug information,
>>> pointer "bug_fn_t *fn" in the data section is converted to a function
>>> pointer, which is then used to get such information.
>>
>> If the pointer converted pointed into the data section, it would fault
>> upon being used to call what it points to, for the lack of execute
>> permissions there.
>>
>> The change itself looks okay to me, but the description imo needs
>> updating, to be as precise as possible.
> 
> 
> What about:
> 
> In "xen/common/bug.c", in order to get additional debug information,
> pointer "bug_fn_t *fn" is converted to a function pointer, which is then
> used to get such information.
> 
> ?

This may do; I, however, was rather hoping for the description to be
extended rather than shrunk. E.g. '..., pointer "bug_fn_t *fn", obtained
by arithmetic on a pointer originating in the data section, is converted
to a function pointer, ...'

Jan

On Thu, 12 Dec 2024, Jan Beulich wrote:
> On 12.12.2024 03:29, Stefano Stabellini wrote:
> > On Wed, 11 Dec 2024, Jan Beulich wrote:
> >> On 11.12.2024 12:05, Alessandro Zucchelli wrote:
> >>> Rule 11.1 states as following: "Conversions shall not be performed
> >>> between a pointer to a function and any other type".
> >>>
> >>> In "xen/common/bug.c", in order to get additional debug information,
> >>> pointer "bug_fn_t *fn" in the data section is converted to a function
> >>> pointer, which is then used to get such information.
> >>
> >> If the pointer converted pointed into the data section, it would fault
> >> upon being used to call what it points to, for the lack of execute
> >> permissions there.
> >>
> >> The change itself looks okay to me, but the description imo needs
> >> updating, to be as precise as possible.
> > 
> > 
> > What about:
> > 
> > In "xen/common/bug.c", in order to get additional debug information,
> > pointer "bug_fn_t *fn" is converted to a function pointer, which is then
> > used to get such information.
> > 
> > ?
> 
> This may do; I, however, was rather hoping for the description to be
> extended rather than shrunk. E.g. '..., pointer "bug_fn_t *fn", obtained
> by arithmetic on a pointer originating in the data section, is converted
> to a function pointer, ...'

That's fine. 

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

If you feel like fixing it on commit, please go ahead.