public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Alan Modra <amodra@gmail.com>
Cc: Jan Beulich <JBeulich@novell.com>,
	"H.J. Lu" <hjl.tools@gmail.com>,
	binutils@sourceware.org, linux-kernel@vger.kernel.org,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: PATCH: Add --size-check=[error|warning]
Date: Mon, 14 Mar 2011 12:26:00 -0000	[thread overview]
Message-ID: <20110314122539.GA27205@elte.hu> (raw)
In-Reply-To: <20110314122342.GA26825@elte.hu>


(resend, fixed the To line)

* Alan Modra <amodra@gmail.com> wrote:
 
> On Mon, Mar 14, 2011 at 10:55:34AM +0100, Ingo Molnar wrote:
> > The thing is, it is absolutely, breath-takingy incompetent for
> 
> kernel developers to write such poor asm!  And not notice the error
> for 4 years! [...]

It is not 'poor asm'.

The 'bug' is just a slight assymetry in ENTRY()/END() debug-symbols sequences, with 
lots of assembly code between the ENTRY() and the END(). Here's an example:
    
         ENTRY(xen_do_hypervisor_callback)   # do_hypervisor_callback(struct *pt_regs)
           ...
         END(do_hypervisor_callback)

Human reviewers almost never catch such small mismatches, and binutils never even 
warned about it either - for over a decade.

Now kernel bisections are insta-broken on latest binutils, and there's nothing to do 
about it on the kernel side as during bisection all later fixes are unfolded. The 
fix itself i already applied - but my argument was not about that:

> [...] Oh, and the binutils developers to write such a poor assembler in the first 
> place.  ;-)
> 
> Seriously, you are complaining because something is fixed??
 
No, i reported this bug because the kernel build gets broken going back 130,000 
commits, breaking bisection and causing other damage - while issuing a warning 
message would achieve the same effect of warning the developer about the mismatch.

> > The correct solution is to turn it into a warning as me and others have suggested.
> 
> I disagree.  The whole world is not the linux kernel.  I think HJ is
> bending over backwards to even offer a switch that turns the error
> into a warning.
 
It's not about a switch at all - it's to not break builds by default. I.e. the 
default behavior should be to issue a warning and ignore the directive.
 
This is a very simple concept of compatibility: the build environment should always 
be very permissive - stuff that build fine before should be allowed to build.

Also, i hope you are not suggesting to break projects just because they are not 
important to you personally? The fix is exceedingly simple to do for the binutils 
project - and impossible to do for the kernel project (because during bisection - 
which is a very powerful debugging tool - older versions of the source get checked 
out).

Thanks,

	Ingo

  reply	other threads:[~2011-03-14 12:26 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-11 16:58 H.J. Lu
2011-03-11 17:04 ` Jan Beulich
2011-03-11 17:20   ` H.J. Lu
2011-03-14  8:44     ` Jan Beulich
2011-03-14  9:55       ` Ingo Molnar
2011-03-14 10:41         ` Alan Modra
2011-03-14 10:50           ` Pekka Enberg
2011-03-14 18:04             ` Alan Cox
2011-03-14 10:52           ` Jan Beulich
     [not found]           ` <AANLkTi=3AiLaw5Gnis8Ha4eRXirk0s-Cnk2zzN12YDpH__45869.1711457961$1300099861$gmane$org@mail.gmail.com>
2011-03-14 11:03             ` Andreas Schwab
2011-03-14 11:12               ` Pekka Enberg
2011-03-14 12:02                 ` Andreas Schwab
2011-03-14 12:13                   ` Ingo Molnar
2011-03-14 12:55                     ` Andreas Schwab
2011-03-14 13:17                       ` Ingo Molnar
2011-03-14 14:11                         ` Andreas Schwab
2011-03-14 12:11                 ` Ingo Molnar
2011-03-14 12:23           ` Ingo Molnar
2011-03-14 12:26             ` Ingo Molnar [this message]
2011-03-14 13:16             ` git bisect plus fixes (was: PATCH: Add --size-check=[error|warning]) Ralf Wildenhues
2011-03-14 13:55       ` PATCH: Add --size-check=[error|warning] H.J. Lu
2011-03-16 23:56 ` Alan Modra
2011-03-18 11:20   ` Alan Modra
2011-03-18 11:59     ` Alan Modra
2011-03-14 11:02 Sedat Dilek
2011-03-14 11:25 ` Jan Beulich
2011-03-14 11:35   ` Ingo Molnar
2011-03-14 11:39   ` Sedat Dilek
2011-03-14 11:53     ` Sedat Dilek
2011-03-14 12:22       ` Jan Beulich
2011-03-14 12:38         ` Sedat Dilek
2011-03-14 15:56         ` Ian Lance Taylor
2011-03-14 18:02           ` H. Peter Anvin
2011-03-14 16:33   ` H. Peter Anvin
2011-03-14 19:24     ` Steven Rostedt
2011-03-14 20:06       ` Linus Torvalds
2011-03-14 12:57 ` Ingo Molnar

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=20110314122539.GA27205@elte.hu \
    --to=mingo@elte.hu \
    --cc=JBeulich@novell.com \
    --cc=akpm@linux-foundation.org \
    --cc=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=hjl.tools@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).