[PATCH v5] automation/eclair: update configuration of D4.10

Stefano Stabellini posted 1 patch 4 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/alpine.DEB.2.22.394.2506201918100.2978375@ubuntu-linux-20-04-desktop
There is a newer version of this series
automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++----
automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
docs/misra/deviations.rst                        | 12 ++++++++++++
xen/arch/x86/include/asm/cpufeatures.h           |  8 +++++---
xen/include/xen/compile.h.in                     |  3 +++
xen/tools/process-banner.sed                     |  5 +++++
6 files changed, 33 insertions(+), 7 deletions(-)
[PATCH v5] automation/eclair: update configuration of D4.10
Posted by Stefano Stabellini 4 months, 1 week ago
MISRA C Directive 4.10 states that "Precautions shall be taken in order
to prevent the contents of a header file being included more than
once".

Add a SAF tag to the existing comment on top of cpufeatures.h.
Add a header inclusion guard to compile.h.

Update ECLAIR configuration to:
- extend existing deviation to other comments explicitly saying a file
  is intended for multiple inclusion;
- extend existing deviation to other autogenerated files;
- tag the guidelines as clean.

Update deviations.rst accordingly.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v5:
- add missing spaces in in-code comment
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++----
 automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
 docs/misra/deviations.rst                        | 12 ++++++++++++
 xen/arch/x86/include/asm/cpufeatures.h           |  8 +++++---
 xen/include/xen/compile.h.in                     |  3 +++
 xen/tools/process-banner.sed                     |  5 +++++
 6 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 9c67358d46..3b5bc87e1d 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -72,11 +72,14 @@ they are not instances of commented-out code."
 -config=MC3A2.D4.3,reports+={deliberate, "any_area(any_loc(file(arm64_bitops))&&context(name(int_clear_mask16)))"}
 -doc_end
 
--doc_begin="Files that are intended to be included more than once do not need to
-conform to the directive."
+-doc_begin="Files that are intended to be included more than once (and have
+a comment that says this explicitly) do not need to conform to the directive."
 -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* This file is intended to be included multiple times\\. \\*/$, begin-4))"}
--config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-3))"}
--config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/generated/autoconf.h$)))"}
+-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-3...begin-2))"}
+-doc_end
+
+-doc_begin="Autogenerated files that do not need to conform to the directive."
+-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/generated/autoconf\\.h$)))"}
 -doc_end
 
 -doc_begin="Including multiple times a .c file is safe because every function or data item
diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl b/automation/eclair_analysis/ECLAIR/tagging.ecl
index f9da5d5f4d..b95f07feb0 100644
--- a/automation/eclair_analysis/ECLAIR/tagging.ecl
+++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
@@ -23,6 +23,7 @@
 "MC3A2.D1.1||
 MC3A2.D2.1||
 MC3A2.D4.1||
+MC3A2.D4.10||
 MC3A2.D4.11||
 MC3A2.D4.14||
 MC3A2.R1.1||
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index fe0b1e10a2..63caa8f4a2 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -30,6 +30,18 @@ Deviations related to MISRA C:2012 Directives:
        not to add an additional encapsulation layer.
      - Tagged as `deliberate` for ECLAIR.
 
+   * - D4.10
+     - Files that are intended to be included more than once (and have
+       a comment that says this explicitly) do not need to conform to the
+       directive.
+     - Tagged as `safe` for ECLAIR.
+
+   * - D4.10
+     - There are autogenerated files that do not need to comply to the
+       directive.
+     - Tagged as `safe` for ECLAIR. Such files are:
+        - xen/include/generated/autoconf.h
+
    * - D4.10
      - Including multiple times a .c file is safe because every function or data item
        it defines would in (the common case) be already defined.
diff --git a/xen/arch/x86/include/asm/cpufeatures.h b/xen/arch/x86/include/asm/cpufeatures.h
index 9e3ed21c02..69041219cb 100644
--- a/xen/arch/x86/include/asm/cpufeatures.h
+++ b/xen/arch/x86/include/asm/cpufeatures.h
@@ -1,6 +1,6 @@
-/*
- * Explicitly intended for multiple inclusion.
- */
+/* This file is intended to be included multiple times. */
+/* #ifndef X86_CPUFEATURES_H */
+/* #define X86_CPUFEATURES_H */
 
 #include <xen/lib/x86/cpuid-autogen.h>
 
