All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Frans Pop <elendil@planet.nl>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kbuild@vger.kernel.org, barryn@pobox.com,
	bugme-daemon@bugzilla.kernel.org,
	Ian Lance Taylor <iant@google.com>
Subject: Re: [Bug 13012] 2.6.28.9 causes init to segfault on Debian etch; 2.6.28.8 OK
Date: Sun, 12 Jul 2009 10:58:21 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0907121058040.3552@localhost.localdomain> (raw)
In-Reply-To: <200907101659.31813.elendil@planet.nl>



On Fri, 10 Jul 2009, Frans Pop wrote:
> 
> Prompted by the same suggestion from Ben Hutchings I checked this too, 
> but -fno-strict-overflow was only introduced in gcc 4.2.
> So using it instead of -fwrapv *would* fix the problem for gcc 4.1, but 
> *only* because it would effectively do the same as the patch I proposed: 
> not add an option at all for gcc 4.1.
> 
> So that change seems illogical unless there are other reasons to 
> prefer -fno-strict-overflow over -fwrapv (well, it would avoid the
> gcc version check).
>
> It does however make it somewhat more logical to change the test in my 
> proposed patch to also allow -fwrapv for gcc 4.2.

Hmm. It all really makes me suspect that we should really be using
-fno-strict-overflow instead.

That not only apparently avoids the unnecessary gcc version check (by 
virtue of the option only existing in compilers that don't have the 
problem), but qutie frankly, one of the core people involved with the 
whole thing (Ian Lance Taylor) seems to think it's the better option.

See for example

	http://www.airs.com/blog/archives/120

on how gcc actually generates better code with -fno-strict-overflow.

I added Ian to the cc.

Ian: we generally do try to be careful and use explicit unsigned types for 
code that cares about overflow, but we use -fwrapv because there have been 
some cases where we didn't (and used pointer comparisons or signed 
integers).

The problem is that apparently gcc-4.1.x was literally generating buggy 
code with -fwrapv. So now the choice for us is between switching to an 
explicit version test:

	-KBUILD_CFLAGS  += $(call cc-option,-fwrapv)
	+KBUILD_CFLAGS  += $(shell if [ $(call cc-version) -ge 0402 ]; then \
	+                   echo $(call cc-option,-fwrapv); fi ;)

or just switching to -fno-strict-overflow instead:

	-KBUILD_CFLAGS  += $(call cc-option,-fwrapv)
	+KBUILD_CFLAGS  += $(call cc-option,-fno-strict-overflow)

which avoids the buggy gcc versions because it's simply not even supported 
by gcc-4.1.x (and even if that wasn't the case, possibly because only 
'wrapv' is the problematic case - apparently the difference _does_ 
matter to gcc).

>From everything I have been able to find, I really prefer the second 
version. Not only is the patch cleaner, but it looks like code generation 
is better too (for some inexplicable reason, but I suspect it's because 
-fno-strict-overflow is just saner).

		Linus

WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Frans Pop <elendil@planet.nl>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kbuild@vger.kernel.org, barryn@pobox.com,
	bugme-daemon@bugzilla.kernel.org,
	Ian Lance Taylor <iant@google.com>
Subject: Re: [Bug 13012] 2.6.28.9 causes init to segfault on Debian etch; 2.6.28.8 OK
Date: Sun, 12 Jul 2009 10:58:21 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0907121058040.3552@localhost.localdomain> (raw)
In-Reply-To: <200907101659.31813.elendil@planet.nl>



On Fri, 10 Jul 2009, Frans Pop wrote:
> 
> Prompted by the same suggestion from Ben Hutchings I checked this too, 
> but -fno-strict-overflow was only introduced in gcc 4.2.
> So using it instead of -fwrapv *would* fix the problem for gcc 4.1, but 
> *only* because it would effectively do the same as the patch I proposed: 
> not add an option at all for gcc 4.1.
> 
> So that change seems illogical unless there are other reasons to 
> prefer -fno-strict-overflow over -fwrapv (well, it would avoid the
> gcc version check).
>
> It does however make it somewhat more logical to change the test in my 
> proposed patch to also allow -fwrapv for gcc 4.2.

Hmm. It all really makes me suspect that we should really be using
-fno-strict-overflow instead.

That not only apparently avoids the unnecessary gcc version check (by 
virtue of the option only existing in compilers that don't have the 
problem), but qutie frankly, one of the core people involved with the 
whole thing (Ian Lance Taylor) seems to think it's the better option.

See for example

	http://www.airs.com/blog/archives/120

on how gcc actually generates better code with -fno-strict-overflow.

I added Ian to the cc.

Ian: we generally do try to be careful and use explicit unsigned types for 
code that cares about overflow, but we use -fwrapv because there have been 
some cases where we didn't (and used pointer comparisons or signed 
integers).

The problem is that apparently gcc-4.1.x was literally generating buggy 
code with -fwrapv. So now the choice for us is between switching to an 
explicit version test:

	-KBUILD_CFLAGS  += $(call cc-option,-fwrapv)
	+KBUILD_CFLAGS  += $(shell if [ $(call cc-version) -ge 0402 ]; then \
	+                   echo $(call cc-option,-fwrapv); fi ;)

or just switching to -fno-strict-overflow instead:

	-KBUILD_CFLAGS  += $(call cc-option,-fwrapv)
	+KBUILD_CFLAGS  += $(call cc-option,-fno-strict-overflow)

which avoids the buggy gcc versions because it's simply not even supported 
by gcc-4.1.x (and even if that wasn't the case, possibly because only 
'wrapv' is the problematic case - apparently the difference _does_ 
matter to gcc).

From everything I have been able to find, I really prefer the second 
version. Not only is the patch cleaner, but it looks like code generation 
is better too (for some inexplicable reason, but I suspect it's because 
-fno-strict-overflow is just saner).

		Linus

  reply	other threads:[~2009-07-12 17:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-10  7:28 [Bug 13012] 2.6.28.9 causes init to segfault on Debian etch; 2.6.28.8 OK Frans Pop
2009-07-10 14:59 ` Frans Pop
2009-07-12 17:58   ` Linus Torvalds [this message]
2009-07-12 17:58     ` Linus Torvalds
2009-07-12 18:24     ` Linus Torvalds
2009-07-13  5:29     ` Ian Lance Taylor
2009-07-25  3:23       ` Dave Jones
2009-07-25 16:49         ` Linus Torvalds
2009-07-10 20:05 ` [PATCH,v2] Only add '-fwrapv' to gcc CFLAGS for gcc 4.2 and later Frans Pop
2009-07-17 22:18   ` Sam Ravnborg
2009-07-17 22:43     ` Frans Pop
2009-07-18  6:59       ` Sam Ravnborg
2009-07-23 12:46         ` Frans Pop
2009-07-23 14:27           ` Sam Ravnborg

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.01.0907121058040.3552@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=barryn@pobox.com \
    --cc=bugme-daemon@bugzilla.kernel.org \
    --cc=elendil@planet.nl \
    --cc=iant@google.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.