lkml.org 
[lkml]   [2022]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH v3 11/12] powerpc: Remove unreachable() from WARN_ON()
    On Mon, Jul 04, 2022 at 12:44:30PM +0000, Christophe Leroy wrote:
    >
    >
    > Le 04/07/2022 à 14:05, Peter Zijlstra a écrit :
    > > On Sat, Jun 25, 2022 at 06:46:54AM +0000, Christophe Leroy wrote:
    > >>
    > >>
    > >> Le 24/06/2022 à 20:32, Sathvika Vasireddy a écrit :
    > >>> objtool is throwing *unannotated intra-function call*
    > >>> warnings with a few instructions that are marked
    > >>> unreachable. Remove unreachable() from WARN_ON()
    > >>> to fix these warnings, as the codegen remains same
    > >>> with and without unreachable() in WARN_ON().
    > >>
    > >> Did you try the two exemples described in commit 1e688dd2a3d6
    > >> ("powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with
    > >> asm goto") ?
    > >>
    > >> Without your patch:
    > >>
    > >> 00000640 <test>:
    > >> 640: 81 23 00 84 lwz r9,132(r3)
    > >> 644: 71 29 40 00 andi. r9,r9,16384
    > >> 648: 40 82 00 0c bne 654 <test+0x14>
    > >> 64c: 80 63 00 0c lwz r3,12(r3)
    > >> 650: 4e 80 00 20 blr
    > >> 654: 0f e0 00 00 twui r0,0
    > >>
    > >> 00000658 <test9w>:
    > >> 658: 2c 04 00 00 cmpwi r4,0
    > >> 65c: 41 82 00 0c beq 668 <test9w+0x10>
    > >> 660: 7c 63 23 96 divwu r3,r3,r4
    > >> 664: 4e 80 00 20 blr
    > >> 668: 0f e0 00 00 twui r0,0
    > >> 66c: 38 60 00 00 li r3,0
    > >> 670: 4e 80 00 20 blr
    > >
    > > Per this construct you should do as x86 does and assume twui terminates
    > > control flow and explicitly annotate the WARN case. That is, given the
    > > fact that BUG as no instructions following it, you can't very well
    > > annotate that.
    >
    > That exactly the problem I guess. I'm fine with replacing the
    > unreachable() by __builtin_unreachable() with our __WARN_FLAGS() and
    > BUG() but we will still have a problem with some of the unrachable()
    > that are in core parts of the kernel.
    >
    > Even the ones in arch/powerpc/, they are valid and should remain. The
    > point seems that the generic annotate_unreachable() is wrong for powerpc
    > as is, and activating CONFIG_OBJTOOL lead to bad code generation.

    Right; I'm not against making that depend on yet-another OBJTOOL_$config
    thing.

    > By the way, for which functionnalities of objtool is that analysis
    > necessary ? I understand it is not necessary to mcount accounting, so
    > maybe the not empty annotate_unreachable() should be limited to those
    > those functionnalities ?

    For all the things where it needs to follow control flow, so stack
    validation, ORC generation, unreachable instruction validation etc..

    I'd need to double check code-gen on x86, but the way
    __builtin_unreachable() makes code-gen stop dead, I'm not entirely sure
    it's right for us either.

    > > Alternatively, you can teach objtool to look at __bug_table to
    > > distinguish these cases.
    >
    > Isn't it enough to tell objtool that execution never go past twui, using
    > INSN_BUG ?

    That should work I suppose.

    > By the way, for __WARN_FLAGS, we use the __extable for the continuation.
    > Is objtools able to follow __extable ?

    Yes.

    \
     
     \ /
      Last update: 2022-07-04 16:21    [W:3.188 / U:1.572 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site