Messages in this thread | | | Date | Tue, 31 Mar 2020 15:23:15 -0500 | From | Josh Poimboeuf <> | Subject | Re: [RFC][PATCH] objtool,ftrace: Implement UNWIND_HINT_RET_OFFSET |
| |
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
| |