@@ -63,3 +63,5 @@ XEN_CPUFEATURE(USE_VMCALL,        X86_SYNTH(30)) /* Use VMCALL instead of VMMCAL
 
 /* Total number of capability words, inc synth and bug words. */
 #define NCAPINTS (FSCAPINTS + X86_NR_SYNTH + X86_NR_BUG) /* N 32-bit words worth of info */
+
+/* #endif X86_CPUFEATURES_H */
diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
index 3151d1e7d1..9206341ba6 100644
--- a/xen/include/xen/compile.h.in
+++ b/xen/include/xen/compile.h.in
@@ -1,3 +1,6 @@
+#ifndef XEN_COMPILE_H
+#define XEN_COMPILE_H
+
 #define XEN_COMPILE_DATE	"@@date@@"
 #define XEN_COMPILE_TIME	"@@time@@"
 #define XEN_COMPILE_BY		"@@whoami@@"
diff --git a/xen/tools/process-banner.sed b/xen/tools/process-banner.sed
index 56c76558bc..4cf3f9a116 100755
--- a/xen/tools/process-banner.sed
+++ b/xen/tools/process-banner.sed
@@ -12,3 +12,8 @@ s_(.*)_"\1\\n"_
 
 # Trailing \ on all but the final line.
 $!s_$_ \\_
+
+# Append closing header guard
+$a\
+\
+#endif /* XEN_COMPILE_H */
-- 
2.25.1
Re: [PATCH v5] automation/eclair: update configuration of D4.10
Posted by Nicola Vetrini 4 months, 1 week ago
On 2025-06-21 04:19, Stefano Stabellini wrote:
> MISRA C Directive 4.10 states that "Precautions shall be taken in order
> to prevent the contents of a header file being included more than
> once".
> 
> Add a SAF tag to the existing comment on top of cpufeatures.h.

You say this, but technically the comment is not a SAF comment, just a 
regular one that is interpreted to tailor the guideline.

> Add a header inclusion guard to compile.h.
> 
> Update ECLAIR configuration to:
> - extend existing deviation to other comments explicitly saying a file
>   is intended for multiple inclusion;
> - extend existing deviation to other autogenerated files;
> - tag the guidelines as clean.
> 
> Update deviations.rst accordingly.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> ---
> Changes in v5:
> - add missing spaces in in-code comment
> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++----
>  automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
>  docs/misra/deviations.rst                        | 12 ++++++++++++
>  xen/arch/x86/include/asm/cpufeatures.h           |  8 +++++---
>  xen/include/xen/compile.h.in                     |  3 +++
>  xen/tools/process-banner.sed                     |  5 +++++
>  6 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 9c67358d46..3b5bc87e1d 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -72,11 +72,14 @@ they are not instances of commented-out code."
>  -config=MC3A2.D4.3,reports+={deliberate, 
> "any_area(any_loc(file(arm64_bitops))&&context(name(int_clear_mask16)))"}
>  -doc_end
> 
> --doc_begin="Files that are intended to be included more than once do 
> not need to
> -conform to the directive."
> +-doc_begin="Files that are intended to be included more than once (and 
> have
> +a comment that says this explicitly) do not need to conform to the 
> directive."
>  -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* This file 
> is intended to be included multiple times\\. \\*/$, begin-4))"}

[1] Here

