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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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@amd.com wrote:
>>>>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>>> 
>>>>> 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@bugseng.com>
>>>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>>>> Signed-off-by: Victor Lira <victorm.lira@amd.com>
>>>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>>>>> ---
>>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> Cc: Anthony PERARD <anthony.perard@vates.tech>
>>>>> Cc: Michal Orzel <michal.orzel@amd.com>
>>>>> Cc: Jan Beulich <jbeulich@suse.com>
>>>>> Cc: Julien Grall <julien@xen.org>
>>>>> Cc: Roger Pau Monné <roger.pau@citrix.com>
>>>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>>>> Cc: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>>> Cc: Federico Serafini <federico.serafini@bugseng.com>
>>>>> Cc: Bertrand Marquis <bertrand.marquis@arm.com>
>>>>> ---
>>>>>    .../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