Re: [PATCH v1] misra: add deviation for rules 21.1 and 21.2

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-07-18 11:28, Dmytro Prokopchuk1 wrote:
> On 7/18/25 12:17, Dmytro Prokopchuk wrote:
> > On 7/18/25 08:31, Jan Beulich wrote:
> > > On 17.07.2025 22:47, Dmytro Prokopchuk1 wrote:
> > > > On 4/23/25 20:54, victorm.lira@xxxxxxx wrote:
> > > > > From: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > > > >
> > > > > MISRA C Rules 21.1 ("#define and #undef shall not be used on a
> > > > > reserved identifier or reserved macro name") and R21.2 ("A reserved
> > > > > identifier or reserved macro name shall not be declared") 
> > > > > violations
> > > > > are not problematic for Xen, as it does not use the C or POSIX
> > > > > libraries.
> > > > >
> > > > > Xen uses -fno-builtin and -nostdinc to ensure this, but there are 
> > > > > still
> > > > > __builtin_* functions from the compiler that are available so
> > > > > a deviation is formulated for all identifiers not starting with
> > > > > "__builtin_".
> > > > >
> > > > > The missing text of a deviation for Rule 21.2 is added to
> > > > > docs/misra/deviations.rst.
> > > > >
> > > > > To avoid regressions, tag both rules as clean and add them to the
> > > > > monitored set.
> > > > >
> > > > > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > > > > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > > > > Signed-off-by: Victor Lira <victorm.lira@xxxxxxx>
> > > > > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > > > ---
> > > > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > > Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > > > > Cc: Michal Orzel <michal.orzel@xxxxxxx>
> > > > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > > > Cc: Julien Grall <julien@xxxxxxx>
> > > > > Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > > > Cc: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > > > > Cc: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > > > > Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> > > > > ---
> > > > >    .../eclair_analysis/ECLAIR/deviations.ecl     |  9 ++++++-
> > > > >    .../eclair_analysis/ECLAIR/monitored.ecl      |  2 ++
> > > > >    automation/eclair_analysis/ECLAIR/tagging.ecl |  2 ++
> > > > >    docs/misra/deviations.rst                     | 26 
> > > > > ++++++++++++++
> > > > > ++++-
> > > > >    4 files changed, 37 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/
> > > > > automation/eclair_analysis/ECLAIR/deviations.ecl
> > > > > index 2c8fb92713..ffa23b53b7 100644
> > > > > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > > > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > > > @@ -639,8 +639,15 @@ in the expansion."
> > > > >    # Series 21.
> > > > >    #
> > > > >
> > > > > +-doc_begin="Xen does not use C and POSIX libraries:
> > > > > +identifiers reserved by these libraries can be used safely, except
> > > > > for those
> > > > > +beginning with '__builtin_'."
> > > > > +-config=MC3A2.R21.1,macros={safe, "!^__builtin_.*$"}
> > > > > +-config=MC3A2.R21.2,declarations={safe, "!^__builtin_.*$"}
> > > > > +-doc_end
> > > > > +
> > > > >    -doc_begin="or, and and xor are reserved identifiers because 
> > > > > they
> > > > > constitute alternate
> > > > > -spellings for the corresponding operators (they are defined as
> > > > > macros by iso646.h).
> > > > > +spellings for the corresponding logical operators (as defined in
> > > > > header 'iso646.h').
> > > > >    However, Xen doesn't use standard library headers, so there is 
> > > > > no
> > > > > risk of overlap."
> > > > >    -config=MC3A2.R21.2,reports+={safe,
> > > > > "any_area(stmt(ref(kind(label)&&^(or|and|xor|not)$)))"}
> > > > >    -doc_end
> > > > > diff --git a/automation/eclair_analysis/ECLAIR/monitored.ecl b/
> > > > > automation/eclair_analysis/ECLAIR/monitored.ecl
> > > > > index 8351996ec8..da229a0d84 100644
> > > > > --- a/automation/eclair_analysis/ECLAIR/monitored.ecl
> > > > > +++ b/automation/eclair_analysis/ECLAIR/monitored.ecl
> > > > > @@ -79,6 +79,8 @@
> > > > >    -enable=MC3A2.R20.12
> > > > >    -enable=MC3A2.R20.13
> > > > >    -enable=MC3A2.R20.14
> > > > > +-enable=MC3A2.R21.1
> > > > > +-enable=MC3A2.R21.2
> > > > >    -enable=MC3A2.R21.3
> > > > >    -enable=MC3A2.R21.4
> > > > >    -enable=MC3A2.R21.5
> > > > > diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl b/
> > > > > automation/eclair_analysis/ECLAIR/tagging.ecl
> > > > > index 1d078d8905..3292bf751e 100644
> > > > > --- a/automation/eclair_analysis/ECLAIR/tagging.ecl
> > > > > +++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
> > > > > @@ -88,6 +88,8 @@ MC3A2.R20.11||
> > > > >    MC3A2.R20.12||
> > > > >    MC3A2.R20.13||
> > > > >    MC3A2.R20.14||
> > > > > +MC3A2.R21.1||
> > > > > +MC3A2.R21.2||
> > > > >    MC3A2.R21.3||
> > > > >    MC3A2.R21.4||
> > > > >    MC3A2.R21.5||
> > > > > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > > > > index fe0b1e10a2..88328eaa8a 100644
> > > > > --- a/docs/misra/deviations.rst
> > > > > +++ b/docs/misra/deviations.rst
> > > > > @@ -587,7 +587,31 @@ Deviations related to MISRA C:2012 Rules:
> > > > >           construct is deviated only in Translation Units that
> > > > > present a violation
> > > > >           of the Rule due to uses of this macro.
> > > > >         - Tagged as `deliberate` for ECLAIR.
> > > > > -
> > > > > +
> > > > > +   * - R21.1
> > > > > +     - Rule 21.1 reports identifiers reserved for the C and POSIX
> > > > > standard
> > > > > +       libraries. Xen does not use such libraries and all
> > > > > translation units
> > > > > +       are compiled with option '-nostdinc', therefore there is no
> > > > > reason to
> > > > > +       avoid to use `#define` or `#undef` on such identifiers
> > > > > except for those
> > > > > +       beginning with `__builtin_` for which compilers may perform
> > > > > (wrong)
> > > > > +       optimizations.
> > > > > +     - Tagged as `safe` for ECLAIR.
> > > > > +
> > > > > +   * - R21.2
> > > > > +     - Rule 21.2 reports identifiers reserved for the C and POSIX
> > > > > standard
> > > > > +       libraries. Xen does not use such libraries and all
> > > > > translation units
> > > > > +       are compiled with option '-nostdinc', therefore there is no
> > > > > reason to
> > > > > +       avoid declaring such identifiers except for those beginning
> > > > > with
> > > > > +       `__builtin_` for which compilers may perform (wrong)
> > > > > optimizations.
> > > > > +     - Tagged as `safe` for ECLAIR.
> > > > > +
> > > > > +   * - R21.2
> > > > > +     - `or`, `and` and `xor` are reserved identifiers because they
> > > > > constitute
> > > > > +       alternate spellings for the corresponding logical operators
> > > > > +       (as defined in Standard Library header `\<iso646.h\>`). Xen
> > > > > does not use
> > > > > +       Standard library headers, so there is no risk of overlap.
> > > > > +     - Tagged as `safe` for ECLAIR.
> > > > > +
> > > > >       * - R21.9
> > > > >         - Xen does not use the `bsearch` and `qsort` functions
> > > > > provided by the C
> > > > >           Standard Library, but provides in source form its own
> > > > > implementation,
> > > > > --
> > > > > 2.47.0
> > > > Hello All!
> > > >
> > > > I tried to play with Rule 21.1 deviations.
> > > >
> > > > After applying the following configurations:
> > > >
> > > > -config=MC3A2.R21.1,macros+={safe, "^offsetof$ || ^(is|to)[a-z]+$ ||
> > > > name(NULL) || name(bool) || name(true) || name(false)"}
> > > > -config=MC3A2.R21.1,macros+={safe,
> > > > "loc(file(^xen/include/xen/inttypes\\.h$))"}
> > > > -config=MC3A2.R21.1,macros+={safe, "loc(file(^xen/include/xen/types\
> > > > \.h$))"}
> > > > -config=MC3A2.R21.1,macros+={safe, "^str[a-z]+$ || ^(v)?sprintf$ ||
> > > > ^va_[a-z]+$"}
> > > Can you spell these out in words? I can only vaguely interpret these
> > > Eclair
> > > patterns, sorry.
> > Yes, sure.
> >
> > That means to deviate the following macros:
> > - offsetof
> > - begin with either ‘is’ or ‘to’ followed by a lowercase letters
> > (islower, isdigit, tolower, toupper, etc.)
> > - NULL
> > - bool
> > - true
> > - false
> > - all PRI/SCN macros for printing/scanning format specifiers from 
> > header
> > file xen/include/xen/inttypes.h
> > - all macros from header file xen/include/xen/types.h (limits:
> > UINT8_MAX, INT_MAX, LONG_MIN, etc.)
> > - begin with 'str' followed by a lowercase letters (string functions)
> > - sprintf/vsprintf
> > - begin with 'va_' followed by a lowercase letters (va_copy, va_start,
> > va_end, va_arg)
> >
> > > > Eclair showed 699 remaining violations.
> > > > All of them were related to names beginning with an underscore (_).
> > > >
> > > > It's possible to resolve the rest of them with help of (all, except 
> > > > for
> > > > those beginning with '__builtin_' and '__x86_64__'):
> > > >
> > > > -config=MC3A2.R21.1,macros+={safe, "^_.*$ && !^__builtin_.*$ &&
> > > > !^__x86_64__$"}
> > > >
> > > > Probably, the exception list can be extended.
> > > >
> > > > Jan,
> > > > I know you don't want to disallow "_all_" such reserved identifiers.
> > > > But how to deal with that?
> > > How do I not want this? I've been arguing for years that we should
> > > respect
> > > the reserved name spaces. (Did you perhaps mean "... you don't want 
> > > to
> > > deviate ..."?) There are exceptions, yes, e.g. ...
> > Yes, I meant about deviations. Sorry.
> >
> > > > Try to cover all macros? Like this, for example (I assume that there 
> > > > are
> > > > no such reserved macros):
> > > > -config=MC3A2.R21.1,macros+={safe, "^.*XEN.*$ || ^.*HYPERVISOR.*$"}
> > > ... anything we may have (wrongly) introduced into the public 
> > > headers. We
> > > can't very well change naming there.
> > Looks like the only way is to deviate all macros (that are currently
> > used in Xen), tag rule as "clean" and prohibit using reserved names in
> > the future.
> >
> > Any suggestions?
> >
> > Dmytro
> BTW, not all violations are in public headers.
> Probably, they could be fixed in code.
> But the number of them is huge...
This is precisely the issue I was pointing out when you offered to 
respin this patch. Yes, Xen could fix those rather than deviate, but the 
sheer number of violations makes this in my opinion unfeasible.
> Dmytro
>
> > > Jan
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253