> --config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated 
> file, do not edit! \\*/$, begin-3))"}
> --config=MC3A2.D4.10,reports+={safe, 
> "all_area(all_loc(file(^xen/include/generated/autoconf.h$)))"}
> +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated 
> file, do not edit! \\*/$, begin-3...begin-2))"}
> +-doc_end
> +
> +-doc_begin="Autogenerated files that do not need to conform to the 
> directive."
> +-config=MC3A2.D4.10,reports+={safe, 
> "all_area(all_loc(file(^xen/include/generated/autoconf\\.h$)))"}
>  -doc_end
> 
>  -doc_begin="Including multiple times a .c file is safe because every 
> function or data item
> diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl 
> b/automation/eclair_analysis/ECLAIR/tagging.ecl
> index f9da5d5f4d..b95f07feb0 100644
> --- a/automation/eclair_analysis/ECLAIR/tagging.ecl
> +++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
> @@ -23,6 +23,7 @@
>  "MC3A2.D1.1||
>  MC3A2.D2.1||
>  MC3A2.D4.1||
> +MC3A2.D4.10||
>  MC3A2.D4.11||
>  MC3A2.D4.14||
>  MC3A2.R1.1||
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index fe0b1e10a2..63caa8f4a2 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -30,6 +30,18 @@ Deviations related to MISRA C:2012 Directives:
>         not to add an additional encapsulation layer.
>       - Tagged as `deliberate` for ECLAIR.
> 
> +   * - D4.10
> +     - Files that are intended to be included more than once (and have
> +       a comment that says this explicitly) do not need to conform to 
> the
> +       directive.
> +     - Tagged as `safe` for ECLAIR.
> +
> +   * - D4.10
> +     - There are autogenerated files that do not need to comply to the
> +       directive.
> +     - Tagged as `safe` for ECLAIR. Such files are:
> +        - xen/include/generated/autoconf.h
> +
>     * - D4.10
>       - Including multiple times a .c file is safe because every 
> function or data item
>         it defines would in (the common case) be already defined.
> diff --git a/xen/arch/x86/include/asm/cpufeatures.h 
> b/xen/arch/x86/include/asm/cpufeatures.h
> index 9e3ed21c02..69041219cb 100644
> --- a/xen/arch/x86/include/asm/cpufeatures.h
> +++ b/xen/arch/x86/include/asm/cpufeatures.h
> @@ -1,6 +1,6 @@
> -/*
> - * Explicitly intended for multiple inclusion.
> - */
> +/* This file is intended to be included multiple times. */
> +/* #ifndef X86_CPUFEATURES_H */
> +/* #define X86_CPUFEATURES_H */
> 

Are these two lines really needed? I may be mistaken, but I think the 
violation's first location would be the #include below with a comment a 
couple of lines above captured by the config at the top [1]. @Federico 
thoughts?

>  #include <xen/lib/x86/cpuid-autogen.h>
> 
> @@ -63,3 +63,5 @@ XEN_CPUFEATURE(USE_VMCALL,        X86_SYNTH(30)) /* 
> Use VMCALL instead of VMMCAL
> 
>  /* Total number of capability words, inc synth and bug words. */
>  #define NCAPINTS (FSCAPINTS + X86_NR_SYNTH + X86_NR_BUG) /* N 32-bit 
> words worth of info */
> +
> +/* #endif X86_CPUFEATURES_H */
> diff --git a/xen/include/xen/compile.h.in 
> b/xen/include/xen/compile.h.in
> index 3151d1e7d1..9206341ba6 100644
> --- a/xen/include/xen/compile.h.in
> +++ b/xen/include/xen/compile.h.in
> @@ -1,3 +1,6 @@
> +#ifndef XEN_COMPILE_H
> +#define XEN_COMPILE_H
> +
>  #define XEN_COMPILE_DATE	"@@date@@"
>  #define XEN_COMPILE_TIME	"@@time@@"
>  #define XEN_COMPILE_BY		"@@whoami@@"
> diff --git a/xen/tools/process-banner.sed 
> b/xen/tools/process-banner.sed
> index 56c76558bc..4cf3f9a116 100755
> --- a/xen/tools/process-banner.sed
> +++ b/xen/tools/process-banner.sed
> @@ -12,3 +12,8 @@ s_(.*)_"\1\\n"_
> 
>  # Trailing \ on all but the final line.
>  $!s_$_ \\_
> +
> +# Append closing header guard
> +$a\
> +\
> +#endif /* XEN_COMPILE_H */

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
Re: [PATCH v5] automation/eclair: update configuration of D4.10
Posted by Stefano Stabellini 4 months, 1 week ago
On Sun, 22 Jun 2025, Nicola Vetrini wrote:
> On 2025-06-21 04:19, Stefano Stabellini wrote:
> > MISRA C Directive 4.10 states that "Precautions shall be taken in order
> > to prevent the contents of a header file being included more than
> > once".
> > 
> > Add a SAF tag to the existing comment on top of cpufeatures.h.
> 
> You say this, but technically the comment is not a SAF comment, just a regular
> one that is interpreted to tailor the guideline.

