Re: [PATCH] misra: address MISRA C Rule 18.3 compliance

Thu, 24 Jul 2025 00:30:38 -0700 

On 2025-07-23 12:12, Dmytro Prokopchuk1 wrote:

Rule 18.3: "The relational operators >, >=, < and <=
shall not be applied to objects of pointer type
except where they point into the same object".

Update relational comparison to cast `text_start`
(void pointer) to `unsigned long`. This ensures the
comparison occurs between two values of the same integral type.

Update relational comparison between `lock1` and `lock2` to cast
pointers to `uintptr_t`. This ensures MISRA C compliance and avoids
undefined behavior when comparing pointer values directly.

Update for-loop condition to cast pointers `p` and `params->checksum`
to `uintptr_t` for the relational `<` operator. This resolves a MISRA C 18.3 violation by ensuring that relational operations are not performed directly 
on pointers of different objects (which is undefined behavior).

Add Rule 18.3 deviations.

Add Rule 18.3 to the monitored set.

No functional changes to program behavior.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopch...@epam.com>
---
This patch eliminates MISRA C Rule 18.3 violations for both ARM and X86.
Test CI: https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1943306650 
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
 automation/eclair_analysis/ECLAIR/monitored.ecl  | 1 +
 xen/arch/x86/efi/efi-boot.h                      | 3 ++-
 xen/common/sched/core.c                          | 2 +-
 xen/common/virtual_region.c                      | 4 ++--
 5 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl 
index 483507e7b9..d89834a49b 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -568,6 +568,14 @@ C99 Undefined Behaviour 45: Pointers that do not point into, or just beyond, the -config=MC3A2.R18.2,reports+={safe, "any_area(any_loc(any_exp(macro(^page_to_mfn$))))"} 
 -doc_end
+-doc_begin="Consider relational pointer comparisons in kernel-related sections as safe and compliant."
This would need some evidence that they don't compare pointers to different arrays, wouldn't it?
+-config=MC3R1.R18.3,reports+={safe, "any_area(any_loc(any_exp(macro(name(is_kernel||is_kernel_text||is_kernel_rodata||is_kernel_inittext)))))"} 
+-doc_end
+
+-doc_begin="Allow deviations for pointer comparisons in BUG_ON and ASSERT macros, treating them as safe for debugging and validation." +-config=MC3R1.R18.3,reports+={safe, "any_area(any_loc(any_exp(macro(name(BUG_ON||ASSERT)))))"} 
+-doc_end
+
same here: no matter that they are used for debugging, if you mark something as "safe", then the argument needs to be there (in the .rst file, but still).
-doc_begin="Flexible array members are deliberately used and XEN developers are aware of the dangers related to them: unexpected result when the structure is given as argument to a sizeof() operator and the truncation in assignment between structures." 
 -config=MC3A2.R18.7,reports+={deliberate, "any()"}
diff --git a/automation/eclair_analysis/ECLAIR/monitored.ecl b/automation/eclair_analysis/ECLAIR/monitored.ecl 
index 00bff9edbe..b8fb297e73 100644
--- a/automation/eclair_analysis/ECLAIR/monitored.ecl
+++ b/automation/eclair_analysis/ECLAIR/monitored.ecl
@@ -68,6 +68,7 @@
 -enable=MC3A2.R17.6
 -enable=MC3A2.R18.1
 -enable=MC3A2.R18.2
+-enable=MC3A2.R18.3
 -enable=MC3A2.R18.6
 -enable=MC3A2.R18.8
 -enable=MC3A2.R19.1
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 0194720003..170c569eb4 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -461,7 +461,8 @@ static void __init efi_arch_edd(void)
                 params->device_path_info_length =
                     sizeof(struct edd_device_params) -
                     offsetof(struct edd_device_params, key);
- for ( p = (const u8 *)&params->key; p < &params->checksum; ++p ) 
+                for ( p = (const u8 *)&params->key;
+ (uintptr_t)p < (uintptr_t)&params->checksum; ++p ) 
                     params->checksum -= *p;
                 break;
             case MEDIA_DEVICE_PATH:
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index ea95dea65a..c3c101c142 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -360,7 +360,7 @@ static always_inline void sched_spin_lock_double(
     {
         *flags = _spin_lock_irqsave(lock1);
     }
-    else if ( lock1 < lock2 )
+    else if ( (uintptr_t)lock1 < (uintptr_t)lock2 )
     {
         *flags = _spin_lock_irqsave(lock1);
         _spin_lock(lock2);
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index 1dc2e9f592..515751b6c3 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -83,8 +83,8 @@ const struct virtual_region *find_text_region(unsigned long addr) 
     rcu_read_lock(&rcu_virtual_region_lock);
     list_for_each_entry_rcu ( iter, &virtual_region_list, list )
     {
-        if ( (void *)addr >= iter->text_start &&
-             (void *)addr <  iter->text_end )
+        if ( addr >= (unsigned long)iter->text_start &&
+             addr <  (unsigned long)iter->text_end )
         {
             region = iter;
             break;


--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Reply via email to