Messages in this thread Patch in this message | | | Date | Wed, 17 Jan 2018 09:13:09 +0100 | From | Peter Zijlstra <> | Subject | Re: [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
| |