On 08.08.2023 17:25, Julien Grall wrote: > On 02/08/2023 15:38, Nicola Vetrini wrote: >> The break statement after the return statement is definitely unreachable. >> As such, an call to the ASSERT_UNREACHABLE() macro is added to signal >> the intentionality of such construct. > > How about using unreachable() rather than ASSERT_UNREACHABLE()? The main > difference is the later will hint the compiler that the code cannot be > reached and will not crash Xen in debug build (this could be changed).
Isn't using unreachable() in place of ASSERT_UNREACHABLE() unsafe (not here but in general)? It'll tell the compiler that (in the extreme case) it can omit the function epilogue, including the return instruction. In the resulting build, if the code turns out to be reachable, execution would fall through into whatever follows. In the case here ... >> --- a/xen/common/sched/core.c >> +++ b/xen/common/sched/core.c >> @@ -2233,6 +2233,7 @@ static bool sched_tasklet_check_cpu(unsigned int cpu) >> /* fallthrough */ >> case TASKLET_enqueued|TASKLET_scheduled: >> return true; >> + ASSERT_UNREACHABLE(); >> break; >> case TASKLET_scheduled: >> clear_bit(_TASKLET_scheduled, tasklet_work); ... "return" alone already tells the compiler that "break" is unreachable. You don't need unreachable() for that. As said before, "break" like this simply want purging (and Misra is wrong to demand "break" everywhere - it should instead demand no [unannotated] fall-through, which can also be achieved by return, continue, and goto). Jan
