On 2023-12-12 16:49, Julien Grall wrote:
Hi,
On 11/12/2023 12:32, Julien Grall wrote:
Hi,
On 11/12/2023 10:30, Nicola Vetrini wrote:
The branches of the switch after a call to 'do_unexpected_trap'
cannot return, but there is one path that may return, hence
only some clauses are marked with ASSERT_UNREACHABLE().
I don't understand why this is necessary. The code should never be
reachable because do_unexpected_trap() is a noreturn().
From the matrix discussion, it wasn't clear what was my position on
this patch.
I would much prefer if the breaks are kept. I could accept:
ASSERT_UNREACHABLE();
break;
But this solution is a Nack because if you are concerned about
functions like do_unexpected_trap() to return by mistaken, then it
needs to also be safe in production.
The current proposal is not safe.
Cheers,
Ok. I wonder whether the should be applied here in vcpreg.c:
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index 39aeda9dab62..089d2f03eb5e 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -707,7 +707,8 @@ void do_cp10(struct cpu_user_regs *regs, const union
hsr hsr)
inject_undef_exception(regs, hsr);
return;
}
-
+
+ ASSERT_UNREACHABLE();
advance_pc(regs, hsr);
}
the rationale being that, should the switch somehow fail to return, the
advance_pc would be called, rather than doing nothing.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
