Re: [PATCH v2 10/10] objtool: More complex static jump implementation

From: Josh Poimboeuf
Date: Tue Jan 16 2018 - 22:05:51 EST


On Tue, Jan 16, 2018 at 03:28:35PM +0100, Peter Zijlstra wrote:
> When using something like:
>
> -#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
> +#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]) && \
> + (arch_static_assert(), true))
>
> we get an objtool assertion fail like:
>
> kernel/sched/fair.o: warning: objtool: hrtick_update()+0xd: static assert FAIL
>
> where:
>
> 0000000000001140 <hrtick_update>:
> 1140: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
> 1145: c3 retq
> 1146: 48 8b b7 30 09 00 00 mov 0x930(%rdi),%rsi
> 114d: 8b 87 d8 09 00 00 mov 0x9d8(%rdi),%eax
> 1153: 48 0f a3 05 00 00 00 bt %rax,0x0(%rip) # 115b <hrtick_update+0x1b>
> 115a: 00
> 1157: R_X86_64_PC32 __cpu_active_mask-0x4
>
> and:
>
> RELOCATION RECORDS FOR [__jump_table]:
> 0000000000000150 R_X86_64_64 .text+0x0000000000001140
> 0000000000000158 R_X86_64_64 .text+0x0000000000001146
>
> RELOCATION RECORDS FOR [.discard.jump_assert]:
> 0000000000000028 R_X86_64_64 .text+0x000000000000114d
>
> IOW, GCC managed to place the assertion 1 instruction _after_ the
> static jump target.
>
> So while the code generation is fine, the assertion gets placed wrong.
> We can 'fix' this by not only considering the immediate static jump
> locations but also all the unconditional code after it, terminating
> the basic block on any unconditional instruction or branch entry
> point.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

This is pretty similar to something I've been wanting to do, which is to
track all basic blocks. But this is fine enough for now.

One nit, can you rename "branch_target" to "jump_dest" for consistency
with the existing naming?

--
Josh