On Fri, May 29, 2020 at 05:33:36PM +0200, Peter Zijlstra wrote:
> On Fri, May 29, 2020 at 04:53:25PM +0200, Peter Zijlstra wrote:
> > On Fri, May 29, 2020 at 04:35:56PM +0200, Peter Zijlstra wrote:
>
> > *groan*, this is one of those CONFIG_PROFILE_ALL_BRANCHES builds. If I
> > disable that it goes away.
> >
> > Still trying to untangle the mess it generated, but on first go it
> > looks like objtool is right, but I'm not sure what went wrong.
>
> $ tools/objtool/objtool check -fab arch/x86/lib/csum-wrappers_64.o
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0x29f: call to memset() with UACCESS enabled
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0x283: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0x113: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: .altinstr_replacement+0xffffffffffffffff: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0xea: (alt)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: .altinstr_replacement+0xffffffffffffffff: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0xe7: (alt)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0xd2: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0x7e: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0x43: (branch)
> arch/x86/lib/csum-wrappers_64.o: warning: objtool: csum_and_copy_from_user()+0x0: <=== (sym)
>
> The problem is with the +0x113 branch, which is at 0x1d1.
>
> That looks to be:
>
> if (!likely(user_access_begin(src, len)))
> goto out_err;
>
> Except that the brach profiling stuff confused GCC enough to leak STAC
> into the error path or something.
It looks to me like GCC is doing the right thing. That likely()
translates to:
# define likely(x) (__branch_check__(x, 1, __builtin_constant_p(x)))
which becomes:
#define __branch_check__(x, expect, is_constant) ({ \
long ______r; \
static struct ftrace_likely_data \
__aligned(4) \
__section(_ftrace_annotated_branch) \
______f = { \
.data.func = __func__, \
.data.file = __FILE__, \
.data.line = __LINE__, \
}; \
______r = __builtin_expect(!!(x), expect); \
ftrace_likely_update(&______f, ______r, \
expect, is_constant); \
______r; \
})
Here 'x' is the call to user_access_begin(). It evaluates 'x' -- and
thus calls user_access_begin() -- before the call to
ftrace_likely_update().
So it's working as designed, right? The likely() just needs to be
changed to likely_notrace().
--
Josh
