| Message ID | 20221107104739.10404-5-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Static analyser finding deviation | expand |
On 07.11.2022 11:47, Luca Fancellu wrote: > --- a/xen/include/xen/kernel.h > +++ b/xen/include/xen/kernel.h > @@ -65,24 +65,28 @@ > 1; \ > }) > > +/* SAF-0-safe R8.6 linker script defined symbols */ > extern char _start[], _end[], start[]; > #define is_kernel(p) ({ \ > char *__p = (char *)(unsigned long)(p); \ > (__p >= _start) && (__p < _end); \ > }) > > +/* SAF-0-safe R8.6 linker script defined symbols */ > extern char _stext[], _etext[]; > #define is_kernel_text(p) ({ \ > char *__p = (char *)(unsigned long)(p); \ > (__p >= _stext) && (__p < _etext); \ > }) > > +/* SAF-0-safe R8.6 linker script defined symbols */ > extern const char _srodata[], _erodata[]; > #define is_kernel_rodata(p) ({ \ > const char *__p = (const char *)(unsigned long)(p); \ > (__p >= _srodata) && (__p < _erodata); \ > }) > > +/* SAF-0-safe R8.6 linker script defined symbols */ > extern char _sinittext[], _einittext[]; > #define is_kernel_inittext(p) ({ \ > char *__p = (char *)(unsigned long)(p); \ Why the "R8.6" everywhere here? Didn't we agree that the in-code comments should be tool-agnostic? Jan
> On 7 Nov 2022, at 11:49, Jan Beulich <jbeulich@suse.com> wrote: > > On 07.11.2022 11:47, Luca Fancellu wrote: >> --- a/xen/include/xen/kernel.h >> +++ b/xen/include/xen/kernel.h >> @@ -65,24 +65,28 @@ >> 1; \ >> }) >> >> +/* SAF-0-safe R8.6 linker script defined symbols */ >> extern char _start[], _end[], start[]; >> #define is_kernel(p) ({ \ >> char *__p = (char *)(unsigned long)(p); \ >> (__p >= _start) && (__p < _end); \ >> }) >> >> +/* SAF-0-safe R8.6 linker script defined symbols */ >> extern char _stext[], _etext[]; >> #define is_kernel_text(p) ({ \ >> char *__p = (char *)(unsigned long)(p); \ >> (__p >= _stext) && (__p < _etext); \ >> }) >> >> +/* SAF-0-safe R8.6 linker script defined symbols */ >> extern const char _srodata[], _erodata[]; >> #define is_kernel_rodata(p) ({ \ >> const char *__p = (const char *)(unsigned long)(p); \ >> (__p >= _srodata) && (__p < _erodata); \ >> }) >> >> +/* SAF-0-safe R8.6 linker script defined symbols */ >> extern char _sinittext[], _einittext[]; >> #define is_kernel_inittext(p) ({ \ >> char *__p = (char *)(unsigned long)(p); \ > Hi Jan, > Why the "R8.6" everywhere here? Didn't we agree that the in-code > comments should be tool-agnostic? The R8.6 is not tool specific, it is to give the quick hint that we are deviating from MISRA Rule 8.6. > > Jan
On 07.11.2022 12:53, Luca Fancellu wrote: >> On 7 Nov 2022, at 11:49, Jan Beulich <jbeulich@suse.com> wrote: >> On 07.11.2022 11:47, Luca Fancellu wrote: >>> --- a/xen/include/xen/kernel.h >>> +++ b/xen/include/xen/kernel.h >>> @@ -65,24 +65,28 @@ >>> 1; \ >>> }) >>> >>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>> extern char _start[], _end[], start[]; >>> #define is_kernel(p) ({ \ >>> char *__p = (char *)(unsigned long)(p); \ >>> (__p >= _start) && (__p < _end); \ >>> }) >>> >>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>> extern char _stext[], _etext[]; >>> #define is_kernel_text(p) ({ \ >>> char *__p = (char *)(unsigned long)(p); \ >>> (__p >= _stext) && (__p < _etext); \ >>> }) >>> >>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>> extern const char _srodata[], _erodata[]; >>> #define is_kernel_rodata(p) ({ \ >>> const char *__p = (const char *)(unsigned long)(p); \ >>> (__p >= _srodata) && (__p < _erodata); \ >>> }) >>> >>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>> extern char _sinittext[], _einittext[]; >>> #define is_kernel_inittext(p) ({ \ >>> char *__p = (char *)(unsigned long)(p); \ >> >> Why the "R8.6" everywhere here? Didn't we agree that the in-code >> comments should be tool-agnostic? > > The R8.6 is not tool specific, it is to give the quick hint that we are deviating > from MISRA Rule 8.6. Well, yes, "tool" was wrong for me to write. Imo references to a specific spec should equally be avoided in in-code comments, as other specs may turn up. Jan
On 07/11/2022 12:56, Jan Beulich wrote: > On 07.11.2022 12:53, Luca Fancellu wrote: >>> On 7 Nov 2022, at 11:49, Jan Beulich <jbeulich@suse.com> wrote: >>> On 07.11.2022 11:47, Luca Fancellu wrote: >>>> --- a/xen/include/xen/kernel.h >>>> +++ b/xen/include/xen/kernel.h >>>> @@ -65,24 +65,28 @@ >>>> 1; \ >>>> }) >>>> >>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>> extern char _start[], _end[], start[]; >>>> #define is_kernel(p) ({ \ >>>> char *__p = (char *)(unsigned long)(p); \ >>>> (__p >= _start) && (__p < _end); \ >>>> }) >>>> >>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>> extern char _stext[], _etext[]; >>>> #define is_kernel_text(p) ({ \ >>>> char *__p = (char *)(unsigned long)(p); \ >>>> (__p >= _stext) && (__p < _etext); \ >>>> }) >>>> >>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>> extern const char _srodata[], _erodata[]; >>>> #define is_kernel_rodata(p) ({ \ >>>> const char *__p = (const char *)(unsigned long)(p); \ >>>> (__p >= _srodata) && (__p < _erodata); \ >>>> }) >>>> >>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>> extern char _sinittext[], _einittext[]; >>>> #define is_kernel_inittext(p) ({ \ >>>> char *__p = (char *)(unsigned long)(p); \ >>> >>> Why the "R8.6" everywhere here? Didn't we agree that the in-code >>> comments should be tool-agnostic? >> >> The R8.6 is not tool specific, it is to give the quick hint that we are deviating >> from MISRA Rule 8.6. > > Well, yes, "tool" was wrong for me to write. Imo references to a specific > spec should equally be avoided in in-code comments, as other specs may > turn up. +1. The comment duplication is not great and sometimes even a short explanation it may not fit in 80 characters (AFAICT the justification should be a one line comment). Cheers,
> On 7 Nov 2022, at 19:06, Julien Grall <julien@xen.org> wrote: > > > > On 07/11/2022 12:56, Jan Beulich wrote: >> On 07.11.2022 12:53, Luca Fancellu wrote: >>>> On 7 Nov 2022, at 11:49, Jan Beulich <jbeulich@suse.com> wrote: >>>> On 07.11.2022 11:47, Luca Fancellu wrote: >>>>> --- a/xen/include/xen/kernel.h >>>>> +++ b/xen/include/xen/kernel.h >>>>> @@ -65,24 +65,28 @@ >>>>> 1; \ >>>>> }) >>>>> >>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>> extern char _start[], _end[], start[]; >>>>> #define is_kernel(p) ({ \ >>>>> char *__p = (char *)(unsigned long)(p); \ >>>>> (__p >= _start) && (__p < _end); \ >>>>> }) >>>>> >>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>> extern char _stext[], _etext[]; >>>>> #define is_kernel_text(p) ({ \ >>>>> char *__p = (char *)(unsigned long)(p); \ >>>>> (__p >= _stext) && (__p < _etext); \ >>>>> }) >>>>> >>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>> extern const char _srodata[], _erodata[]; >>>>> #define is_kernel_rodata(p) ({ \ >>>>> const char *__p = (const char *)(unsigned long)(p); \ >>>>> (__p >= _srodata) && (__p < _erodata); \ >>>>> }) >>>>> >>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>> extern char _sinittext[], _einittext[]; >>>>> #define is_kernel_inittext(p) ({ \ >>>>> char *__p = (char *)(unsigned long)(p); \ >>>> >>>> Why the "R8.6" everywhere here? Didn't we agree that the in-code >>>> comments should be tool-agnostic? >>> >>> The R8.6 is not tool specific, it is to give the quick hint that we are deviating >>> from MISRA Rule 8.6. >> Well, yes, "tool" was wrong for me to write. Imo references to a specific >> spec should equally be avoided in in-code comments, as other specs may >> turn up. > > +1. The comment duplication is not great and sometimes even a short explanation it may not fit in 80 characters (AFAICT the justification should be a one line comment). > Ok we can remove the R8.6 from the comments, is the remaining part ok? > Cheers, > > -- > Julien Grall
Hi Luca, On 08/11/2022 11:00, Luca Fancellu wrote: > > >> On 7 Nov 2022, at 19:06, Julien Grall <julien@xen.org> wrote: >> >> >> >> On 07/11/2022 12:56, Jan Beulich wrote: >>> On 07.11.2022 12:53, Luca Fancellu wrote: >>>>> On 7 Nov 2022, at 11:49, Jan Beulich <jbeulich@suse.com> wrote: >>>>> On 07.11.2022 11:47, Luca Fancellu wrote: >>>>>> --- a/xen/include/xen/kernel.h >>>>>> +++ b/xen/include/xen/kernel.h >>>>>> @@ -65,24 +65,28 @@ >>>>>> 1; \ >>>>>> }) >>>>>> >>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>> extern char _start[], _end[], start[]; >>>>>> #define is_kernel(p) ({ \ >>>>>> char *__p = (char *)(unsigned long)(p); \ >>>>>> (__p >= _start) && (__p < _end); \ >>>>>> }) >>>>>> >>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>> extern char _stext[], _etext[]; >>>>>> #define is_kernel_text(p) ({ \ >>>>>> char *__p = (char *)(unsigned long)(p); \ >>>>>> (__p >= _stext) && (__p < _etext); \ >>>>>> }) >>>>>> >>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>> extern const char _srodata[], _erodata[]; >>>>>> #define is_kernel_rodata(p) ({ \ >>>>>> const char *__p = (const char *)(unsigned long)(p); \ >>>>>> (__p >= _srodata) && (__p < _erodata); \ >>>>>> }) >>>>>> >>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>> extern char _sinittext[], _einittext[]; >>>>>> #define is_kernel_inittext(p) ({ \ >>>>>> char *__p = (char *)(unsigned long)(p); \ >>>>> >>>>> Why the "R8.6" everywhere here? Didn't we agree that the in-code >>>>> comments should be tool-agnostic? >>>> >>>> The R8.6 is not tool specific, it is to give the quick hint that we are deviating >>>> from MISRA Rule 8.6. >>> Well, yes, "tool" was wrong for me to write. Imo references to a specific >>> spec should equally be avoided in in-code comments, as other specs may >>> turn up. >> >> +1. The comment duplication is not great and sometimes even a short explanation it may not fit in 80 characters (AFAICT the justification should be a one line comment). >> > > Ok we can remove the R8.6 from the comments, is the remaining part ok? I am afraid no. The comment should only be /* SAF-0-safe */. Cheers,
> On 8 Nov 2022, at 11:32, Julien Grall <julien@xen.org> wrote: > > Hi Luca, > > On 08/11/2022 11:00, Luca Fancellu wrote: >>> On 7 Nov 2022, at 19:06, Julien Grall <julien@xen.org> wrote: >>> >>> >>> >>> On 07/11/2022 12:56, Jan Beulich wrote: >>>> On 07.11.2022 12:53, Luca Fancellu wrote: >>>>>> On 7 Nov 2022, at 11:49, Jan Beulich <jbeulich@suse.com> wrote: >>>>>> On 07.11.2022 11:47, Luca Fancellu wrote: >>>>>>> --- a/xen/include/xen/kernel.h >>>>>>> +++ b/xen/include/xen/kernel.h >>>>>>> @@ -65,24 +65,28 @@ >>>>>>> 1; \ >>>>>>> }) >>>>>>> >>>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>>> extern char _start[], _end[], start[]; >>>>>>> #define is_kernel(p) ({ \ >>>>>>> char *__p = (char *)(unsigned long)(p); \ >>>>>>> (__p >= _start) && (__p < _end); \ >>>>>>> }) >>>>>>> >>>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>>> extern char _stext[], _etext[]; >>>>>>> #define is_kernel_text(p) ({ \ >>>>>>> char *__p = (char *)(unsigned long)(p); \ >>>>>>> (__p >= _stext) && (__p < _etext); \ >>>>>>> }) >>>>>>> >>>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>>> extern const char _srodata[], _erodata[]; >>>>>>> #define is_kernel_rodata(p) ({ \ >>>>>>> const char *__p = (const char *)(unsigned long)(p); \ >>>>>>> (__p >= _srodata) && (__p < _erodata); \ >>>>>>> }) >>>>>>> >>>>>>> +/* SAF-0-safe R8.6 linker script defined symbols */ >>>>>>> extern char _sinittext[], _einittext[]; >>>>>>> #define is_kernel_inittext(p) ({ \ >>>>>>> char *__p = (char *)(unsigned long)(p); \ >>>>>> >>>>>> Why the "R8.6" everywhere here? Didn't we agree that the in-code >>>>>> comments should be tool-agnostic? >>>>> >>>>> The R8.6 is not tool specific, it is to give the quick hint that we are deviating >>>>> from MISRA Rule 8.6. >>>> Well, yes, "tool" was wrong for me to write. Imo references to a specific >>>> spec should equally be avoided in in-code comments, as other specs may >>>> turn up. >>> >>> +1. The comment duplication is not great and sometimes even a short explanation it may not fit in 80 characters (AFAICT the justification should be a one line comment). >>> >> Ok we can remove the R8.6 from the comments, is the remaining part ok? > > I am afraid no. The comment should only be /* SAF-0-safe */. Ok I will go for that in the next serie > > Cheers, > > -- > Julien Grall
diff --git a/docs/misra/safe.json b/docs/misra/safe.json index e079d3038120..e3c8a1d8eb36 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -3,6 +3,15 @@ "content": [ { "id": "SAF-0-safe", + "analyser": { + "eclair": "MC3R1.R8.6", + "coverity": "misra_c_2012_rule_8_6_violation" + }, + "name": "Rule 8.6: linker script defined symbols", + "text": "It is safe to declare this symbol because it is defined in the linker script." + }, + { + "id": "SAF-1-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h index 8cd142032d3b..efcd24b355d6 100644 --- a/xen/include/xen/kernel.h +++ b/xen/include/xen/kernel.h @@ -65,24 +65,28 @@ 1; \ }) +/* SAF-0-safe R8.6 linker script defined symbols */ extern char _start[], _end[], start[]; #define is_kernel(p) ({ \ char *__p = (char *)(unsigned long)(p); \ (__p >= _start) && (__p < _end); \ }) +/* SAF-0-safe R8.6 linker script defined symbols */ extern char _stext[], _etext[]; #define is_kernel_text(p) ({ \ char *__p = (char *)(unsigned long)(p); \ (__p >= _stext) && (__p < _etext); \ }) +/* SAF-0-safe R8.6 linker script defined symbols */ extern const char _srodata[], _erodata[]; #define is_kernel_rodata(p) ({ \ const char *__p = (const char *)(unsigned long)(p); \ (__p >= _srodata) && (__p < _erodata); \ }) +/* SAF-0-safe R8.6 linker script defined symbols */ extern char _sinittext[], _einittext[]; #define is_kernel_inittext(p) ({ \ char *__p = (char *)(unsigned long)(p); \
Eclair and Coverity found violation of the MISRA rule 8.6 for the symbols _start, _end, start, _stext, _etext, _srodata, _erodata, _sinittext, _einittext which are declared in xen/include/xen/kernel.h. All those symbols are defined by the liker script so we can deviate from the rule 8.6 for these cases. Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> --- docs/misra/safe.json | 9 +++++++++ xen/include/xen/kernel.h | 4 ++++ 2 files changed, 13 insertions(+)