The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Pavel Machek <pavel@ucw.cz>,
	kernel list <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Lutomirski <luto@kernel.org>,
	Borislav Petkov <bp@alien8.de>, Brian Gerst <brgerst@gmail.com>,
	Denys Vlasenko <dvlasenk@redhat.com>, Peter Anvin <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: v4.10: kernel stack frame pointer .. has bad value (null)
Date: Tue, 7 Mar 2017 12:28:55 -0600	[thread overview]
Message-ID: <20170307182855.262ezbon2pm67qfd@treble> (raw)
In-Reply-To: <CALCETrWHdra-Po0rNrHn-cV2G=AM2pWR45rRf4t2RZfQR8Lqjg@mail.gmail.com>

On Tue, Mar 07, 2017 at 09:59:44AM -0800, Andy Lutomirski wrote:
> On Tue, Mar 7, 2017 at 9:52 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Tue, Mar 7, 2017 at 9:38 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >>
> >> So I'm thinking we should have -maccumulate-outgoing-args always enabled
> >> on x86_32 just like we already do on x86_64.
> >
> > Ugh. I realize we have workarounds for bugs, but I think
> > -maccumulate-outgoing-args is nasty. It just generates worse code by
> > avoiding the much nicer push/pop sequences, afaik.

Yes, maybe the pushes/pops around a function call are a little easier to
read than movs.

But the -maccumulate-outgoing-args realignment prologue is a *lot* worse
for readability, IMO.

Also, the gcc documentation says -maccumulate-outgoing-args is
"generally beneficial for performance and size."

Not to mention the fact that -maccumulate-outgoing-args seems to already
be enabled in most cases anyway.  Having it uniformly enabled everywhere
makes it less confusing overall when the rare divergences are
encountered.  From looking at some of the changes related to
ADD_ACCUMULATE_OUTGOING_ARGS in arch/x86/Makefile_32.cpu, I can tell
that several others before me have stumbled into this prologue issue.

> > On x86-64 it's not such a big deal, because we pass the first six
> > arguments in registers anyway, so the arguments on the stack is a
> > fairly unusual special case.
> >
> > But on x86-32, we only have three argument registers, so this
> > braindamage is potentially worse.
> >
> > I guess we already do this in most situations due to the gcc bugs, but
> > I do think it's sad that we would do it for our _own_ bugs too.
> >
> 
> Is it our bug or a gcc bug?  I would have thought
> -fno-omit-frame-pointer meant that the call-frame-to-return-address
> offset should be constant and -fomit-frame-pointer meant "do
> whatever".

I don't think it's a gcc bug because it doesn't seem to violate frame
pointer conventions:

    pushl -0x4(%edi)	# copy return address
    push %ebp

The frame pointer and return address are still stored adjacently.  And
it normally allows unwinds to work fine.

The problem is the kernel unwinder's assumption that the last frame
pointer is at a certain address.  That assumption breaks with the DRAP
prologue.

> Also, maybe I'm missing something, but does gcc's code even allow the
> function to return sensibly?  It could do it by a nasty calculation
> involving backing out the old esp from edi, but that seems quite
> overcomplicated.

That's what it does:

    lea -0x8(%edi),%esp
    pop %edi
    ret

-- 
Josh

  reply	other threads:[~2017-03-07 18:37 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-21 22:14 v4.10: kernel stack frame pointer .. has bad value (null) Pavel Machek
2017-02-21 23:12 ` Josh Poimboeuf
2017-02-21 23:15   ` H. Peter Anvin
2017-02-22 16:45     ` Josh Poimboeuf
2017-02-22 20:51       ` H. Peter Anvin
2017-02-22 21:15         ` Josh Poimboeuf
2017-02-22 21:05   ` Pavel Machek
2017-02-22 21:21     ` Josh Poimboeuf
2017-02-22 22:47       ` Pavel Machek
2017-02-22 22:56         ` Josh Poimboeuf
2017-02-22 23:18           ` Josh Poimboeuf
2017-02-23 20:10             ` Pavel Machek
2017-02-25  5:04               ` Josh Poimboeuf
2017-03-02 23:45                 ` Josh Poimboeuf
2017-03-06 16:38                   ` Pavel Machek
2017-03-07 17:38                     ` Josh Poimboeuf
2017-03-07 17:52                       ` Linus Torvalds
2017-03-07 17:59                         ` Andy Lutomirski
2017-03-07 18:28                           ` Josh Poimboeuf [this message]
2017-03-07 18:30                             ` Josh Poimboeuf
2017-03-07 18:40                             ` Linus Torvalds
2017-03-08 17:37                               ` Josh Poimboeuf
2017-03-08 18:25                                 ` Linus Torvalds
2017-03-08 18:54                                   ` Andy Lutomirski
2017-03-08 21:22                                   ` Pavel Machek
2017-03-09  9:38                                     ` Geert Uytterhoeven
2017-03-09 10:56                                       ` Pavel Machek
2017-03-09 12:16                                         ` Geert Uytterhoeven
2017-03-10 13:17                                           ` Compiling kernels faster (was Re: v4.10: kernel stack frame pointer .. has bad value (null)) Pavel Machek
2017-03-10 13:28                                             ` Geert Uytterhoeven
2017-03-10 14:15                                             ` Willy Tarreau
2017-03-09 10:49                                     ` Old compiler versions " Pavel Machek
2017-03-09 18:05                                       ` Linus Torvalds
2017-03-09 15:29                                     ` v4.10: kernel stack frame pointer .. has bad value (null) Peter Zijlstra
2017-03-09 21:12                                       ` Pavel Machek
2017-03-08 21:29                                   ` Josh Poimboeuf
2017-03-09 14:14                                     ` Steven Rostedt
2017-03-09 18:31                                       ` Josh Poimboeuf
2017-03-16 15:42                                     ` [PATCH] x86: mostly disable '-maccumulate-outgoing-args' Josh Poimboeuf
2017-03-16 17:32                                       ` Steven Rostedt
2017-03-16 18:36                                         ` Josh Poimboeuf
2017-03-16 18:53                                           ` Josh Poimboeuf
2017-03-16 19:04                                             ` Josh Poimboeuf
2017-03-16 19:07                                               ` Steven Rostedt
2017-03-16 19:06                                           ` Steven Rostedt
2017-03-16 19:31                                       ` [PATCH v2] " Josh Poimboeuf
2017-03-22  7:51                                         ` Ingo Molnar
2017-03-22 15:48                                           ` Josh Poimboeuf
2017-03-28  8:13                                         ` [tip:x86/urgent] x86/build: Mostly " tip-bot for Josh Poimboeuf
2017-03-28 16:17                                           ` Josh Poimboeuf
2017-03-30  9:58                                         ` tip-bot for Josh Poimboeuf

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=20170307182855.262ezbon2pm67qfd@treble \
    --to=jpoimboe@redhat.com \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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