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 <[email protected]>
> > 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 <[email protected]>
Date: Tue Jan 16 15:27:36 CET 2018
for testing.. not sure we wants this in general
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
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
