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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-04-28 11:04, Jan Beulich wrote:
> On 28.04.2025 10:09, Nicola Vetrini wrote:
> > On 2025-04-28 08:15, Jan Beulich wrote:
> > > On 25.04.2025 17:53, Nicola Vetrini wrote:
> > > > On 2025-04-25 10:07, Jan Beulich wrote:
> > > > > On 24.04.2025 23:45, Stefano Stabellini wrote:
> > > > > > On Thu, 24 Apr 2025, Jan Beulich wrote:
> > > > > > > On 23.04.2025 19: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>
> > > > > > > While the rule is in the library section, ...
> > > > > > >
> > > > > > > > --- 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.
> > > > > > > ... I'd like to ask that it be explicitly clarified here that 
> > > > > > > it's
> > > > > > > solely
> > > > > > > the library (and not e.g. the compiler itself) that are of 
> > > > > > > concern
> > > > > > > here.
> > > > > > The language can be clarified:
> > > > > >
> > > > > > - 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 C and POSIX standard
> > > > > > libraries
> > > > > >   identifiers except for those beginning with `__builtin_` for 
> > > > > > which
> > > > > >   compilers may perform (wrong) optimizations.
> > > > > Which makes it more apparent that there is a gap: What about e.g.
> > > > > __x86_64__?
> > > > > That falls within what the rules cover, is not a C or POSIX 
> > > > > standard
> > > > > library
> > > > > identifier, yet very clearly must not be fiddled with. Whereas the
> > > > > text
> > > > > above deviates it.
> > > > that is true, even if unlikely: one approach could be to avoid
> > > > deviating
> > > > predefined macros for all CUs as -nostdinc and -fno-builtins should
> > > > take
> > > > care of the rest; this kind of deviation is not currently possible 
> > > > in
> > > > ECLAIR, but it might be in the future.
> > > Is this perhaps because by "all pre-defined macros" you really mean
> > > _just_
> > > those, and not the entire reserved (for that purpose) sub-namespace?
> > > Imo
> > > we should not go by what a particular compiler may pre-define (we may
> > > even
> > > overlook something if we did it this way).
> > >
> > > Jan
> > I think there is a slight misalignment here: maybe I'm interpreting 
> > your
> > answer incorrectly. Let me try to restate the proposal: the following
> > identifiers are not allowed to be #define'd or #undef'd:
> > - __builtin_*
> > - for each CU, all macro identifiers already defined upon 
> > preprocessing
> > that CU by the compiler invocation for that CU. This set of 
> > identifiers
> > is always a subset of all the reserved identifiers, but is also
> > dependent on the compiler invocation that is used for that CU, (e.g.
> > __x86_64__ for an Arm target is usually safe to define, as it's
> > typically not a predefined macro introduced by the compiler for that
> > invocation,
> No, it's not - elsewhere in the tree we may use this to distinguish
> architectures. Plus isn't Misra heavily about avoiding developer
> confusion? Defining __x86_64__ on Arm code is, imo, a pretty confusing
> thing to do.
Indeed it is confusing, but likely safe from the perspective of 
preventing UB, which is the main rationale of this rule. For the 
purposes of distinguishing architectures I'd expect a #ifdef __x86_64__ 
or #if defined(__x86_64__) and those are fine, as this only applies to 
#define or #undef.
> > while (re)defining __STDC_VERSION__ or __SIZEOF_INT__ will
> > be a violation in any command line I can think of). Note that this is
> > not a static set, but is based on what is determined to be predefined 
> > at
> > the moment of the analysis, so it is not tied to what a particular
> > compiler pre-defines.
> Right. Yet what I'm trying to get to is that we disallow _all_ such
> reserved identifiers, not just a subset.
I understand now. There are thousands of locations to be touched to 
remove all uses of reserved identifiers, since they are used quite 
extensively in Xen (A rough estimation is around 1.5k such identifiers, 
with ~900 violations on Arm and ~1000 on x86, without counting their 
occurrences). That is a very disruptive change, even if split very 
finely.
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253