Re: [RFC][PATCH] objtool,ftrace: Implement UNWIND_HINT_RET_OFFSET

[Josh Poimboeuf]

On Tue, Mar 31, 2020 at 01:16:52PM +0200, Peter Zijlstra wrote:
> Subject: objtool,ftrace: Implement UNWIND_HINT_RET_OFFSET
> 
> This replaces the SAVE/RESTORE hints with a RET_OFFSET hint that applies
> to the following instructions:
> 
>  - any instruction that terminates a function, like: RETURN and sibling
>    calls. It allows the stack-frame to be off by @sp_offset, ie. it
>    allows stuffing the return stack.
> 
>  - EXCEPTION_RETURN (a new INSN_type that splits IRET out of
>    CONTEXT_SWITCH) and here it denotes a @sp_offset sized POP and makes
>    the instruction continue.

Looking closer, I see how my UNWIND_HINT_ADJUST idea doesn't work for
the ftrace_regs_caller() case.  The ORC data is actually correct there.
So basically we need a way to tell objtool to be quiet.

I now understand what you're trying to do with the RET_TAIL thing, and I
guess it's ok for the ftrace case.  But I'd rather an UNWIND_HINT_IGNORE
before the tail cail, which would tell objtool to just silence the tail
call warning.  It's simpler for the user to understand, it's simpler
logic in objtool, and I think an "ignore warnings for the next insn"
hint would be more generally applicable anyway.

But also... the RET_OFFSET usage for sync_core() *really* bugs me.

I know you said it's like an indirect tail call with a bigger frame, but
that's kind of stretching it because the function frame is still there.

And objtool doesn't treat it like a tail call at all.  In fact, it
handles it *completely* differently from the normal ret-tail-call case.
Instead of silencing a tail call warning, it adjusts the stack offset
and continues the code path.

This basically adds *two* new hint types, while trying to call them the
same thing.  There's no overlapping functionality between them in
objtool, other than the use of the same insn->ret_offset variable.  But
it's two distinct functionalities, depending on the context (return/tail
vs IRETQ).

I'll try to work up some patches with a different approach in a bit.

-- 
Josh