lkml.org 
[lkml]   [2020]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 1/3] objtool: check: Fully validate the stack frame
    From
    Date


    On 9/29/20 8:18 PM, Josh Poimboeuf wrote:
    > On Mon, Sep 28, 2020 at 10:36:29AM +0100, Julien Thierry wrote:
    >> +++ b/tools/objtool/arch/x86/include/cfi_regs.h
    >> @@ -22,4 +22,7 @@
    >> #define CFI_RA 16
    >> #define CFI_NUM_REGS 17
    >
    > A few more naming nitpicks:
    >
    >> +#define STACKFRAME_BP_OFFSET -16
    >> +#define STACKFRAME_RA_OFFSET -8
    >
    > "Stack frame" has more than one meaning now, I suppose. i.e. it could
    > also include the callee-saved registers and any other stack space
    > allocated by the function.
    >
    > Would "call frame" be clearer?
    >
    > CALL_FRAME_BP_OFFSET
    > CALL_FRAME_RA_OFFSET
    >
    > ?

    I would've thought that the call-frame could include the stackframe +
    other callee saved regs. Whereas stackframe tends to used for the
    caller's frame pointer + return address (i.e. what allows unwinding).
    Unless I'm getting lost with things.

    And if call frame is associated with the region starting from the stack
    pointer at the parent call point (since this is what CFA is), then it
    shouldn't be associated with the framepointer + return address structure
    since this could be anywhere on the call frame (not at a fixed offset)
    as long as the new frame pointer points to the structure.

    >
    >> +++ b/tools/objtool/cfi.h
    >> @@ -35,4 +35,6 @@ struct cfi_state {
    >> bool end;
    >> };
    >>
    >> +#define STACKFRAME_SIZE 16
    >
    > CALL_FRAME_SIZE ?
    >
    > I'm sort of contradicting my previous comment here, but even though this
    > value may be generic, it's also very much intertwined with the
    > CALL_FRAME_{BP|RA}_OFFSET values. So I get the feeling it really
    > belongs in the arch-specific cfi_regs.h next to the other defines after
    > all.
    >

    Agreed.

    --
    Julien Thierry

    \
     
     \ /
      Last update: 2020-10-12 12:23    [W:6.529 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site