[PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2

Alessandro Zucchelli posted 2 patches 5 months, 1 week ago
[PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2
Posted by Alessandro Zucchelli 5 months, 1 week ago
Rule 21.2 reports identifiers reserved for the C and POSIX standard
libraries: all xen's translation units are compiled with option
-nostdinc, this guarantees that these libraries are not used, therefore
a justification is provided for allowing uses of such identifiers in
the project.
Builtins starting with "__builtin_" still remain available.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 447c1e6661..9fa9a7f01c 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -487,6 +487,17 @@ leads to a violation of the Rule are deviated."
 # Series 21.
 #
 
+-doc_begin="Rules 21.1 and 21.2 report identifiers reserved for the C and POSIX
+standard libraries: if these libraries are not used there is no reason to avoid such
+identifiers. All xen's translation units are compiled with option -nostdinc,
+this guarantees that these libraries are not used. Some compilers could perform
+optimization using built-in functions: this risk is partially addressed by
+using the compilation option -fno-builtin. Builtins starting with \"__builtin_\"
+still remain available."
+-config=MC3R1.R21.1,macros={safe , "!^__builtin_$" }
+-config=MC3R1.R21.2,declarations+={safe, "!^__builtin_.*$"}
+-doc_end
+
 -doc_begin="Xen does not use the functions provided by the Standard Library, but
 implements a set of functions that share the same names as their Standard Library equivalent.
 The implementation of these functions is available in source form, so the undefined, unspecified
-- 
2.34.1
Re: [PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2
Posted by Jan Beulich 5 months, 1 week ago
On 19.06.2024 19:09, Alessandro Zucchelli wrote:
> Rule 21.2 reports identifiers reserved for the C and POSIX standard
> libraries: all xen's translation units are compiled with option
> -nostdinc, this guarantees that these libraries are not used, therefore
> a justification is provided for allowing uses of such identifiers in
> the project.
> Builtins starting with "__builtin_" still remain available.
> 
> No functional change.
> 
> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 447c1e6661..9fa9a7f01c 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -487,6 +487,17 @@ leads to a violation of the Rule are deviated."
>  # Series 21.
>  #
>  
> +-doc_begin="Rules 21.1 and 21.2 report identifiers reserved for the C and POSIX
> +standard libraries: if these libraries are not used there is no reason to avoid such
> +identifiers. All xen's translation units are compiled with option -nostdinc,
> +this guarantees that these libraries are not used. Some compilers could perform
> +optimization using built-in functions: this risk is partially addressed by
> +using the compilation option -fno-builtin. Builtins starting with \"__builtin_\"
> +still remain available."

While the sub-section "Reserved Identifiers" is part of Section 7,
"Library", close coordination is needed between the library and the
compiler, which only together form an "implementation". Therefore any
use of identifiers beginning with two underscores or beginning with an
underscore and an upper case letter is at risk of colliding not only
with a particular library implementation (which we don't use), but
also of such with a particular compiler implementation (which we cannot
avoid to use). How can we permit use of such potentially problematic
identifiers?

Further, as to the rule mentioning file scope identifiers: Why is that?
The text in the C99 specification does not preclude their use, it merely
restricts what they may be used for. Why does Misra go yet farther?

Finally, why "partially addressed"? What part is unaddressed?

> +-config=MC3R1.R21.1,macros={safe , "!^__builtin_$" }
> +-config=MC3R1.R21.2,declarations+={safe, "!^__builtin_.*$"}

First: Why the differences in = vs += and in absence vs presence of .*

Second: The rules, according to my understanding, are about us defining
or declaring such identifiers, not about us using them. There shouldn't
be any #define, #undef, or declaration (let alone definition) of such
entities. All we do is _use_ them as e.g. expansion of #define-s. Thus:
Why is a deviation needed here in the first place? Then again - maybe
I'm reading this wrong, especially the leading ! may perhaps be some
form of negation.

Jan
Re: [PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2
Posted by Stefano Stabellini 5 months, 1 week ago
On Thu, 20 Jun 2024, Jan Beulich wrote:
> On 19.06.2024 19:09, Alessandro Zucchelli wrote:
> > Rule 21.2 reports identifiers reserved for the C and POSIX standard
> > libraries: all xen's translation units are compiled with option
> > -nostdinc, this guarantees that these libraries are not used, therefore
> > a justification is provided for allowing uses of such identifiers in
> > the project.
> > Builtins starting with "__builtin_" still remain available.
> > 
> > No functional change.
> > 
> > Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> > ---
> >  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index 447c1e6661..9fa9a7f01c 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -487,6 +487,17 @@ leads to a violation of the Rule are deviated."
> >  # Series 21.
> >  #
> >  
> > +-doc_begin="Rules 21.1 and 21.2 report identifiers reserved for the C and POSIX
> > +standard libraries: if these libraries are not used there is no reason to avoid such
> > +identifiers. All xen's translation units are compiled with option -nostdinc,
> > +this guarantees that these libraries are not used. Some compilers could perform
> > +optimization using built-in functions: this risk is partially addressed by
> > +using the compilation option -fno-builtin. Builtins starting with \"__builtin_\"
> > +still remain available."
> 
> While the sub-section "Reserved Identifiers" is part of Section 7,
> "Library", close coordination is needed between the library and the
> compiler, which only together form an "implementation". Therefore any
> use of identifiers beginning with two underscores or beginning with an
> underscore and an upper case letter is at risk of colliding not only
> with a particular library implementation (which we don't use), but
> also of such with a particular compiler implementation (which we cannot
> avoid to use). How can we permit use of such potentially problematic
> identifiers?

Alternative question: is there a way we can check if there is clash of
some sort between a compiler implementation of something and a MACRO or
identifier we have in Xen? An error or a warning from the compiler for
instance? That could be an easy way to prove we are safe.

Also, can we use the fact that the compiler we use is the same compiler
used to compile Linux, and Linux makes extensive use of identifiers and
macros starting with underscores as one of the reason for being safe
from clashes?
Re: [PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2
Posted by Jan Beulich 5 months ago
On 21.06.2024 03:02, Stefano Stabellini wrote:
> On Thu, 20 Jun 2024, Jan Beulich wrote:
>> On 19.06.2024 19:09, Alessandro Zucchelli wrote:
>>> Rule 21.2 reports identifiers reserved for the C and POSIX standard
>>> libraries: all xen's translation units are compiled with option
>>> -nostdinc, this guarantees that these libraries are not used, therefore
>>> a justification is provided for allowing uses of such identifiers in
>>> the project.
>>> Builtins starting with "__builtin_" still remain available.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
>>> ---
>>>  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> index 447c1e6661..9fa9a7f01c 100644
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -487,6 +487,17 @@ leads to a violation of the Rule are deviated."
>>>  # Series 21.
>>>  #
>>>  
>>> +-doc_begin="Rules 21.1 and 21.2 report identifiers reserved for the C and POSIX
>>> +standard libraries: if these libraries are not used there is no reason to avoid such
>>> +identifiers. All xen's translation units are compiled with option -nostdinc,
>>> +this guarantees that these libraries are not used. Some compilers could perform
>>> +optimization using built-in functions: this risk is partially addressed by
>>> +using the compilation option -fno-builtin. Builtins starting with \"__builtin_\"
>>> +still remain available."
>>
>> While the sub-section "Reserved Identifiers" is part of Section 7,
>> "Library", close coordination is needed between the library and the
>> compiler, which only together form an "implementation". Therefore any
>> use of identifiers beginning with two underscores or beginning with an
>> underscore and an upper case letter is at risk of colliding not only
>> with a particular library implementation (which we don't use), but
>> also of such with a particular compiler implementation (which we cannot
>> avoid to use). How can we permit use of such potentially problematic
>> identifiers?
> 
> Alternative question: is there a way we can check if there is clash of
> some sort between a compiler implementation of something and a MACRO or
> identifier we have in Xen? An error or a warning from the compiler for
> instance? That could be an easy way to prove we are safe.

Well. I think it is the default for the compiler to warn when re-#define-
ing a previously #define-d (by the compiler or by us) symbol, so on that
side we ought to be safe at any given point in time, yet we're still
latently unsafe (as to compilers introducing new pre-defines). For
built-in declarations, though, there's nothing I'm aware of that would
indicate collisions.

> Also, can we use the fact that the compiler we use is the same compiler
> used to compile Linux, and Linux makes extensive use of identifiers and
> macros starting with underscores as one of the reason for being safe
> from clashes?

I think we could, but I don't think we should.

Jan
Re: [PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2
Posted by Stefano Stabellini 5 months ago
On Fri, 21 Jun 2024, Jan Beulich wrote:
> On 21.06.2024 03:02, Stefano Stabellini wrote:
> > On Thu, 20 Jun 2024, Jan Beulich wrote:
> >> On 19.06.2024 19:09, Alessandro Zucchelli wrote:
> >>> Rule 21.2 reports identifiers reserved for the C and POSIX standard
> >>> libraries: all xen's translation units are compiled with option
> >>> -nostdinc, this guarantees that these libraries are not used, therefore
> >>> a justification is provided for allowing uses of such identifiers in
> >>> the project.
> >>> Builtins starting with "__builtin_" still remain available.
> >>>
> >>> No functional change.
> >>>
> >>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
> >>> ---
> >>>  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++++++
> >>>  1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> >>> index 447c1e6661..9fa9a7f01c 100644
> >>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> >>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> >>> @@ -487,6 +487,17 @@ leads to a violation of the Rule are deviated."
> >>>  # Series 21.
> >>>  #
> >>>  
> >>> +-doc_begin="Rules 21.1 and 21.2 report identifiers reserved for the C and POSIX
> >>> +standard libraries: if these libraries are not used there is no reason to avoid such
> >>> +identifiers. All xen's translation units are compiled with option -nostdinc,
> >>> +this guarantees that these libraries are not used. Some compilers could perform
> >>> +optimization using built-in functions: this risk is partially addressed by
> >>> +using the compilation option -fno-builtin. Builtins starting with \"__builtin_\"
> >>> +still remain available."
> >>
> >> While the sub-section "Reserved Identifiers" is part of Section 7,
> >> "Library", close coordination is needed between the library and the
> >> compiler, which only together form an "implementation". Therefore any
> >> use of identifiers beginning with two underscores or beginning with an
> >> underscore and an upper case letter is at risk of colliding not only
> >> with a particular library implementation (which we don't use), but
> >> also of such with a particular compiler implementation (which we cannot
> >> avoid to use). How can we permit use of such potentially problematic
> >> identifiers?
> > 
> > Alternative question: is there a way we can check if there is clash of
> > some sort between a compiler implementation of something and a MACRO or
> > identifier we have in Xen? An error or a warning from the compiler for
> > instance? That could be an easy way to prove we are safe.
> 
> Well. I think it is the default for the compiler to warn when re-#define-
> ing a previously #define-d (by the compiler or by us) symbol, so on that
> side we ought to be safe at any given point in time,

OK, that's good. It seems to me that this explanation should be part of
the deviation text.


> yet we're still latently unsafe (as to compilers introducing new
> pre-defines).

Sure, but we don't need to be safe in relation to future compiler. Right
now, we are targeting gcc-12.1.0 as written in
docs/misra/C-language-toolchain.rst. When we decide to enable a new
compiler in Xen we can fix/change any specific define as needed. Also
note the large amount of things written in C-language-toolchain.rst that
need to be checked and verified for a new compiler to make sure we can
actually use it safely (we make many assumptions).


> For built-in declarations, though, there's nothing I'm aware of that
> would indicate collisions.

For builtins, Alessandro was suggesting -fno-builtin. One question to
Alessandro is why would -fno-builtin only "partially" address the
problem.

Another question for Jan and also Alessandro: given that builtins
starting with __builtin_ remain available, any drawbacks in using
-fno-builtin in a Xen build?



> > Also, can we use the fact that the compiler we use is the same compiler
> > used to compile Linux, and Linux makes extensive use of identifiers and
> > macros starting with underscores as one of the reason for being safe
> > from clashes?
> 
> I think we could, but I don't think we should.
Re: [PATCH 1/2] automation/eclair_analysis: deviate MISRA C Rule 21.2
Posted by Jan Beulich 5 months ago
On 22.06.2024 01:27, Stefano Stabellini wrote:
> On Fri, 21 Jun 2024, Jan Beulich wrote:
>> On 21.06.2024 03:02, Stefano Stabellini wrote:
>>> On Thu, 20 Jun 2024, Jan Beulich wrote:
>>>> On 19.06.2024 19:09, Alessandro Zucchelli wrote:
>>>>> Rule 21.2 reports identifiers reserved for the C and POSIX standard
>>>>> libraries: all xen's translation units are compiled with option
>>>>> -nostdinc, this guarantees that these libraries are not used, therefore
>>>>> a justification is provided for allowing uses of such identifiers in
>>>>> the project.
>>>>> Builtins starting with "__builtin_" still remain available.
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
>>>>> ---
>>>>>  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> index 447c1e6661..9fa9a7f01c 100644
>>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> @@ -487,6 +487,17 @@ leads to a violation of the Rule are deviated."
>>>>>  # Series 21.
>>>>>  #
>>>>>  
>>>>> +-doc_begin="Rules 21.1 and 21.2 report identifiers reserved for the C and POSIX
>>>>> +standard libraries: if these libraries are not used there is no reason to avoid such
>>>>> +identifiers. All xen's translation units are compiled with option -nostdinc,
>>>>> +this guarantees that these libraries are not used. Some compilers could perform
>>>>> +optimization using built-in functions: this risk is partially addressed by
>>>>> +using the compilation option -fno-builtin. Builtins starting with \"__builtin_\"
>>>>> +still remain available."
>>>>
>>>> While the sub-section "Reserved Identifiers" is part of Section 7,
>>>> "Library", close coordination is needed between the library and the
>>>> compiler, which only together form an "implementation". Therefore any
>>>> use of identifiers beginning with two underscores or beginning with an
>>>> underscore and an upper case letter is at risk of colliding not only
>>>> with a particular library implementation (which we don't use), but
>>>> also of such with a particular compiler implementation (which we cannot
>>>> avoid to use). How can we permit use of such potentially problematic
>>>> identifiers?
>>>
>>> Alternative question: is there a way we can check if there is clash of
>>> some sort between a compiler implementation of something and a MACRO or
>>> identifier we have in Xen? An error or a warning from the compiler for
>>> instance? That could be an easy way to prove we are safe.
>>
>> Well. I think it is the default for the compiler to warn when re-#define-
>> ing a previously #define-d (by the compiler or by us) symbol, so on that
>> side we ought to be safe at any given point in time,
> 
> OK, that's good. It seems to me that this explanation should be part of
> the deviation text.
> 
> 
>> yet we're still latently unsafe (as to compilers introducing new
>> pre-defines).
> 
> Sure, but we don't need to be safe in relation to future compiler. Right
> now, we are targeting gcc-12.1.0 as written in
> docs/misra/C-language-toolchain.rst. When we decide to enable a new
> compiler in Xen we can fix/change any specific define as needed. Also
> note the large amount of things written in C-language-toolchain.rst that
> need to be checked and verified for a new compiler to make sure we can
> actually use it safely (we make many assumptions).
> 
> 
>> For built-in declarations, though, there's nothing I'm aware of that
>> would indicate collisions.
> 
> For builtins, Alessandro was suggesting -fno-builtin. One question to
> Alessandro is why would -fno-builtin only "partially" address the
> problem.
> 
> Another question for Jan and also Alessandro: given that builtins
> starting with __builtin_ remain available, any drawbacks in using
> -fno-builtin in a Xen build?

Just to mention it - we're building with -fno-builtin already anyway.
What I'm puzzled by is that it looks like I was under the wrong
impression that we're actually building -ffreestanding (which implies
-fno-builtin).

Jan