diff mbox series

[XEN] x86/cpu-policy: justify a violation of MISRA C:2012 Rule 1.3

Message ID 9ec42e01258968d2dc71d59c5ad41da448a7112e.1690895099.git.nicola.vetrini@bugseng.com (mailing list archive)
State Superseded
Headers show
Series [XEN] x86/cpu-policy: justify a violation of MISRA C:2012 Rule 1.3 | expand

Commit Message

Nicola Vetrini Aug. 1, 2023, 1:06 p.m. UTC
The empty feature set 'str_7c1' in 'tools/misc/xen-cpuid.c' causes the
struct declaration to have no named members, hence violating
Rule 1.3:
"There shall be no occurrence of undefined or critical unspecified behaviour"
because it is forbidden by ISO/IEC 9899:1999(E), Section 6.7.2.1.7:
"If the struct-declaration-list contains no named
members, the behavior is undefined."

It has been assessed that the feature set declaration is intentionally empty,
and that no risk of undesired behaviour stems from it, hence the struct
declaration is marked safe.

No functional changes.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
As agreed during the MISRA C group meetings, this violation is dealt
with by means of a comment deviation, as future changes may eliminate the
root cause, which is the empty feature set.
My justification for the claim and the commit message may need some adjusting.
---
 docs/misra/safe.json                 | 9 +++++++++
 xen/include/xen/lib/x86/cpu-policy.h | 1 +
 2 files changed, 10 insertions(+)

Comments

Jan Beulich Aug. 1, 2023, 1:40 p.m. UTC | #1
On 01.08.2023 15:06, Nicola Vetrini wrote:
> The empty feature set 'str_7c1' in 'tools/misc/xen-cpuid.c' causes the
> struct declaration to have no named members, hence violating
> Rule 1.3:
> "There shall be no occurrence of undefined or critical unspecified behaviour"
> because it is forbidden by ISO/IEC 9899:1999(E), Section 6.7.2.1.7:
> "If the struct-declaration-list contains no named
> members, the behavior is undefined."
> 
> It has been assessed that the feature set declaration is intentionally empty,
> and that no risk of undesired behaviour stems from it, hence the struct
> declaration is marked safe.
> 
> No functional changes.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> As agreed during the MISRA C group meetings, this violation is dealt
> with by means of a comment deviation, as future changes may eliminate the
> root cause, which is the empty feature set.
> My justification for the claim and the commit message may need some adjusting.

A reference to the compiler extension would be nice; the use of extensions
(which generally are well-defined, even if not always well-documented)
should eliminate the UB that the standard specifies.

Jan
Nicola Vetrini Aug. 1, 2023, 1:50 p.m. UTC | #2
On 01/08/2023 15:40, Jan Beulich wrote:
> On 01.08.2023 15:06, Nicola Vetrini wrote:
>> The empty feature set 'str_7c1' in 'tools/misc/xen-cpuid.c' causes the
>> struct declaration to have no named members, hence violating
>> Rule 1.3:
>> "There shall be no occurrence of undefined or critical unspecified 
>> behaviour"
>> because it is forbidden by ISO/IEC 9899:1999(E), Section 6.7.2.1.7:
>> "If the struct-declaration-list contains no named
>> members, the behavior is undefined."
>> 
>> It has been assessed that the feature set declaration is intentionally 
>> empty,
>> and that no risk of undesired behaviour stems from it, hence the 
>> struct
>> declaration is marked safe.
>> 
>> No functional changes.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>> As agreed during the MISRA C group meetings, this violation is dealt
>> with by means of a comment deviation, as future changes may eliminate 
>> the
>> root cause, which is the empty feature set.
>> My justification for the claim and the commit message may need some 
>> adjusting.
> 
> A reference to the compiler extension would be nice; the use of 
> extensions
> (which generally are well-defined, even if not always well-documented)
> should eliminate the UB that the standard specifies.
> 

It sure is a good idea to specify this. Since the use of this compiler 
extension is already
documented in 'docs/misra/C-language-toolchain.rst' I can just add a 
reference to that
in the justification.
diff mbox series

Patch

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index e3c8a1d8eb..9239460e63 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -12,6 +12,15 @@ 
         },
         {
             "id": "SAF-1-safe",
+            "analyser": {
+                "eclair": "MC3R1.R1.3",
+                "text": "The following declaration of a struct with no named members results is deliberate and it has been assessed that no unintended behaviour arises from it."
+            },
+            "name": "Sentinel",
+            "text": "Next ID to be used"
+        },
+        {
+            "id": "SAF-2-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index bab3eecda6..6b52f080c9 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -203,6 +203,7 @@  struct cpu_policy
             };
             union {
                 uint32_t _7c1;
+                /* SAF-1-safe */
                 struct { DECL_BITFIELD(7c1); };
             };
             union {