| Message ID | 8db58416ce215a3c5fdba8074dc21f32116e8a41.1733915076.git.alessandro.zucchelli@bugseng.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | misra: add deviation for MISRA C Rule R11.1. | expand |
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.
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);
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(+)