lkml.org 
[lkml]   [2016]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] x86/asm/entry: fix stack return address retrieval in thunk
    From
    On Tue, May 17, 2016 at 7:43 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
    >
    > index 98df1fa..dae7ca0 100644
    > --- a/arch/x86/entry/thunk_64.S
    > +++ b/arch/x86/entry/thunk_64.S
    > @@ -15,9 +15,10 @@
    > .globl \name
    > .type \name, @function
    > \name:
    > + /* push 1 register if frame pointers are enabled */
    > FRAME_BEGIN
    >
    > - /* this one pushes 9 elems, the next one would be %rIP */
    > + /* push 9 registers */

    I don't hate this patch, but quite frankly, as with the other case,
    I'd just make the frame pointer be unconditional in this case.

    If we push nine other registers, the frame pointer setup code is *not*
    going to matter.

    The reason to avoid frame pointers in code generation is two-fold:

    1) for small leaf functions, it often ends up dominating

    2) it removes a register that is otherwise usable, which can be
    particularly bad on 32-bit x86 due to the much more limited number of
    registers (and was apparently really noticeable on the older on-order
    atom cores)

    and in this case neither of them is really an issue.

    So I would suggest that any case that actually depends on a frame
    access just make the frame pointer not just unconditional, but
    _explicit_.

    So not just avoiding the macro because it's conditional, but write out
    the sequence to actually set up the frame, and then use

    - movq 9*8(%rsp), %rdi
    + movq 8(%rbp), %rdi # return address

    to entirely avoid all kind of "how many registers have we pushed" math.

    Considering that we got this wrong in two places, it's clearly too
    subtle for our little brains as-is.

    Linus

    \
     
     \ /
      Last update: 2016-05-17 18:41    [W:6.308 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site