[PATCH v2] docs/misra: new rules addition

[PATCH v2] docs/misra: new rules addition

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@amd.com>

For Dir 1.1, a document describing all implementation-defined behaviour
(i.e. gcc-specific behavior) will be added to docs/misra, also including
implementation-specific (gcc-specific) appropriate types for bit-field
relevant to Rule 6.1.

Rule 21.21 is lacking an example on gitlab but the rule is
straightforward: we don't use stdlib at all in Xen.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v2:
- drop 5.6
- specify additional appropriate types for 6.1
---
 docs/misra/rules.rst | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index d5a6ee8cb6..8be6868496 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -40,6 +40,12 @@ existing codebase are work-in-progress.
      - Summary
      - Notes
 
+   * - `Dir 1.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_01_01.c>`_
+     - Required
+     - Any implementation-defined behaviour on which the output of the
+       program depends shall be documented and understood
+     -
+
    * - `Dir 2.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_02_01.c>`_
      - Required
      - All source files shall compile without any compilation errors
@@ -57,6 +63,13 @@ existing codebase are work-in-progress.
        header file being included more than once
      -
 
+   * - `Dir 4.11 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_11.c>`_
+     - Required
+     - The validity of values passed to library functions shall be checked
+     - We do not have libraries in Xen (libfdt and others are not
+       considered libraries from MISRA C point of view as they are
+       imported in source form)
+
    * - `Dir 4.14 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_14.c>`_
      - Required
      - The validity of values received from external sources shall be
@@ -133,6 +146,13 @@ existing codebase are work-in-progress.
        headers (xen/include/public/) are allowed to retain longer
        identifiers for backward compatibility.
 
+   * - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_
+     - Required
+     - Bit-fields shall only be declared with an appropriate type
+     - In addition to the C99 types, we also consider appropriate types:
+       unsigned char, unsigned short, unsigned long, unsigned long long,
+       enum.
+
    * - `Rule 6.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_02.c>`_
      - Required
      - Single-bit named bit fields shall not be of a signed type
@@ -143,6 +163,12 @@ existing codebase are work-in-progress.
      - Octal constants shall not be used
      -
 
+   * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
+     - Required
+     - A "u" or "U" suffix shall be applied to all integer constants
+       that are represented in an unsigned type
+     -
+
    * - `Rule 7.3 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_03.c>`_
      - Required
      - The lowercase character l shall not be used in a literal suffix
@@ -314,6 +340,11 @@ existing codebase are work-in-progress.
        used following a subsequent call to the same function
      -
 
+   * - Rule 21.21
+     - Required
+     - The Standard Library function system of <stdlib.h> shall not be used
+     -
+
    * - `Rule 22.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_22_02.c>`_
      - Mandatory
      - A block of memory shall only be freed if it was allocated by means of a
-- 
2.25.1

Re: [PATCH v2] docs/misra: new rules addition

[Jan Beulich]

On 09.06.2023 19:45, Stefano Stabellini wrote:
> @@ -133,6 +146,13 @@ existing codebase are work-in-progress.
>         headers (xen/include/public/) are allowed to retain longer
>         identifiers for backward compatibility.
>  
> +   * - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_
> +     - Required
> +     - Bit-fields shall only be declared with an appropriate type
> +     - In addition to the C99 types, we also consider appropriate types:
> +       unsigned char, unsigned short, unsigned long, unsigned long long,
> +       enum.

What about their signed equivalents? I'm surprised that I found only very
few uses (in Arm insn decoding afaict), but they generally have a purpose.
Are the uses we have (and new ones which may appear) intended to become
deviations?

> @@ -143,6 +163,12 @@ existing codebase are work-in-progress.
>       - Octal constants shall not be used
>       -
>  
> +   * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
> +     - Required
> +     - A "u" or "U" suffix shall be applied to all integer constants
> +       that are represented in an unsigned type
> +     -

I continue to consider "represented in" problematic here without
further qualification.

> @@ -314,6 +340,11 @@ existing codebase are work-in-progress.
>         used following a subsequent call to the same function
>       -
>  
> +   * - Rule 21.21
> +     - Required
> +     - The Standard Library function system of <stdlib.h> shall not be used
> +     -

Still no "inapplicable" note (whichever way it would be worded to also
please Roberto)?

Jan

Re: [PATCH v2] docs/misra: new rules addition

