lkml.org 
[lkml]   [2020]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: mmotm 2020-05-13-20-30 uploaded (objtool warnings)
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

\
 
 \ /
  Last update: 2020-05-29 18:07    [W:0.089 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site