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:

quoted

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.