From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Julien Thierry <jthierry@redhat.com>
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
mbenes@suse.cz, raphael.gault@arm.com, benh@kernel.crashing.org
Subject: Re: [PATCH v2 1/3] objtool: check: Fully validate the stack frame
Date: Tue, 29 Sep 2020 14:18:34 -0500 [thread overview]
Message-ID: <20200929191834.7daofidv6b5aef3y@treble> (raw)
In-Reply-To: <20200928093631.210610-2-jthierry@redhat.com>
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
?
> +++ 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.
--
Josh
next prev parent reply other threads:[~2020-09-29 19:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-28 9:36 [PATCH v2 0/3] objtool: Extend CFA updating/checking Julien Thierry
2020-09-28 9:36 ` [PATCH v2 1/3] objtool: check: Fully validate the stack frame Julien Thierry
2020-09-29 19:18 ` Josh Poimboeuf [this message]
2020-10-12 10:21 ` Julien Thierry
2020-10-12 15:35 ` Josh Poimboeuf
2020-10-13 12:12 ` Julien Thierry
2020-09-28 9:36 ` [PATCH v2 2/3] objtool: check: Support addition to set CFA base Julien Thierry
2020-09-28 9:36 ` [PATCH v2 3/3] objtool: check: Make SP memory operation match PUSH/POP semantics Julien Thierry
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200929191834.7daofidv6b5aef3y@treble \
--to=jpoimboe@redhat.com \
--cc=benh@kernel.crashing.org \
--cc=jthierry@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=peterz@infradead.org \
--cc=raphael.gault@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®