On 2025-08-11 22:30, Dmytro Prokopchuk1 wrote:
MISRA Rule 16.4: Every switch statement shall have a default label.
The default clause must contain either a statement or a comment
prior to its terminating break statement.

However, there is a documented rule that apply to the Xen in
'docs/misra/rules.rst':
Switch statements with integer types as controlling expression
should have a default label:
 - if the switch is expected to handle all possible cases
  explicitly, then a default label shall be added to handle
  unexpected error conditions, using BUG(), ASSERT(), WARN(),
  domain_crash(), or other appropriate methods;

These changes add `ASSERT_UNREACHABLE()` macro to the default clause of
switch statements that already explicitly handle all possible cases. This
ensures compliance with MISRA, avoids undefined behavior in unreachable
paths, and helps detect errors during development.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopch...@epam.com>
---
 xen/arch/arm/decode.c      |  3 +++
 xen/arch/arm/guest_walk.c  |  4 ++++
 xen/common/grant_table.c   | 10 ++++++++--
 xen/drivers/char/console.c |  3 +++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 2537dbebc1..cb64137b3b 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -178,6 +178,9 @@ static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
         case 3: /* Signed byte */
             update_dabt(dabt, reg, 0, true);
             break;
+        default:
+            ASSERT_UNREACHABLE();
+            break;
         }


I think this is fine, and there should be no problems with the break being unreachable in some configs due to the call property for ASSERT_UNREACHABLE

-doc_begin="Calls to function `__builtin_unreachable()' in the expansion of macro `ASSERT_UNREACHABLE()' are not considered to have the `noreturn' property." -call_properties+={"name(__builtin_unreachable)&&stmt(begin(any_exp(macro(name(ASSERT_UNREACHABLE)))))", {"noreturn(false)"}}
-doc_end


         break;
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index 09fe486598..9199a29602 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -167,6 +167,10 @@ static bool guest_walk_sd(const struct vcpu *v,
             *perms |= GV2M_EXEC;

         break;
+
+        default:
+            ASSERT_UNREACHABLE();
+            break;
     }


This one instead, besides being indented misleadingly IMO, should instead be on an enum instead of

/*
 * First level translation table descriptor types used by the AArch32
 * short-descriptor translation table format.
 */
#define L1DESC_INVALID                      (0)
#define L1DESC_PAGE_TABLE                   (1)
#define L1DESC_SECTION                      (2)
#define L1DESC_SECTION_PXN                  (3)

so that

-doc_begin="Switch statements having a controlling expression of enum type deliberately do not have a default case: gcc -Wall enables -Wswitch which warns (and breaks the build as we use -Werror) if one of the enum labels is missing from the switch." -config=MC3A2.R16.4,reports+={deliberate,'any_area(kind(context)&&^.* has no `default.*$&&stmt(node(switch_stmt)&&child(cond,skip(__non_syntactic_paren_stmts,type(canonical(enum_underlying_type(any())))))))'}
-doc_end

applies. What do you think?

     return true;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index cf131c43a1..60fc47f0c8 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -330,9 +330,12 @@ shared_entry_header(struct grant_table *t, grant_ref_t ref) /* Returned values should be independent of speculative execution */
         block_speculation();
         return &shared_entry_v2(t, ref).hdr;
+
+    default:
+        ASSERT_UNREACHABLE();
+        break;
     }

-    ASSERT_UNREACHABLE();
     block_speculation();


This is ok I think, same as (1).

     return NULL;
@@ -727,10 +730,13 @@ static unsigned int nr_grant_entries(struct grant_table *gt) /* Make sure we return a value independently of speculative execution */
         block_speculation();
         return f2e(nr_grant_frames(gt), 2);
+
+    default:
+        ASSERT_UNREACHABLE();
+        break;
 #undef f2e
     }

-    ASSERT_UNREACHABLE();
     block_speculation();


Same here.

     return 0;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 9bd5b4825d..608616f2af 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -889,6 +889,9 @@ static int cf_check parse_console_timestamps(const char *s)
         opt_con_timestamp_mode = TSM_DATE;
con_timestamp_mode_upd(param_2_parfs(parse_console_timestamps));
         return 0;
+    default:
+        ASSERT_UNREACHABLE();
+        break;
     }
     if ( *s == '\0' || /* Compat for old booleanparam() */
          !strcmp(s, "date") )

And here as well.

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

Reply via email to