Re: [PATCH 3/3] ftrace/x86: Set ftrace_stub to weak to prevent gcc from using short jumps to it

From: Namhyung Kim
Date: Sun Jun 19 2016 - 21:16:44 EST


Hi Steve,

On Sun, May 22, 2016 at 04:28:49PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
>
> Matt Fleming reported seeing crashes when enabling and disabling
> function profiling which uses function graph tracer. Later Namhyung Kim
> hit a similar issue and he found that the issue was due to the jmp to
> ftrace_stub in ftrace_graph_call was only two bytes, and when it was
> changed to jump to the tracing code, it overwrote the ftrace_stub that
> was after it.
>
> Masami Hiramatsu bisected this down to a binutils change:
>
> 8dcea93252a9ea7dff57e85220a719e2a5e8ab41 is the first bad commit
> commit 8dcea93252a9ea7dff57e85220a719e2a5e8ab41
> Author: H.J. Lu <hjl.tools@xxxxxxxxx>
> Date: Fri May 15 03:17:31 2015 -0700
>
> Add -mshared option to x86 ELF assembler
>
> This patch adds -mshared option to x86 ELF assembler. By default,
> assembler will optimize out non-PLT relocations against defined non-weak
> global branch targets with default visibility. The -mshared option tells
> the assembler to generate code which may go into a shared library
> where all non-weak global branch targets with default visibility can
> be preempted. The resulting code is slightly bigger. This option
> only affects the handling of branch instructions.
>
> Declaring ftrace_stub as a weak call prevents gas from using two byte
> jumps to it, which would be converted to a jump to the function graph
> code.
>
> Link: http://lkml.kernel.org/r/20160516230035.1dbae571@xxxxxxxxxxxxxxxxxx

Shouldn't it go to the stable tree?

Thanks,
Namhyung


>
> Reported-by: Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx>
> Reported-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> Tested-by: Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> ---
> arch/x86/kernel/mcount_64.S | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/mcount_64.S
> index ed48a9f465f8..61924222a9e1 100644
> --- a/arch/x86/kernel/mcount_64.S
> +++ b/arch/x86/kernel/mcount_64.S
> @@ -182,7 +182,8 @@ GLOBAL(ftrace_graph_call)
> jmp ftrace_stub
> #endif
>
> -GLOBAL(ftrace_stub)
> +/* This is weak to keep gas from relaxing the jumps */
> +WEAK(ftrace_stub)
> retq
> END(ftrace_caller)
>
> --
> 2.8.0.rc3
>
>