mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Richard Guenther <richard.guenther@gmail.com>
Cc: rostedt@goodmis.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@elte.hu>, "H. Peter Anvin" <hpa@zytor.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	feng.tang@intel.com, Fr??d??ric Weisbecker <fweisbec@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	jakub@redhat.com, gcc@gcc.gnu.org
Subject: Re: BUG: GCC-4.4.x changes the function frame on some functions
Date: Thu, 19 Nov 2009 10:38:59 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.0911191026420.2793@localhost.localdomain> (raw)
In-Reply-To: <84fc9c000911191003t244eb864o3d5b355ab5485f@mail.gmail.com>



On Thu, 19 Nov 2009, Richard Guenther wrote:
> 
> Note that I only can reproduce the issue with
> -mincoming-stack-boundary=2, not with -mpreferred-stack-boundary=2.

Since you can reproduce it with -mincoming-stack-boundary=2, I woul 
suggest just fixing mcount handling that way regardless of anything else. 
The current code generated by gcc is just insane - even for the case where 
you _want_ 16-byte stack alignment.

Instead crazy code like

>       push   %edi
>       lea    0x8(%esp),%edi
>       and    $0xfffffff0,%esp
>       pushl  -0x4(%edi)
>       push   %ebp
>       mov    %esp,%ebp
>       ...
>       call   mcount

the sane thing to do would be to just do it as

	push   %ebp
	mov    %esp,%ebp
	call   mcount
	and    $0xfffffff0,%esp

since

 - no sane 'mcount' implementation can ever care about 16-byte stack 
   alignment anyway, so aliging the stack before mcount is crazy.

 - mcount is special anyway, and is the only thing that cares about that 
   whole ebp/return address thing is mcount, and _all_ your games with 
   %edi are about that mcount thing.

IOW, once you as a compiler person understand that the 'mcount' call is 
special, you should have realized that all the work you did for it was 
totally pointless and stupid to begin with. 

You must already have that special mcount logic (the whole code to save a 
register early and push the fake mcount stack frame), so instead of _that_ 
special logic, change it to a different mcount special logic that 
associates the 'mcount' call with theframe pointer pushing. 

That will not only make the Linux kernel tracer happy, it will make all 
your _other_ users happier too, since you can generate smaller and more 
efficient code.

Admittedly, anybody who compiles with -pg probably doesn't care deeply 
about smaller and more efficient code, since the mcount call overhead 
tends to make the thing moot anyway, but it really looks like a win-win 
situation to just fix the mcount call sequence regardless.

> And you didn't provide us with a testcase either ... so please open a 
> bugzilla and attach preprocessed source of a file that shows the 
> problem, note the function it happens in and provide the command-line 
> options you used for building.
> 
> Otherwise it's going to be all speculation on our side.

See above - all you need to do is to just fix mcount calling.

