1. Patchew
  2. Xen
  3. View series

[XEN PATCH] x86: p2m-pod: address violation of MISRA C Rule 2.1

Nicola Vetrini posted 1 patch 1 year, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
docs/misra/safe.json      | 8 ++++++++
xen/arch/x86/mm/p2m-pod.c | 1 +
2 files changed, 9 insertions(+)
[XEN PATCH] x86: p2m-pod: address violation of MISRA C Rule 2.1
Posted by Nicola Vetrini 1 year, 7 months ago 
The label 'out_unmap' is only reachable after ASSERT_UNREACHABLE,
so the code below is only executed upon erroneously reaching that
program point and calling domain_crash, thus resulting in the
for loop after 'out_unmap' to become unreachable in some configurations.

This is a defensive coding measure to have a safe fallback that is
reachable in non-debug builds, and can thus be deviated with a
comment-based deviation.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 docs/misra/safe.json      | 8 ++++++++
 xen/arch/x86/mm/p2m-pod.c | 1 +
 2 files changed, 9 insertions(+)

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index c213e0a0be3b..b114c9485c86 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -60,6 +60,14 @@
         },
         {
             "id": "SAF-7-safe",
+            "analyser": {
+                "eclair": "MC3R1.R2.1"
+            },
+            "name": "MC3R1.R2.1: statement unreachable in some configurations",
+            "text": "Every path that can reach this statement is preceded by statements that make it unreachable in certain configurations (e.g. ASSERT_UNREACHABLE). This is understood as a means of defensive programming."
+        },
+        {
+            "id": "SAF-8-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index bd84fe9e27ee..5a96c46a2286 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1040,6 +1040,7 @@ out_unmap:
      * Something went wrong, probably crashing the domain.  Unmap
      * everything and return.
      */
+    /* SAF-7-safe Rule 2.1: defensive programming */
     for ( i = 0; i < count; i++ )
         if ( map[i] )
             unmap_domain_page(map[i]);
-- 
2.34.1
Re: [XEN PATCH] x86: p2m-pod: address violation of MISRA C Rule 2.1
Posted by Stefano Stabellini 1 year, 7 months ago 
On Thu, 27 Jun 2024, Nicola Vetrini wrote:
> The label 'out_unmap' is only reachable after ASSERT_UNREACHABLE,
> so the code below is only executed upon erroneously reaching that
> program point and calling domain_crash, thus resulting in the
> for loop after 'out_unmap' to become unreachable in some configurations.
> 
> This is a defensive coding measure to have a safe fallback that is
> reachable in non-debug builds, and can thus be deviated with a
> comment-based deviation.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

The patch needs rebasing as it doesn't apply to staging anymore

Other than that:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

This is actually going help also in terms of identifying impossible code
paths for coverage

> ---
>  docs/misra/safe.json      | 8 ++++++++
>  xen/arch/x86/mm/p2m-pod.c | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/docs/misra/safe.json b/docs/misra/safe.json
> index c213e0a0be3b..b114c9485c86 100644
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -60,6 +60,14 @@
>          },
>          {
>              "id": "SAF-7-safe",
> +            "analyser": {
> +                "eclair": "MC3R1.R2.1"
> +            },
> +            "name": "MC3R1.R2.1: statement unreachable in some configurations",
> +            "text": "Every path that can reach this statement is preceded by statements that make it unreachable in certain configurations (e.g. ASSERT_UNREACHABLE). This is understood as a means of defensive programming."
> +        },
> +        {
> +            "id": "SAF-8-safe",
>              "analyser": {},
>              "name": "Sentinel",
>              "text": "Next ID to be used"
> diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
> index bd84fe9e27ee..5a96c46a2286 100644
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -1040,6 +1040,7 @@ out_unmap:
>       * Something went wrong, probably crashing the domain.  Unmap
>       * everything and return.
>       */
> +    /* SAF-7-safe Rule 2.1: defensive programming */
>      for ( i = 0; i < count; i++ )
>          if ( map[i] )
>              unmap_domain_page(map[i]);
> -- 
> 2.34.1
>
Re: [XEN PATCH] x86: p2m-pod: address violation of MISRA C Rule 2.1
Posted by Nicola Vetrini 1 year, 7 months ago 
On 2024-06-28 01:18, Stefano Stabellini wrote:
> On Thu, 27 Jun 2024, Nicola Vetrini wrote:
>> The label 'out_unmap' is only reachable after ASSERT_UNREACHABLE,
>> so the code below is only executed upon erroneously reaching that
>> program point and calling domain_crash, thus resulting in the
>> for loop after 'out_unmap' to become unreachable in some 
>> configurations.
>> 
>> This is a defensive coding measure to have a safe fallback that is
>> reachable in non-debug builds, and can thus be deviated with a
>> comment-based deviation.
>> 
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> The patch needs rebasing as it doesn't apply to staging anymore
> 
> Other than that:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> This is actually going help also in terms of identifying impossible 
> code
> paths for coverage
> 

Thanks, I just sent a rebased v2 version.

>> ---
>>  docs/misra/safe.json      | 8 ++++++++
>>  xen/arch/x86/mm/p2m-pod.c | 1 +
>>  2 files changed, 9 insertions(+)
>> 
>> diff --git a/docs/misra/safe.json b/docs/misra/safe.json
>> index c213e0a0be3b..b114c9485c86 100644
>> --- a/docs/misra/safe.json
>> +++ b/docs/misra/safe.json
>> @@ -60,6 +60,14 @@
>>          },
>>          {
>>              "id": "SAF-7-safe",
>> +            "analyser": {
>> +                "eclair": "MC3R1.R2.1"
>> +            },
>> +            "name": "MC3R1.R2.1: statement unreachable in some 
>> configurations",
>> +            "text": "Every path that can reach this statement is 
>> preceded by statements that make it unreachable in certain 
>> configurations (e.g. ASSERT_UNREACHABLE). This is understood as a 
>> means of defensive programming."
>> +        },
>> +        {
>> +            "id": "SAF-8-safe",
>>              "analyser": {},
>>              "name": "Sentinel",
>>              "text": "Next ID to be used"
>> diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
>> index bd84fe9e27ee..5a96c46a2286 100644
>> --- a/xen/arch/x86/mm/p2m-pod.c
>> +++ b/xen/arch/x86/mm/p2m-pod.c
>> @@ -1040,6 +1040,7 @@ out_unmap:
>>       * Something went wrong, probably crashing the domain.  Unmap
>>       * everything and return.
>>       */
>> +    /* SAF-7-safe Rule 2.1: defensive programming */
>>      for ( i = 0; i < count; i++ )
>>          if ( map[i] )
>>              unmap_domain_page(map[i]);
>> --
>> 2.34.1
>> 

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.