[Roberto Bagnara]

On 12/06/23 09:33, Jan Beulich wrote:
> On 09.06.2023 19:45, Stefano Stabellini wrote:
>> @@ -133,6 +146,13 @@ existing codebase are work-in-progress.
>>          headers (xen/include/public/) are allowed to retain longer
>>          identifiers for backward compatibility.
>>   
>> +   * - `Rule 6.1 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_06_01.c>`_
>> +     - Required
>> +     - Bit-fields shall only be declared with an appropriate type
>> +     - In addition to the C99 types, we also consider appropriate types:
>> +       unsigned char, unsigned short, unsigned long, unsigned long long,
>> +       enum.
> 
> What about their signed equivalents? I'm surprised that I found only very
> few uses (in Arm insn decoding afaict), but they generally have a purpose.
> Are the uses we have (and new ones which may appear) intended to become
> deviations?

Explicitly signed integer types are all supported by GCC as well.
So they can be added to the list.

>> @@ -143,6 +163,12 @@ existing codebase are work-in-progress.
>>        - Octal constants shall not be used
>>        -
>>   
>> +   * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
>> +     - Required
>> +     - A "u" or "U" suffix shall be applied to all integer constants
>> +       that are represented in an unsigned type
>> +     -
> 
> I continue to consider "represented in" problematic here without
> further qualification.

We should distinguish two things here.  The headline of Rule 7.2
is non negotiable: it is simply as it is.  As all headlines,
it is a compromise between conciseness and mnemonic value.
If what is wanted there is not the headline, then you can add
"implicitly" before "represented".  Or you may leave the headline
and add an explanatory note afterwards.

>> @@ -314,6 +340,11 @@ existing codebase are work-in-progress.
>>          used following a subsequent call to the same function
>>        -
>>   
>> +   * - Rule 21.21
>> +     - Required
>> +     - The Standard Library function system of <stdlib.h> shall not be used
>> +     -
> 
> Still no "inapplicable" note (whichever way it would be worded to also
> please Roberto)?

I am not the one to be pleased ;-)

But really, I don't follow: when you say the rule is inapplicable
your reasoning is, IIUC, "nobody would even dream using system() in Xen".
Which is exactly what the rule is asking.  If Xen adopts the rule,
tooling will make sure system() is not used, and seeing that the rule
is applied, assessors will be pleased.

Re: [PATCH v2] docs/misra: new rules addition

[Jan Beulich]

On 12.06.2023 11:34, Roberto Bagnara wrote:
> On 12/06/23 09:33, Jan Beulich wrote:
>> On 09.06.2023 19:45, Stefano Stabellini wrote:
>>> @@ -143,6 +163,12 @@ existing codebase are work-in-progress.
>>>        - Octal constants shall not be used
>>>        -
>>>   
>>> +   * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
>>> +     - Required
>>> +     - A "u" or "U" suffix shall be applied to all integer constants
>>> +       that are represented in an unsigned type
>>> +     -
>>
>> I continue to consider "represented in" problematic here without
>> further qualification.
> 
> We should distinguish two things here.  The headline of Rule 7.2
> is non negotiable: it is simply as it is.

I understand this, and ...

>  As all headlines,
> it is a compromise between conciseness and mnemonic value.
> If what is wanted there is not the headline, then you can add
> "implicitly" before "represented".  Or you may leave the headline
> and add an explanatory note afterwards.

... such a note is what my comment was heading towards.

>>> @@ -314,6 +340,11 @@ existing codebase are work-in-progress.
>>>          used following a subsequent call to the same function
>>>        -
>>>   
>>> +   * - Rule 21.21
>>> +     - Required
>>> +     - The Standard Library function system of <stdlib.h> shall not be used
>>> +     -
>>
>> Still no "inapplicable" note (whichever way it would be worded to also
>> please Roberto)?
> 
> I am not the one to be pleased ;-)
> 
> But really, I don't follow: when you say the rule is inapplicable
> your reasoning is, IIUC, "nobody would even dream using system() in Xen".
> Which is exactly what the rule is asking.  If Xen adopts the rule,
> tooling will make sure system() is not used, and seeing that the rule
> is applied, assessors will be pleased.

