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

[Demi Marie Obenour <demiobenour@xxxxxxxxx>]

On 7/18/25 07:08, Nicola Vetrini wrote:
> 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.
Time for a sed script?  Only half-joking.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

Attachment: OpenPGP_0xB288B55FFF9C22C1.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature