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

From: Peter Zijlstra <[email-masked]>
Date: Mon, 4 Jul 2022 13:43:15 +0200

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(),

Yes, ud2 is the traditional 'kill' instruction and a number of emulators
treat it as such, however it also being the shortest encoding (2 bytes)
for #UD Linux has opted to (ab)use it to implement WARN/BUG.

As such interpretation of 'ud2' needs to assume control flow stops
(compiler will also emit ud2 in a number of cases with that intent).
However, if it's used as WARN we then need to annotate the thing to not
be terminal.

> 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.

Careful.. INSN_TRAP is mostly used for purposes of speculation stop and
padding. That is, INSN_TRAP does indeed not affect control flow, but the
way objtool treats it might not be quite what you want.

Specifically, straight-line-speculation checks want INT3 after indirect
control transfers (indirect jump and return -- indirect call is
'difficult'); these locations are architecturally not executed and as
such placing a random trap instruction there is 'harmless'. Of course,
were the branch predictor to go wobbly and attempt to execute it, the
fact that it's a trap will stop speculation dead.

Additionally, int3, being a single byte instruction, is also used to
fill dead code space, any #BP trap on it will not have a descriptor and
mostly cause the kernel to go splat.

Per the last usage, validate_reachable_instructions() will ignore it.
I'm not sure you want to always ignore all your (unreachable) trap
instructions.