[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/elf: Remove ASM_CALL_CONSTRAINT from elf_core_save_regs()
On 26.03.2025 10:17, Andrew Cooper wrote: > On 26/03/2025 9:00 am, Jan Beulich wrote: >> On 25.03.2025 19:00, Andrew Cooper wrote: >>> I was mistaken about when ASM_CALL_CONSTRAINT is applicable. It is not >>> applicable for plain pushes/pops, so remove it from the flags logic. >>> >>> Clarify the description of ASM_CALL_CONSTRAINT to be explicit about >>> unwinding >>> using framepointers. >>> >>> Fixes: 0754534b8a38 ("x86/elf: Improve code generation in >>> elf_core_save_regs()") >>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> >>> --- >>> CC: Jan Beulich <JBeulich@xxxxxxxx> >>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> >>> --- >>> xen/arch/x86/include/asm/asm_defns.h | 5 +++-- >>> xen/arch/x86/include/asm/x86_64/elf.h | 2 +- >>> 2 files changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/x86/include/asm/asm_defns.h >>> b/xen/arch/x86/include/asm/asm_defns.h >>> index 92b4116a1564..689d1dcbf754 100644 >>> --- a/xen/arch/x86/include/asm/asm_defns.h >>> +++ b/xen/arch/x86/include/asm/asm_defns.h >>> @@ -28,8 +28,9 @@ asm ( "\t.equ CONFIG_INDIRECT_THUNK, " >>> >>> /* >>> * This output constraint should be used for any inline asm which has a >>> "call" >>> - * instruction. Otherwise the asm may be inserted before the frame pointer >>> - * gets set up by the containing function. >>> + * instruction, which forces the stack frame to be set up prior to the asm >>> + * block. This matters when unwinding using framepointers, where the asm's >>> + * function can get skipped over. >> Does "forces the stack frame to be set up" really mean the stack frame, or >> the >> frame pointer (if one is in use)? > > What do you consider to be the difference, given how frame pointers work > in our ABI? My point is that frame pointers are an optional part. Sufficiently high optimization levels omit them by default, iirc. And depending on CONFIG_FRAME_POINTER we may explicitly pass -fno-omit-frame-pointer. Even in that case there is a stack frame that the compiler is setting up. Yet in that case the effect of ASM_CALL_CONSTRAINT is not relevant. Hence also why the construct expands to nothing in that case. The comment, however, is placed outside if the #ifdef, and hence applies to both forms (according to the way I read such, at least). > It is the frame pointer which needs setting up, which at a minimum > involves spilling registers to the stack and getting %rsp into it's > eventual position. Right, and all I'm effectively asking for is s/stack frame/frame pointer/ in the new comment text. Then: Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> Alternatively part or all of the comment could be moved inside the #ifdef. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |