Re: [XEN PATCH 5/7] xen/arm: traps: add ASSERT_UNREACHABLE() where needed

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-12-11 17:05, Julien Grall wrote:
> On 11/12/2023 15:59, Julien Grall wrote:
> > Hi Nicola,
> >
> > On 11/12/2023 14:54, Nicola Vetrini wrote:
> > > On 2023-12-11 13: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().
> > > > Cheers,
> > > It was meant as a safeguard against mistakes.
> > I am confused about which mistakes you are trying to prevent. Are you 
> > saying you are not trusting the noreturn attribute?
> > And if so, are you intending to add ASSERT_UNREACHABLE() after every 
> > single call to noreturn functions?
> Replying to myself. What's confusing the most is that in [1], you 
> decided to not add the ASSERT_UNREACHABLE(). Yet the problem is 
> similar.
> I'd also like to point out that by removing the "break", then if the 
> 'noreturn' function turns out to return, then in prod build you would 
> fallthrough to the next case. And who knows what's going to happen...
> All of this really adds some confusion...
I should have checked before responding: do_trap_hyp_sync is not afaik 
noreturn. Specifically, do_trap_brk may return. If I worked under the 
wrong assumption, then certainly the ASSERT_UNREACHABLE-s should be 
dropped.
> > > There are MISRA rules that deal with this aspect (e.g., a noreturn 
> > > function should not return), but they are not in Amendment 2, which 
> > > is what Xen is following.
> > Do you mind providing a bit more details about the rules you are 
> > talking about? At least the numbers would be helpful.
> [1] 
> https://lore.kernel.org/xen-devel/c0a8a12e39d688e101936d221af0f8eeefabe352.1702283415.git.nicola.vetrini@xxxxxxxxxxx/
> Cheers,
Sure. Here are the rules concerned with _Noreturn:

R17.9: A function declared with a _Noreturn function specifier shall not 
return to its caller
R17.10: A function declared with a _Noreturn function specifier shall 
have void return type
R17.11: A function that never returns should be declared with a 
_Noreturn function specifier
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)