Re: [RFC PATCH v3 11/12] powerpc: Remove unreachable() from WARN_ON()

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jun 30, 2022 at 10:58:11AM -0500, Segher Boessenkool wrote:
> On Thu, Jun 30, 2022 at 04:07:47PM +0530, Naveen N. Rao wrote:
> > Objtool classifies 'ud2' as INSN_BUG, and 'int3' as INSN_TRAP. In x86 
> > BUG(), there is no need for an annotation since objtool assumes that 
> > 'ud2' terminates control flow. But, for __WARN_FLAGS(), since 'ud2' is 
> > used, an explicit annotate_reachable() is needed. That's _reachable_, to 
> > indicate that the control flow can continue with the next instruction.
> > 
> > On powerpc, we should (eventually) classify all trap variants as 
> > INSN_TRAP. Even in the absence of that classification today, objtool 
> > assumes that control flow continues with the next instruction. With your 
> > work to utilize asm goto for __WARN_FLAGS(), with no extra instructions 
> > being generated, I think it is appropriate to just use 
> > __builtin_unreachable() and to not use the annotation.
> > 
> > In any case, we are only hitting this since gcc is generating a 'bl' due 
> > to that annotation. We are not yet enabling full objtool validation on 
> > powerpc, so I think we can revisit this at that point.
> 
> See also <https://gcc.gnu.org/PR99299> that asks for a __builtin_trap()
> variant that does not terminate control flow ("that is recoverable").

Re comment 9 there, x86 must assume ud2 will abort. If the compiler were
to emit ud2 for non-abort purposes then it needs to somehow annotate
this so that both objtool and the kernel can determine this.

That said; the whole annotate_reachable() thing in our WARN is
superfluous, we should read __bug_table instead. That said, we still
need _ASM_REACHABLE to handle a few noreturn cases, so there is no real
pressing reason to go clean that up. (if only the compiler would tell us
about noreturn)

ARM has an immediate in their break instruction and varies the desired
behaviour depending on the immediate.