[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH 5/7] xen/arm: traps: add ASSERT_UNREACHABLE() where needed
Hi Stefano, On 12/12/2023 01:36, Stefano Stabellini wrote: On Mon, 11 Dec 2023, Nicola Vetrini wrote: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.It looks like we could add noreturn to do_trap_brk. Julien what do you think? You can't add noreturn to do_trap_brk(). It is used to handle WARN_ON() that *will* return. Cheers, -- Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |