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