Change Details

__builtin_unreachable doesn't raise any compile-time warnings/errors on its own, so problems with its usage can't be easily detected. While it would be nice for this situation to change and compilers to at least add a warning for trivial cases where local state means the instruction can't be reached, this isn't the case at the moment. Push __unreachable out of sys/cdefs.h for _KERNEL builds[*], making sys/systm.h a requirement for it in the kernel. We then turn it into a straight panic[**] under INVARIANTS, as the context will very likely not have taken into account what might happen if this happened. [*] Alternate option here, I think, would be to *instead* provide a __kernel_unreachable and use that in the kernel instead of __unreachable, leaving sys/cdefs.h untouched. I opted not do so in this initial work so that out-of-tree stuff using __unreachable either breaks or gets the new behavior. [**] I had also considered a kassert_panic, but I'm unsure -- a lot of the same things would apply to a KASSERT() apply here, but this is perhaps more likely to be used in scenarios where we really really won't do the right thing if it's reached.

__builtin_unreachable doesn't raise any compile-time warnings/errors on its own, so problems with its usage can't be easily detected. While it would be nice for this situation to change and compilers to at least add a warning for trivial cases where local state means the instruction can't be reached, this isn't the case at the moment. This commit adds an __assert_unreachable, whose intend is incredibly clear: it asserts that this instruction is unreachable. On INVARIANTS builds, it's a panic(), and on non-INVARIANTS it's __unreachable().

__builtin_unreachable doesn't raise any compile-time warnings/errors on its own, so problems with its usage can't be easily detected. While it would be nice for this situation to change and compilers to at least add a warning for trivial cases where local state means the instruction can't be reached, this isn't the case at the moment. Push __unreachable out of sys/cdefs.h for _KERNEL builds[*], making sys/systm.h a requirement for it in the kernel. We then turn it into a straight panic[**] under INVARIANTS, as the context will very likely not have taken into account what might happen if this happened. [*] Alternate option here, I think, would be to *instead* provide a __kernel_unreachable and use that in the kernel instead of _This commit adds an __assert_unreachable, leaving sys/cdefs.h untouchedwhose intend is incredibly clear: it asserts that this instruction is unreachable. I opted not do so in this initial work so that out-of-tree stuff using __unreachable either breaks or gets the new behavior. [**] I had also considered a kassert_panicOn INVARIANTS builds, but I'm unsure -- a lot of the same things would apply to a KASSERT() apply hereit's a panic(), but this is perhaps more likely to be used in scenarios where we really really won't do the right thing if it's reachedand on non-INVARIANTS it's __unreachable().