lkml.org 
[lkml]   [2018]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v2 11/10] objtool: Even more complex static block checks
    On Tue, Jan 16, 2018 at 09:12:32PM -0600, Josh Poimboeuf wrote:
    > On Tue, Jan 16, 2018 at 08:49:17PM +0100, Peter Zijlstra wrote:
    > > Subject: objtool: Even more complex static block checks
    > > From: Peter Zijlstra <peterz@infradead.org>
    > > Date: Tue Jan 16 20:17:01 CET 2018
    > >
    > > I've observed GCC transform:
    > >
    > > f()
    > > {
    > > if (!static_branch_unlikely())
    > > return;
    > >
    > > static_assert();
    > > A;
    > > }
    > >
    > > g()
    > > {
    > > f();
    > > }
    > >
    > > Into:
    > >
    > > f()
    > > {
    > > static_assert();
    > > A;
    > > }
    > >
    > > g()
    > > {
    > > if (static_branch_unlikely())
    > > f();
    > > }
    > >
    > > Which results in the assertion landing at f+0. The transformation is
    > > valid and useful; it avoids a pointless CALL+RET sequence, so we'll
    > > have to teach objtool how to deal with this.
    > >
    > > Do this by marking all CALL destinations with static_call when called
    > > from a static_block and non_static_call when called outside a
    > > static_block. This allows us to identify functions called exclusively
    > > from a static_block and start them with a static_block.
    >
    > Ew... where'd you place the assertion to trigger this?

    Its the patch I pastebin'ed you earlier, also see below.

    > It's late and my brain has already clocked out, so I'll need to revisit
    > this tomorrow. But now I'm wondering if my basic block idea would be a
    > better way to solve this.

    I would think basic-blocks are inside functions, and this patch goes
    across functions, something you'd still need even if you had basic
    blocks.

    Also, basic blocks are non-trivial because they can overlap. I've
    implemented something like that before for perf, see commit:

    70fbe0574558 ("perf annotate: Add branch stack / basic block")

    We could probably lift that code fairly easily.

    ---
    Subject: jump_label: Add static assertion to every static_branch
    From: Peter Zijlstra <peterz@infradead.org>
    Date: Tue Jan 16 15:27:36 CET 2018

    for testing.. not sure we wants this in general

    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    ---
    arch/x86/include/asm/jump_label.h | 1 +
    include/linux/jump_label.h | 8 ++++++--
    2 files changed, 7 insertions(+), 2 deletions(-)

    --- a/arch/x86/include/asm/jump_label.h
    +++ b/arch/x86/include/asm/jump_label.h
    @@ -76,6 +76,7 @@ static __always_inline bool arch_static_
    *
    * Also works with static_cpu_has().
    */
    +#define arch_static_assert arch_static_assert
    static __always_inline void arch_static_assert(void)
    {
    asm volatile ("1:\n\t"
    --- a/include/linux/jump_label.h
    +++ b/include/linux/jump_label.h
    @@ -323,6 +323,10 @@ extern bool ____wrong_branch_error(void)

    #ifdef HAVE_JUMP_LABEL

    +#ifndef arch_static_assert
    +#define arch_static_assert (void)
    +#endif
    +
    /*
    * Combine the right initial value (type) with the right branch order
    * to generate the desired result.
    @@ -388,7 +392,7 @@ extern bool ____wrong_branch_error(void)
    branch = !arch_static_branch_jump(&(x)->key, true); \
    else \
    branch = ____wrong_branch_error(); \
    - branch; \
    + branch && (arch_static_assert(), true); \
    })

    #define static_branch_unlikely(x) \
    @@ -400,7 +404,7 @@ extern bool ____wrong_branch_error(void)
    branch = arch_static_branch(&(x)->key, false); \
    else \
    branch = ____wrong_branch_error(); \
    - branch; \
    + branch && (arch_static_assert(), true); \
    })

    #else /* !HAVE_JUMP_LABEL */
    > Josh
    \
     
     \ /
      Last update: 2018-01-18 00:20    [W:4.066 / U:0.304 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site