No, that is a mistake in the commit message. It should be changed.


> > Add a header inclusion guard to compile.h.
> > 
> > Update ECLAIR configuration to:
> > - extend existing deviation to other comments explicitly saying a file
> >   is intended for multiple inclusion;
> > - extend existing deviation to other autogenerated files;
> > - tag the guidelines as clean.
> > 
> > Update deviations.rst accordingly.
> > 
> > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > ---
> > Changes in v5:
> > - add missing spaces in in-code comment
> > ---
> >  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++----
> >  automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
> >  docs/misra/deviations.rst                        | 12 ++++++++++++
> >  xen/arch/x86/include/asm/cpufeatures.h           |  8 +++++---
> >  xen/include/xen/compile.h.in                     |  3 +++
> >  xen/tools/process-banner.sed                     |  5 +++++
> >  6 files changed, 33 insertions(+), 7 deletions(-)
> > 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index 9c67358d46..3b5bc87e1d 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -72,11 +72,14 @@ they are not instances of commented-out code."
> >  -config=MC3A2.D4.3,reports+={deliberate,
> > "any_area(any_loc(file(arm64_bitops))&&context(name(int_clear_mask16)))"}
> >  -doc_end
> > 
> > --doc_begin="Files that are intended to be included more than once do not
> > need to
> > -conform to the directive."
> > +-doc_begin="Files that are intended to be included more than once (and have
> > +a comment that says this explicitly) do not need to conform to the
> > directive."
> >  -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* This file is
> > intended to be included multiple times\\. \\*/$, begin-4))"}
> 
> [1] Here
> 
> > --config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file,
> > do not edit! \\*/$, begin-3))"}
> > --config=MC3A2.D4.10,reports+={safe,
> > "all_area(all_loc(file(^xen/include/generated/autoconf.h$)))"}
> > +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file,
> > do not edit! \\*/$, begin-3...begin-2))"}
> > +-doc_end
> > +
> > +-doc_begin="Autogenerated files that do not need to conform to the
> > directive."
> > +-config=MC3A2.D4.10,reports+={safe,
> > "all_area(all_loc(file(^xen/include/generated/autoconf\\.h$)))"}
> >  -doc_end
> > 
> >  -doc_begin="Including multiple times a .c file is safe because every
> > function or data item
> > diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl
> > b/automation/eclair_analysis/ECLAIR/tagging.ecl
> > index f9da5d5f4d..b95f07feb0 100644
> > --- a/automation/eclair_analysis/ECLAIR/tagging.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
> > @@ -23,6 +23,7 @@
> >  "MC3A2.D1.1||
> >  MC3A2.D2.1||
> >  MC3A2.D4.1||
> > +MC3A2.D4.10||
> >  MC3A2.D4.11||
> >  MC3A2.D4.14||
> >  MC3A2.R1.1||
> > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > index fe0b1e10a2..63caa8f4a2 100644
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -30,6 +30,18 @@ Deviations related to MISRA C:2012 Directives:
> >         not to add an additional encapsulation layer.
> >       - Tagged as `deliberate` for ECLAIR.
> > 
> > +   * - D4.10
> > +     - Files that are intended to be included more than once (and have
> > +       a comment that says this explicitly) do not need to conform to the
> > +       directive.
> > +     - Tagged as `safe` for ECLAIR.
> > +
> > +   * - D4.10
> > +     - There are autogenerated files that do not need to comply to the
> > +       directive.
> > +     - Tagged as `safe` for ECLAIR. Such files are:
> > +        - xen/include/generated/autoconf.h
> > +
> >     * - D4.10
> >       - Including multiple times a .c file is safe because every function or
> > data item
> >         it defines would in (the common case) be already defined.
> > diff --git a/xen/arch/x86/include/asm/cpufeatures.h
> > b/xen/arch/x86/include/asm/cpufeatures.h
> > index 9e3ed21c02..69041219cb 100644
> > --- a/xen/arch/x86/include/asm/cpufeatures.h
> > +++ b/xen/arch/x86/include/asm/cpufeatures.h
> > @@ -1,6 +1,6 @@
> > -/*
> > - * Explicitly intended for multiple inclusion.
> > - */
> > +/* This file is intended to be included multiple times. */
> > +/* #ifndef X86_CPUFEATURES_H */
> > +/* #define X86_CPUFEATURES_H */
> > 
> 
> Are these two lines really needed? I may be mistaken, but I think the
> violation's first location would be the #include below with a comment a couple
> of lines above captured by the config at the top [1]. @Federico thoughts?