My point is that "not using functions of stdlib.h" is ambiguous: It may
mean functions implemented in an external library (which the hypervisor
doesn't use), or it may mean functions of identical name (and purpose).
The full text goes even further and forbids the use of these
identifiers (plural; see next paragraph), so it's clearly not only
about an external library, and we also can't put it off as inapplicable.
(I wouldn't be surprised if we had a local variable or label named
"exit" or "abort".)

Btw - I can't find a rule 21.21 in my two (slightly different) copies
of the doc, nor one with this headline and a different number. What I
have is "21.8 The Standard Library functions abort, exit and system of
<stdlib.h> shall not be used". (I further wonder why neither of the two
docs allows me to copy-and-paste a line out of it.)

Jan

Re: [PATCH v2] docs/misra: new rules addition

[Roberto Bagnara]

On 12/06/23 11:50, Jan Beulich wrote:
> On 12.06.2023 11:34, Roberto Bagnara wrote:
>> On 12/06/23 09:33, Jan Beulich wrote:
>>> On 09.06.2023 19:45, Stefano Stabellini wrote:
>>>> @@ -143,6 +163,12 @@ existing codebase are work-in-progress.
>>>>         - Octal constants shall not be used
>>>>         -
>>>>    
>>>> +   * - `Rule 7.2 <https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_07_02.c>`_
>>>> +     - Required
>>>> +     - A "u" or "U" suffix shall be applied to all integer constants
>>>> +       that are represented in an unsigned type
>>>> +     -
>>>
>>> I continue to consider "represented in" problematic here without
>>> further qualification.
>>
>> We should distinguish two things here.  The headline of Rule 7.2
>> is non negotiable: it is simply as it is.
> 
> I understand this, and ...
> 
>>   As all headlines,
>> it is a compromise between conciseness and mnemonic value.
>> If what is wanted there is not the headline, then you can add
>> "implicitly" before "represented".  Or you may leave the headline
>> and add an explanatory note afterwards.
> 
> ... such a note is what my comment was heading towards.

Here is an attempt.  "The rule asks that any integer literal
that is implicitly unsigned is made explicitly unsigned by
using one of the indicated suffixes.  As an example, on
a machine where the int type is 32-bit wide, 0x77777777
is signed whereas 0x80000000 is (implicitly) unsigned.
In order to comply with the rule, the latter should be
rewritten as either 0x80000000u or 0x80000000U.  Consistency
considerations may suggest using the same suffix even
when not required by the rule.  For instance, if one has

    f(0x77777777);  // Original
    f(0x80000000);

one might prefer

    f(0x77777777U); // Solution 1
    f(0x80000000U);

over

    f(0x77777777);  // Solution 2
    f(0x80000000U);

after having ascertained that "Solution 1" is compatible
with the intended semantics."


>>>> @@ -314,6 +340,11 @@ existing codebase are work-in-progress.
>>>>           used following a subsequent call to the same function
>>>>         -
>>>>    
>>>> +   * - Rule 21.21
>>>> +     - Required
>>>> +     - The Standard Library function system of <stdlib.h> shall not be used
>>>> +     -
>>>
>>> Still no "inapplicable" note (whichever way it would be worded to also
>>> please Roberto)?
>>
>> I am not the one to be pleased ;-)
>>
>> But really, I don't follow: when you say the rule is inapplicable
>> your reasoning is, IIUC, "nobody would even dream using system() in Xen".
>> Which is exactly what the rule is asking.  If Xen adopts the rule,
>> tooling will make sure system() is not used, and seeing that the rule
>> is applied, assessors will be pleased.
> 
> My point is that "not using functions of stdlib.h" is ambiguous: It may
> mean functions implemented in an external library (which the hypervisor
> doesn't use), or it may mean functions of identical name (and purpose).
> The full text goes even further and forbids the use of these
> identifiers (plural; see next paragraph), so it's clearly not only
> about an external library, and we also can't put it off as inapplicable.
> (I wouldn't be surprised if we had a local variable or label named
> "exit" or "abort".)
> 
> Btw - I can't find a rule 21.21 in my two (slightly different) copies
> of the doc, nor one with this headline and a different number. What I
> have is "21.8 The Standard Library functions abort, exit and system of
> <stdlib.h> shall not be used". (I further wonder why neither of the two
> docs allows me to copy-and-paste a line out of it.)

Rule 21.21 was added in MISRA C:2012 Amendment 2, which you can download
(free of charge) from https://www.misra.org.uk/app/uploads/2021/06/MISRA-C-2012-AMD2.pdf