Re: [GIT PULL] ftrace/x86: Add frames pointers to trampoline as necessary

[Steven Rostedt]

On Fri, 21 Nov 2014 17:35:10 -0800
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Fri, Nov 21, 2014 at 6:50 AM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> > +.macro create_frame parent rip
> > +#ifdef CC_USING_FENTRY
> > +       pushq \parent
> > +       pushq %rbp
> > +       movq %rsp, %rbp
> 
> This is a very strange frame.
> 
> Why do you do the "pushq \parent" at all? Why isn't this just a *real*
> frame and add it to MCOUNT_SAVE_FRAME.
> 
> You seem to create this fake frame-within-a-frame thing. It's not clear why.
> 

Honestly, I did it this way because it was the simplest way to do it.

There's three different setups this covers:

1) frame pointers and mcount

2) frame pointers and fentry

3) no frame pointers and fentry

(mcount forces frame pointers, so there's no "no frame pointers and
mcount)

You probably know all this but I'm writing this for anyone else that
might be reading this. And also let you know what I understand, which
might be wrong too ;-)

With #3 we don't need to do anything. But when frame pointers
are enabled, we have in the main code:

1) mcount

func:
		push %rbp
		mov %rsp %rbp
		[...]
		call mcount

2) fentry

func:
		call fentry
		push %rbp
		mov %rsp %rbp


On mcount, the frame pointer of the parent is pushed to the stack, with
fentry it is not.

Now when fentry is called, the stack is:

	address of return-to-parent-addr
	address of return-to-func-addr

 If fentry does just:

fentry:
		push %rbp
		mov  %rsp %rbp

Then the stack trace will see after %rbp the return to func, but it
will never see the return to parent, and the parent will be missing
from the stack trace.

At a minimum, fentry needs:

fentry:
		push 8(%rsp)    // Return to parent, not func
		push %rbp
		mov  %rsp %rbp
		push 16(%rbp)	// Return to func:
		push %rbp
		mov  %rsp %rbp

With mcount, the call is done after the parent frame pointer is setup,
so we only really need to

mcount:
		push %rbp
		mov  %rsp %rbp

My version adds an extra push %rip in the mcount case but is the same
in the fentry case which is now the default.

The ftrace_caller_setup took a little bit to get right. I'm nervous
about modifying that and breaking something. Better yet, after that is
called, the parent rip is placed in %rsi and the func rip is placed in
%rdi. By doing it the way I did it, it made it nice and convenient that
I could just make a couple of macros that handle all three cases.

If you want, I can add more comments to explain all this.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/