Without these 2 lines, ECLAIR complains about the following:

#include <xen/lib/x86/cpuid-autogen.h>
       ^
 MC3A2.D4.10non-compliant start of header file


https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/10420715821

https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/people/sstabellini/xen/ECLAIR_normal/ppp3/X86_64/10420715821/PROJECT.ecd;/sources/xen/arch/x86/include/asm/cpufeatures.h.html#R1_1{%22select%22:true,%22selection%22:{%22hiddenAreaKinds%22:[],%22hiddenSubareaKinds%22:[],%22show%22:false,%22selector%22:{%22enabled%22:true,%22negated%22:true,%22kind%22:0,%22domain%22:%22kind%22,%22inputs%22:[{%22enabled%22:true,%22text%22:%22violation%22}]}}}
Re: [PATCH v5] automation/eclair: update configuration of D4.10
Posted by Jan Beulich 4 months, 1 week ago
On 22.06.2025 22:58, Stefano Stabellini wrote:
> On Sun, 22 Jun 2025, Nicola Vetrini wrote:
>> On 2025-06-21 04:19, Stefano Stabellini wrote:
>>> --- a/xen/arch/x86/include/asm/cpufeatures.h
>>> +++ b/xen/arch/x86/include/asm/cpufeatures.h
>>> @@ -1,6 +1,6 @@
>>> -/*
>>> - * Explicitly intended for multiple inclusion.
>>> - */
>>> +/* This file is intended to be included multiple times. */
>>> +/* #ifndef X86_CPUFEATURES_H */
>>> +/* #define X86_CPUFEATURES_H */
>>>
>>
>> Are these two lines really needed? I may be mistaken, but I think the
>> violation's first location would be the #include below with a comment a couple
>> of lines above captured by the config at the top [1]. @Federico thoughts?
> 
> Without these 2 lines, ECLAIR complains about the following:
> 
> #include <xen/lib/x86/cpuid-autogen.h>
>        ^
>  MC3A2.D4.10non-compliant start of header file

And that's where hence a SAF-8 comment cold be placed, I would think.

Jan
Re: [PATCH v5] automation/eclair: update configuration of D4.10
Posted by Stefano Stabellini 4 months, 1 week ago
On Mon, 23 Jun 2025, Jan Beulich wrote:
> On 22.06.2025 22:58, Stefano Stabellini wrote:
> > On Sun, 22 Jun 2025, Nicola Vetrini wrote:
> >> On 2025-06-21 04:19, Stefano Stabellini wrote:
> >>> --- a/xen/arch/x86/include/asm/cpufeatures.h
> >>> +++ b/xen/arch/x86/include/asm/cpufeatures.h
> >>> @@ -1,6 +1,6 @@
> >>> -/*
> >>> - * Explicitly intended for multiple inclusion.
> >>> - */
> >>> +/* This file is intended to be included multiple times. */
> >>> +/* #ifndef X86_CPUFEATURES_H */
> >>> +/* #define X86_CPUFEATURES_H */
> >>>
> >>
> >> Are these two lines really needed? I may be mistaken, but I think the
> >> violation's first location would be the #include below with a comment a couple
> >> of lines above captured by the config at the top [1]. @Federico thoughts?
> > 
> > Without these 2 lines, ECLAIR complains about the following:
> > 
> > #include <xen/lib/x86/cpuid-autogen.h>
> >        ^
> >  MC3A2.D4.10non-compliant start of header file
> 
> And that's where hence a SAF-8 comment cold be placed, I would think.

You are right, I tried it and it works