Now, there is a separate bug that shows that you seem to over-align the 
stack when not asked for, and yes, since we noticed that I hope that 
Thomas and friends will fix that, but I think your mcount logic could (and 
should) be fixed as an independent sillyness.

			Linus

  parent reply	other threads:[~2009-11-19 18:41 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200911172214.nAHMEBh2023787@imap1.linux-foundation.org>
2009-11-18 19:30 ` [patch for 2.6.32? 1/3] hrtimers: remove the "timer_stats_active" check when setting the start info Thomas Gleixner
2009-11-18 20:24   ` [tip:timers/urgent] hrtimer: Fix /proc/timer_list regression tip-bot for Feng Tang
2009-11-19  7:20     ` Ingo Molnar
2009-11-19 10:05       ` Thomas Gleixner
2009-11-19 14:30         ` BUG: function graph tracer function frame assumptions Thomas Gleixner
2009-11-19 15:37           ` BUG: GCC-4.4.x changes the function frame on some functions Thomas Gleixner
2009-11-19 15:44             ` Andrew Haley
2009-11-19 15:54               ` H. Peter Anvin
2009-11-19 15:57                 ` Richard Guenther
2009-11-19 16:02               ` Steven Rostedt
2009-11-19 16:11                 ` H. Peter Anvin
2009-11-19 16:19                 ` Frederic Weisbecker
2009-11-19 16:06               ` Thomas Gleixner
2009-11-19 16:17                 ` Andrew Haley
2009-11-19 16:43                   ` Thomas Gleixner
2009-11-19 16:12               ` Steven Rostedt
2009-11-19 15:45             ` H. Peter Anvin
2009-11-19 15:49               ` Richard Guenther
2009-11-19 15:52                 ` Richard Guenther
2009-11-19 17:37                 ` Andi Kleen
2009-11-19 17:39             ` Linus Torvalds
2009-11-19 17:51               ` Thomas Gleixner
2009-11-19 17:59               ` Steven Rostedt
2009-11-19 18:03                 ` Richard Guenther
2009-11-19 18:22                   ` Andrew Haley
2009-11-19 18:41                     ` Linus Torvalds
2009-11-19 18:43                       ` Linus Torvalds
2009-11-19 18:54                         ` Linus Torvalds
2009-11-19 19:01                           ` Thomas Gleixner
2009-11-23  9:16                             ` Jakub Jelinek
2009-11-23  9:51                               ` Thomas Gleixner
2009-11-19 19:10                           ` David Daney
2009-11-19 19:28                             ` Steven Rostedt
2009-11-19 19:46                               ` Frederic Weisbecker
2009-11-19 19:54                                 ` Kai Tietz
2009-11-19 20:05                                   ` Frederic Weisbecker
2009-11-19 20:05                                 ` Steven Rostedt
2009-11-19 20:17                                   ` Steven Rostedt
2009-11-19 20:28                                     ` Frederic Weisbecker
2009-11-19 20:25                                   ` Frederic Weisbecker
2009-11-19 20:36                                     ` Linus Torvalds
2009-11-19 20:44                                       ` Steven Rostedt
2009-11-19 19:50                               ` H. Peter Anvin
2009-11-19 20:06                                 ` Linus Torvalds
2009-11-19 21:12                                   ` Jeff Law
2009-11-19 20:10                                 ` Steven Rostedt
2009-11-19 21:05                                 ` Jeff Law
2009-11-19 18:31                   ` Thomas Gleixner
2009-11-19 18:38                   ` Linus Torvalds [this message]
2009-11-19 18:47                     ` Ingo Molnar
2009-11-19 19:06                       ` Steven Rostedt
2009-11-19 19:50                         ` Ingo Molnar
2009-11-20  9:57                         ` [PATCH] gcc mcount-nofp was " Andi Kleen
2009-11-20 12:34                           ` Steven Rostedt
2009-11-20 19:06                             ` H. Peter Anvin
2009-11-19 20:36                     ` Thomas Gleixner
2009-11-19 18:20             ` Andrew Haley
2009-11-19 18:33               ` Steven Rostedt
2009-11-19 18:36                 ` Andrew Pinski
2009-11-19 18:36                 ` Andrew Haley
2009-11-19 18:37                 ` H. Peter Anvin
2009-11-19 18:39               ` Thomas Gleixner
2009-11-20  5:23           ` [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect GCC messing with mcount prologue Steven Rostedt
2009-11-20  5:32             ` Steven Rostedt
2009-11-20 17:00             ` Steven Rostedt
2009-11-20 17:13               ` H. Peter Anvin
2009-11-20 19:35               ` Andrew Haley
2009-11-20 19:46                 ` Steven Rostedt
2009-11-20 19:49                   ` H. Peter Anvin
2009-11-22  9:38                 ` H.J. Lu
2009-11-22 17:20                   ` Andrew Haley
2009-11-22 23:30                     ` H.J. Lu
2009-11-24 14:43                       ` Andrew Haley
2009-11-24 14:55                         ` Thomas Gleixner
2009-11-24 15:06                           ` Jakub Jelinek
2009-11-24 15:32                             ` Andrew Haley
2009-11-24 15:36                               ` Jakub Jelinek
2009-11-24 15:46                                 ` Andrew Haley
2009-11-24 16:38                                   ` H. Peter Anvin
2009-11-24 17:12                                     ` Andrew Haley
2009-11-24 17:30                                       ` Steven Rostedt
2009-11-25 20:05                                         ` H. Peter Anvin
2009-11-24 19:55                                       ` H. Peter Anvin
2009-11-25 15:29                             ` Thomas Gleixner
2009-11-25 15:44                               ` Ingo Molnar
2009-11-25 15:53                                 ` Thomas Gleixner
2009-11-25 16:25                                   ` Ingo Molnar
2009-11-25 16:44                                 ` Jakub Jelinek
2009-11-25 20:12                                   ` H. Peter Anvin
2009-11-25 21:00                                     ` Andrew Haley
2009-11-22  9:05               ` Ingo Molnar
2009-11-20 10:30   ` [tip:timers/urgent] hrtimer: Fix /proc/timer_list regression tip-bot for Feng Tang
2009-11-20 14:19     ` Heiko Carstens
2009-11-19 20:48 BUG: GCC-4.4.x changes the function frame on some functions H. Peter Anvin
2009-11-19 21:14 H. Peter Anvin
2009-11-19 21:25 ` Jeff Law
2009-11-19 22:43   ` Steven Rostedt
2009-11-19 23:58     ` Jeff Law
2009-11-20  0:36       ` Thomas Gleixner
2009-11-20  0:59         ` Linus Torvalds
2009-11-20  1:27           ` Thomas Gleixner
2009-11-20  2:14             ` Thomas Gleixner
2009-11-20  1:29           ` H. Peter Anvin
2009-11-20  5:36           ` Ingo Molnar
2009-11-20 12:04         ` Andrew Haley
2009-11-20 12:22           ` Andrew Haley

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=alpine.LFD.2.00.0911191026420.2793@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=feng.tang@intel.com \
    --cc=fweisbec@gmail.com \
    --cc=gcc@gcc.gnu.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=richard.guenther@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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®