Index
Home
About
Blog
Date: Thu, 2 Nov 2000 17:46:20 -0500
From: "Theodore Y. Ts'o" <tytso@MIT.EDU>
Subject: Re: non-gcc linux? (was Re: Where did kgcc go in 2.4.0-test10?)
Newsgroups: fa.linux.kernel
Date: Thu, 02 Nov 2000 13:53:55 -0700
From: Tim Riker <Tim@Rikers.org>
As is being discussed here, C99 has some replacements to the gcc syntax
the kernel uses. I believe the C99 syntax will win in the near future,
and thus the gcc syntax will have to be removed at some point. In the
interim the kernel will either move towards supporting both, or a
quantum jump to support the new gcc3+ compiler only. I am hoping a
little thought can get put into this such that this change will be less
painful down the road.
That's reasonable as a long-term goal. Keep in mind that though there
have been questions in the past about code correctness assumptions of
kernel versus specific GCC versions. This has been one place where GCC
has tended to blame the kernel developers, and kernel developers have
pointed out (rightly, in my opinion) that the GCC documentation of some
of these features has been less than stellar --- in fact, some would say
non-existent. If it's not documented, then you don't have much moral
ground to stand upon when people complain that the changes you made
breaks things.
So moving to a C99 syntax is useful simply from the point of view that
it's well documented (unlike the register constraints for inline
functions, which still give me a headache whenever I try to look at the
GCC "documentation"). The problem here is that C99 doesn't (as far as I
know) give us everything we need, so simply moving to C99 syntax won't
be sufficient to support proprietary C compilers.
There will also be work needed to make sure that a kernel compiled with
gcc 3.x (whenever it's ready) will actually omit code which was intended
by the kernel developers. So we're definitely looking at a 2.5+
project, and one which may actually be fairly high risk; it's certainly
not a trivial task.
- Ted
Newsgroups: fa.linux.kernel
From: torvalds@transmeta.com (Linus Torvalds)
Subject: Re: gcc 2.95 vs 3.21 performance
Original-Message-ID: <b1pbt8$2ll$1@penguin.transmeta.com>
Date: Tue, 4 Feb 2003 21:42:40 GMT
Message-ID: <fa.k5ue3hl.3ji2i9@ifi.uio.no>
In article <200302041935.h14JZ69G002675@darkstar.example.net>,
John Bradford <john@grabjohn.com> wrote:
>> I'm hesitant to enter into this. But from my own experience
>> the issue with big companies supporting these sort of changes
>> in gcc have more to do with the acceptance process of changes
>> into gcc than a lack of desire on the large companies part.
>
>Maybe we should create a KGCC fork, optimise it for kernel
>complilations, then try to get our changes merged back in to GCC
>mainline at a later date.
That's not really the problem.
I think the problem with gcc is that many of the developers are actually
much more interested in Ada or C++ (or even Fortran!), than in plain
old-fashioned C. So it's not a kernel issue per se, gcc is slow to
compile _any_ C project.
And a lot of the optimizations gcc does aren't even interesting to most
C projects. Most "old-fashioned" C projects tend to be written in ways
that mean that the most important optimizations are the truly trivial
ones, and then doing good register allocation.
I'd love to see a small - and fast - C compiler, and I'd be willing to
make kernel changes to make it work with it.
Let's see. There's been some noises on the gcc lists about splitting up
the languages for easier maintenance, we'll see what happens.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: gcc 2.95 vs 3.21 performance
Original-Message-ID: <Pine.LNX.4.44.0302041405500.2638-100000@penguin.transmeta.com>
Date: Tue, 4 Feb 2003 22:15:06 GMT
Message-ID: <fa.oc9ncn8.hk85b0@ifi.uio.no>
On Tue, 4 Feb 2003, John Bradford wrote:
> > I'd love to see a small - and fast - C compiler, and I'd be willing to
> > make kernel changes to make it work with it.
>
> How IA-32 centric would your prefered compiler choice be? In other
> words, if a small and fast C compiler turns up, which lacks support
> for some currently ported to architectures, are you likely to
> encourage kernel changes which will make it difficult for the other
> architectures that have to stay with GCC to keep up?
I don't think being architecture-specific is necessarily a bad thing in
compilers, although most compiler writers obviously try to avoid it.
The kernel shouldn't really care: it does want to have a compiler with
support for inline functions, but other than that it's fairly close to
ANSI C.
Yes, I know we use a _lot_ of gcc extensions (inline asms, variadic macros
etc), but that's at least partly because there simply aren't any really
viable alternatives to gcc, so we've had no incentives to abstract any of
that out.
So the gcc'isms aren't really fundamental per se. Although, quite frankly,
even inline asms are pretty much a "standard" thing for any reasonable C
compiler (since C is often used for things that really want it), and the
main issue tends to be the exact syntax rather than anything else. So I
don't think I'd like to use a compiler that is _so_ limited that it
doesn't have some support for something like that. I certainly would
refuse to use a C compiler that didn't support inline functions.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: gcc 2.95 vs 3.21 performance
Original-Message-ID: <Pine.LNX.4.44.0302041412210.2638-100000@penguin.transmeta.com>
Date: Tue, 4 Feb 2003 22:17:55 GMT
Message-ID: <fa.oappcv5.i4a537@ifi.uio.no>
On 4 Feb 2003, Andi Kleen wrote:
>
> If you want small and fast use lcc.
lcc isn't really something I want to use, since the license is so strange,
and thus can't be improved upon if there are issues with it.
Some people have used the Intel compiler - which obviously also cannot be
improved upon, but which is likely to start off pretty good. I don't
really want to use it myself - what I'd really like to see is gcc
splitting up just the C compiler as a separate project with more attention
to size and speed.
Linus
Newsgroups: fa.linux.kernel
From: torvalds@transmeta.com (Linus Torvalds)
Subject: Re: gcc 2.95 vs 3.21 performance
Original-Message-ID: <b1rni4$3dr$1@penguin.transmeta.com>
Date: Wed, 5 Feb 2003 19:13:21 GMT
Message-ID: <fa.jsei422.92m224@ifi.uio.no>
In article <3E4045D1.4010704@rogers.com>,
Jeff Muizelaar <muizelaar@rogers.com> wrote:
>
>There is also tcc (http://fabrice.bellard.free.fr/tcc/)
>It claims to support gcc-like inline assembler, appears to be much
>smaller and faster than gcc. Plus it is GPL so the liscense isn't a
>problem either.
>Though, I am not really sure of the quality of code generated or of how
>mature it is.
tcc is interesting. The code generation is pretty simplistic (read:
trivially horrible for most things), but it sure is fast and small. And
judging by the changelog, Fabrice is trying to compile the kernel with
it.
For a lot of problems, small-and-fast is good. Hell, some of the things
I'd personally find interesting don't have any code generation part at
all (static analysis of annotated source-code - stanford checker on the
cheap). And development doesn't always need good code generation (right
now some people use "gcc -O0" for that, because anything else hurts too
much. Now, the code from tcc will probably look more like "-O-1", but
at least you can test out things _quickly_).
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: gcc 2.95 vs 3.21 performance
Original-Message-ID: <Pine.LNX.4.44.0302051157580.2999-100000@home.transmeta.com>
Date: Wed, 5 Feb 2003 20:16:08 GMT
Message-ID: <fa.m8eifj0.16gka8i@ifi.uio.no>
On Wed, 5 Feb 2003, Pavel [iso-8859-2] Janík wrote:
>
> Hi Linus,
>
> > lcc isn't really something I want to use, since the license is so
> > strange, and thus can't be improved upon if there are issues with it.
>
> what is the difference between compiler and source management system
> regarding licenses and improvements?
You snipped the part where I said that the intel compiler is likely to be
more interesting to a number of people, since it's at a higher level. So
no, I'm not religious about licenses.
But the real issue is "does it do what we want it to do?" and "do we have
a choice?". There are no open-source SCM's that work for me. But there
_is_ an open-source compiler that does work for me. At which point the
license matters - simply because there is choice in the matter.
Gcc mostly works. But it's slower then I'd like. And it prioritizes things
I don't care about. And competition is always good. So I would definitely
love to see some alternatives.
And if you have issues with BK, maybe you can try to encourage the SCM
people to see why I consider BK to not even have alternatives right now..
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302241259320.13406-100000@penguin.transmeta.com>
Date: Mon, 24 Feb 2003 21:09:00 GMT
Message-ID: <fa.nb517dv.e7uhg1@ifi.uio.no>
On Mon, 24 Feb 2003, Richard B. Johnson wrote:
>
> I think you must keep these warnings in! There are many bugs
> that these uncover uncluding loops that don't terminate correctly
> but seem to work for "most all" cases. These are the hard-to-find
> bugs that hit you six months after release.
At least historically gcc has been so f*cking bad at the "unsigned vs
signed" warnings that they are totally useless.
Maybe things are better in gcc-3.3.
Maybe not.
> size_t i;
>
> while((i = do_forever()) > 0)
> ;
>
> ... do_forever() finally errors out and returns -1 stuck(forever).
Does gcc still warn about things like
#define COUNT (sizeof(array)/sizeof(element))
int i;
for (i = 0; i < COUNT; i++)
...
where COUNT is obviously unsigned (because sizeof is size_t and thus
unsigned)?
Gcc used to complain about things like that, which is a FUCKING DISASTER.
Any compiler that complains about the above should be shot in the head,
and the warning should be killed.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302241429150.13406-100000@penguin.transmeta.com>
Date: Mon, 24 Feb 2003 22:45:34 GMT
Message-ID: <fa.na5b6lv.f7sg81@ifi.uio.no>
On Mon, 24 Feb 2003, Andreas Schwab wrote:
>
> But that's not the point. It's the runtime value of i that gets converted
> (to unsigned), not the compile time value of COUNT. Thus if i ever gets
> negative you have a problem.
The point is that the compiler should see that the run-time value of i is
_obviously_never_negative_ and as such the warning is total and utter
crap.
The compiler actually does end up doing some of that kind of analysis when
optimizing, since it is required for some of the loop optimizations
anyway. But the warning is emitted before the analysis has taken place.
In other words, gcc is stupidly warning about something that is obviously
not an error. And it is obviously not an error because:
- array indexes are "int". They aren't size_t, and never have been. Thus
it is _correct_ to use "int" for the index. You write
int array[5];
and you do NOT write
int array[5UL];
and anybody who writes 'array[5UL]' is considered a stupid git and a
geek. Face it.
Claiming that you should index an array with a size_t is crap.
- the way this has traditionally always been done is the example I
posted. Warning about correct, obvious, and traditional code is WRONG,
if it causes the programmer to have to write something _less_ obvious
just to make a stupid compiler warning go away makes the warning WORSE
THAN USELESS.
And yes, gcc could do the work necessary to only give the warning if it
actually has reason to believe that the code is wrong. As it is, it gives
the warning for code that is good.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302241519520.19762-100000@penguin.transmeta.com>
Date: Mon, 24 Feb 2003 23:26:44 GMT
Message-ID: <fa.na5j765.f74g87@ifi.uio.no>
On 25 Feb 2003, Alan Cox wrote:
> On Mon, 2003-02-24 at 22:39, Linus Torvalds wrote:
> > And yes, gcc could do the work necessary to only give the warning if it
> > actually has reason to believe that the code is wrong. As it is, it gives
> > the warning for code that is good.
>
> gcc gives the warning only when you ask it to annoy you. Seems a good trade off.
That _used_ to be true.
Look at the subject line. gcc-3.3 gives the warning for -Wall.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302241549140.1282-100000@penguin.transmeta.com>
Date: Mon, 24 Feb 2003 23:57:49 GMT
Message-ID: <fa.o9ppff6.i4a636@ifi.uio.no>
On 25 Feb 2003, Alan Cox wrote:
>
> gcc-3.3 doesnt exist yet. Maybe it wont do that now 8)
Right now there are some other problems with gcc-3.3 too, ie the inlining
is apparently broken enough that we'll either have to start using
__attribute__((force_inline)) or we'd better hope that the gcc people
decide to take the "inline" keyword more seriously (it's being discussed
on the gcc lists, so we'll see)
But yes, these are all obviously with "early versions", and it may be that
it changes before the real release.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302241426480.13406-100000@penguin.transmeta.com>
Date: Mon, 24 Feb 2003 22:35:09 GMT
Message-ID: <fa.nblh6ls.dn6g8e@ifi.uio.no>
On Mon, 24 Feb 2003, Andreas Schwab wrote:
> |>
> |> Gcc used to complain about things like that, which is a FUCKING DISASTER.
>
> How can you distinguish that from other occurrences of (int)<(size_t)?
Which is indeed my point. If you cannot distinguish it from incorrect
uses, you shouldn't be warning the user, because the compiler obviously
doesn't know enough to make a sufficiently educated guess.
That said, a good compiler _can_ make a good warning. But to do so, you
have to actually do value analysis, instead of just blindly warning about
code that is obviously correct to a human.
Until gcc does sufficient value analysis, that signed warning is annoying,
worthless and a damn pain in the ass.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302250712110.10210-100000@home.transmeta.com>
Date: Tue, 25 Feb 2003 15:31:39 GMT
Message-ID: <fa.l0cjn9r.1kh821p@ifi.uio.no>
On Tue, 25 Feb 2003, Andreas Schwab wrote:
> |>
> |> The point is that the compiler should see that the run-time value of i is
> |> _obviously_never_negative_ and as such the warning is total and utter
> |> crap.
>
> This requires a complete analysis of the loop body, which means that the
> warning must be moved down from the front end (the common type of the
> operands only depends on the type of the operands, not of any current
> value of the expressions).
So? Gcc does that anyway. _Any_ good compiler has to.
And if the compiler isn't good enough to do it, then the compiler
shouldn't be warning about something that it hasn't got a clue about.
> |> and anybody who writes 'array[5UL]' is considered a stupid git and a
> |> geek. Face it.
>
> But array[-1] is wrong. An array can never have a negative index (I'm
> *not* talking about pointers).
You're wrong.
Yes, when declaring an array, you cannot use "array[-1]". But that's not
because the thing is unsigned: the standard says that the array
declaration has to be a "integer value larger than zero". It is not
unsigned: it's _positive_.
However, in _indexing_ an array (as opposed to declaring it), "array[-1]"
is indeed perfectly fine, and is defined by the C language to be exactly
the same as "*(array-1)". And negative values are perfectly fine, even for
arrays. Trivial example:
int x[2][2];
int main(int argc, char **argv)
{
return x[1][-1];
}
the above is actually a well-defined C program, and 100%
standards-conforming ("strictly conforming").
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302250742400.10210-100000@home.transmeta.com>
Date: Tue, 25 Feb 2003 16:07:35 GMT
Message-ID: <fa.l1sho1r.1m1a39p@ifi.uio.no>
On Tue, 25 Feb 2003, Andreas Schwab wrote:
> |>
> |> So? Gcc does that anyway. _Any_ good compiler has to.
>
> But the point is that determining the common type does not require _any_
> kind of data flow analysis, and this is the place where the unsigned
> warning is generated.
Go back and read my mail. In fact, go back and read just about _any_ of my
mails on the subject. Go back and read the part you snipped:
"And if the compiler isn't good enough to do it, then the compiler
shouldn't be warning about something that it hasn't got a clue about."
In other words, either gcc should do it right, or it should not be done at
all. Right now the warning IS GENERATED IN THE WRONG PLACE. Your argument
seems to be "but that's the place it is generated" is a total
non-argument. It's just stating a fact, and it's exactly that fact that
causes the BUG IN GCC.
> |> Trivial example:
> |>
> |> int x[2][2];
> |>
> |> int main(int argc, char **argv)
> |> {
> |> return x[1][-1];
> |> }
> |>
> |>
> |> the above is actually a well-defined C program, and 100%
> |> standards-conforming ("strictly conforming").
>
> This isn't as trivial as it seems. Look in comp.std.c for recent
> discussions on this topic (out-of-array references).
It is as trivial as it seems, and this is _not_ an out-of-array reference.
Sorry. C very clearly specifies the layout of arrays (6.5.2.1: last
subscript varies fastest, together with sizeof(array) = sizeof(entry)*n),
which _guarantees_ that the pointers involved in the above calculations
are all within the object, and thus there is _zero_ undefined behaviour.
So if they argued about this on comp.std.c, they either had clueless
people there (in addition to the ones that obviously aren't ;), _or_ you
misunderstood the argument.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH] s390 (7/13): gcc 3.3 adaptions.
Original-Message-ID: <Pine.LNX.4.44.0302250811220.10500-100000@home.transmeta.com>
Date: Tue, 25 Feb 2003 16:20:41 GMT
Message-ID: <fa.l0sno1p.1l1g3pr@ifi.uio.no>
On Tue, 25 Feb 2003, Linus Torvalds wrote:
>
> return x[1][-1];
Btw, don't get me wrong. I don't think the above is really code that
should survive, and if the compiler were to generate a warning for
something like that, where the subscripts are clearly out of the range
that they were in the declaration, then I'd be entirely supportive of
that.
I don't know how we got side-tracked to negative subscripts. They are
clearly legal with pointers, but that wasn't even the issue: the code that
generated the "signed/unsigned" warning didn't use any negative
subscripts, never had, and never will.
And unlike the abomination above, the code that generates the warning is
the _clearest_ version of code you can humanly write. And THAT is the
problem with the warning: there's no way to avoid the warning without
making the source code _worse_ in some way.
And _that_ is what my argument really boils down to. Nothing else.
Linus
Newsgroups: fa.linux.kernel
From: torvalds@transmeta.com (Linus Torvalds)
Subject: Re: Invalid compilation without -fno-strict-aliasing
Original-Message-ID: <b3itcd$2bi$1@penguin.transmeta.com>
Date: Wed, 26 Feb 2003 17:32:36 GMT
Message-ID: <fa.joh41q7.f644qv@ifi.uio.no>
In article <20030225234646.GB30611@bougret.hpl.hp.com>,
Jean Tourrilhes <jt@bougret.hpl.hp.com> wrote:
>
> It looks like a compiler bug to me...
Why do you think the kernel uses "-fno-strict-aliasing"?
The gcc people are more interested in trying to find out what can be
allowed by the c99 specs than about making things actually _work_. The
aliasing code in particular is not even worth enabling, it's just not
possible to sanely tell gcc when some things can alias.
> Some users have complained that when the following code is
>compiled without the -fno-strict-aliasing, the order of the write and
>memcpy is inverted (which mean a bogus len is mem-copied into the
>stream).
The "problem" is that we inline the memcpy(), at which point gcc won't
care about the fact that it can alias, so they'll just re-order
everything and claim it's out own fault. Even though there is no sane
way for us to even tell gcc about it.
I tried to get a sane way a few years ago, and the gcc developers really
didn't care about the real world in this area. I'd be surprised if that
had changed, judging by the replies I have already seen.
I'm not going to bother to fight it.
Linus
Newsgroups: fa.linux.kernel
From: torvalds@transmeta.com (Linus Torvalds)
Subject: Re: Invalid compilation without -fno-strict-aliasing
Original-Message-ID: <b3lovr$16j$1@penguin.transmeta.com>
Date: Thu, 27 Feb 2003 19:35:57 GMT
Message-ID: <fa.jc222i1.1fne5i7@ifi.uio.no>
In article <20030226172213.O3910@devserv.devel.redhat.com>,
Jakub Jelinek <jakub@redhat.com> wrote:
>
>To fix that, __constant_memcpy would have to access the data through
>union,
Which is impossible, since memcpy _fundamentally_ cannot know what the
different types are..
> or you could as well forget about __constant_memcpy and use
>__builtin_memcpy where gcc will take care about the constant copying.
Which is impossible because (a) historically __builtin_memcpy does a bad
job and (b) it doesn't solve the generic case anyway, ie for other
non-memcpy things.
The fact is, for type-based alias analysis gcc needs a way to tell it
"this can alias", which it doesn't have. Unions are _not_ useful,
_regardless_ of what silly language lawyers say, since they are not a
generic method. Unions only work for trivial and largely uninteresting
cases, and it doesn't _matter_ what C99 says about the issue, since that
nasty thing called "real life" interferes.
Until we get some non-union way to say "this can alias", that
-fno-strict-alias has to stay because gcc is too broken to allow us
doing interesting stuff in-line without it.
My personal opinion is (and was several years ago when this started
coming up) that a cast (any cast) should do it. But I don't are _what_
it is, as long as it is syntactically sane and isn't limited to special
cases like unions.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: Invalid compilation without -fno-strict-aliasing
Original-Message-ID: <Pine.LNX.4.44.0302271157380.9696-100000@home.transmeta.com>
Date: Thu, 27 Feb 2003 20:04:05 GMT
Message-ID: <fa.maucg2v.100aaov@ifi.uio.no>
On Thu, 27 Feb 2003, Daniel Jacobowitz wrote:
> >
> > My personal opinion is (and was several years ago when this started
> > coming up) that a cast (any cast) should do it. But I don't are _what_
> > it is, as long as it is syntactically sane and isn't limited to special
> > cases like unions.
>
> Well, if that's all you're asking for, it's easy - I don't know if
> you'll agree that the syntax is sane, but it's there. From the GCC 3.3
> manual:
>
> `may_alias'
Halleluja. It still leaves us with all other compilers unaccounted for,
but yes, this will allow fixing the problem for us in the future.
Too bad the current gcc-3.3 is apparently not useful for the kernel for
other reasons right now (ie the inlining issue which is being discussed on
the gcc lists, and that annoying sign warning), but that may well be
resolved by the time it gets released.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: Invalid compilation without -fno-strict-aliasing
Original-Message-ID: <Pine.LNX.4.44.0302271234530.9696-100000@home.transmeta.com>
Date: Thu, 27 Feb 2003 20:42:51 GMT
Message-ID: <fa.mbu4fis.130qb8s@ifi.uio.no>
On Thu, 27 Feb 2003, Daniel Jacobowitz wrote:
>
> We could work around both of these: disable the sign compare warning,
> and use check_gcc to set a high number for -finline-limit...
Oh, both are work-aroundable, no question about it. The same way it was
possible to work around the broken aliasing with previous releases. I'm
just hoping that especially the inline thing can be resolved sanely,
otherwise we'll end up having to use something ugly like
-D'inline=inline __attribute__((force_inline))'
on every single command line..
(I find -finline-limit tasteless, since the limit number is apparently
totally meaningless as far as the user is concerned. It's clearly a
command line option that is totally designed for ad-hoc compiler tweaking,
not for any actual useful user stuff).
Linus
Newsgroups: fa.linux.kernel
From: "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: Invalid compilation without -fno-strict-aliasing
Original-Message-ID: <b3m8hv$5fh$1@cesium.transmeta.com>
Date: Thu, 27 Feb 2003 23:56:46 GMT
Message-ID: <fa.k3727pf.e4qj8l@ifi.uio.no>
Followup to: <Pine.LNX.4.44.0302271234530.9696-100000@home.transmeta.com>
By author: Linus Torvalds <torvalds@transmeta.com>
In newsgroup: linux.dev.kernel
>
> On Thu, 27 Feb 2003, Daniel Jacobowitz wrote:
> >
> > We could work around both of these: disable the sign compare warning,
> > and use check_gcc to set a high number for -finline-limit...
>
> Oh, both are work-aroundable, no question about it. The same way it was
> possible to work around the broken aliasing with previous releases. I'm
> just hoping that especially the inline thing can be resolved sanely,
> otherwise we'll end up having to use something ugly like
>
> -D'inline=inline __attribute__((force_inline))'
>
> on every single command line..
>
Isn't this what compiler.h is for? If the complaint is that some
things don't include compiler.h then we may want to force it with
-include. Better all the cruft in one file IMO.
-hpa
--
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: cris ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: Runaway cron task on 2.5.63/4 bk?
Original-Message-ID: <Pine.LNX.4.44.0303111458390.1709-100000@home.transmeta.com>
Date: Tue, 11 Mar 2003 23:06:25 GMT
Message-ID: <fa.m6uodit.140288p@ifi.uio.no>
On Tue, 11 Mar 2003, Andrew Morton wrote:
>
> gcc will generate 64bit * 64bit multiplies without resorting to
> any library code
However, gcc is unable to do-the-right-thing and generate 32x32->64
multiplies, or 32x64->64 multiplies, even though those are both a _lot_
faster than the full 64x64->64 case.
And in quite a _lot_ of cases, that's actually what you want. It might
actually make sense to add a "do_mul()" thing to allow architectures to do
these cases right, since gcc doesn't.
> and you can probably do the division with do_div().
Yes. This is the same issue - gcc will always promote a 64-bit divide to
be _fully_ 64-bit, even if the mixed-size 64/32 -> [64,32] case is much
faster and simpler. Which is why do_div() exists in the first place.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: Runaway cron task on 2.5.63/4 bk?
Original-Message-ID: <Pine.LNX.4.44.0303111517020.1709-100000@home.transmeta.com>
Date: Tue, 11 Mar 2003 23:37:21 GMT
Message-ID: <fa.m5eccis.15gm98m@ifi.uio.no>
On Tue, 11 Mar 2003, Andrew Morton wrote:
>
> 2.95.3 and 3.2.1 seem to do it right?
Try the "64x32->64" version. gcc didn't use to get that one right, but
maybe it does now.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: Runaway cron task on 2.5.63/4 bk?
Original-Message-ID: <Pine.LNX.4.44.0303111544050.2411-100000@home.transmeta.com>
Date: Tue, 11 Mar 2003 23:51:07 GMT
Message-ID: <fa.m5ucdih.140u88t@ifi.uio.no>
On Tue, 11 Mar 2003, Andrew Morton wrote:
>
> 2.95.3 and 3.2.1 seem to do it right?
Well, they do, but your test case is wrong:
> long a;
> long b;
> long long c;
>
> void foo(void)
> {
> c = a * b;
This is just a 32*32->32 multiply, with a final sign extension to 64 bits.
You need to do
c = (long long) a * b;
to get a 32*32->64 multiply. And yes, gcc gets that case right.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [BK+PATCH] remove __constant_memcpy
Original-Message-ID: <Pine.LNX.4.44.0304161904170.1534-100000@home.transmeta.com>
Date: Thu, 17 Apr 2003 02:07:05 GMT
Message-ID: <fa.m5usd2p.150e9ov@ifi.uio.no>
On Wed, 16 Apr 2003, Jeff Garzik wrote:
>
> gcc's __builtin_memcpy performs the same function (and more) as the
> kernel's __constant_memcpy. So, let's remove __constant_memcpy, and let
> the compiler do it.
Please don't.
There's no way gcc will EVER get the SSE2 cases right. It just cannot do
it. In fact, I live in fear that we will have to turn off the compiler
intrisics entirely some day just because there is always the worry that
gcc will start using FP.
So the advantage of doing our own memcpy() is not that it's necessarily
faster than the gcc built-in, but simply because I do not believe that the
gcc people care enough about the kernel to let them make the choice.
Linus
Newsgroups: fa.linux.kernel
From: torvalds@transmeta.com (Linus Torvalds)
Subject: Re: [RFC][PATCH] Faster generic_fls
Original-Message-ID: <b8q8gq$4o3$1@penguin.transmeta.com>
Date: Thu, 1 May 2003 04:42:01 GMT
Message-ID: <fa.k0us3od.bic2ol@ifi.uio.no>
In article <p73ade730d1.fsf@oldwotan.suse.de>, Andi Kleen <ak@suse.de> wrote:
>Linus Torvalds <torvalds@transmeta.com> writes:
>
>> Yeah, except if you want best code generation you should probably use
>>
>> static inline int fls(int x)
>> {
>> int bit;
>> /* Only well-defined for non-zero */
>> asm("bsrl %1,%0":"=r" (bit):"rm" (x));
>
>"g" should work for the second operand too and it'll give gcc
>slightly more choices with possibly better code.
"g" allows immediates, which is _not_ correct.
>But the __builtin is probably preferable if gcc supports it because
>a builtin can be scheduled, inline assembly can't.
You're over-estimating gcc builtins, and underestimating inline
assembly.
gcc builtins usually generate _worse_ code than well-written inline
assembly, since builtins try to be generic (at least the ones that are
cross-architecture).
And inline asms schedule as well as any built-in, unless they are marked
volatile (either explicitly or implicitly by not having any outputs).
And the proof is in the pudding: I'll bet you a dollar my code generates
better code. AND my code works on the intel compiler _and_ with older
gcc's.
The fact is, gcc builtins are almost never worth it.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [RFC][PATCH] Faster generic_fls
Original-Message-ID: <Pine.LNX.4.44.0305012212190.5230-100000@home.transmeta.com>
Date: Fri, 2 May 2003 05:14:09 GMT
Message-ID: <fa.m8eed2e.16g89oq@ifi.uio.no>
On 1 May 2003, David S. Miller wrote:
>
> This actually is false. GCC does not know what resources the
> instruction uses, therefore it cannot perform accurate instruction
> scheduling.
Yeah, and sadly the fact that gcc-3.2.x does better instruction scheduling
has shown itself to not actually _matter_ that much. I'm quite impressed
by the new scheduler, but gcc-2.x seems to still generate faster code on
too many examples.
CPU's are getting smarter, not dumber. And that implies, for example, that
instruction scheduling matters less and less. What matters on the P4, for
example, seems to be the _choice_ of instructions, not the scheduling of
them.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: [PATCH 2/7] dm: signed/unsigned audit
Original-Message-ID: <Pine.LNX.4.44.0306090830360.12683-100000@home.transmeta.com>
Date: Mon, 9 Jun 2003 15:36:37 GMT
Message-ID: <fa.l2tboa4.1k1k31u@ifi.uio.no>
On Mon, 9 Jun 2003, Kevin Corry wrote:
>
> On Monday 09 June 2003 09:35, Joe Thornber wrote:
> > signed/unsigned audit.
>
> > - int minor, r = -EBUSY;
> > + unsigned int m, r = -EBUSY;
>
> Looks like "r" should still be signed.
Yes.
Guys, be _very_ careful if you do audits based on the -Wsigned flag to
gcc, because THE WARNING OUTPUT OF GCC IS OFTEN TOTAL CRAP.
I consider the -Wsigned flag to be more harmful than not, since a
noticeable percentage of the warnings are for code that is perfectly ok,
and rewriting the code to avoid the warning can lead to very subtle bugs
(or just makes the code less readable).
Some of the false warnings could probably be fixed with some fairly
trivial value range analysis, and I'm hopeful that -Wsign can some day
actually be worthwhile, but for now I really wish people be very very
careful.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@transmeta.com>
Subject: Re: GCC speed (was [PATCH] Isapnp warning)
Original-Message-ID: <Pine.LNX.4.44.0306221049280.15159-100000@home.transmeta.com>
Date: Sun, 22 Jun 2003 17:59:03 GMT
Message-ID: <fa.l5t5nq3.1h1m3hh@ifi.uio.no>
On Sun, 22 Jun 2003, Andrew Morton wrote:
>
> I compile with -O1 all the time and couldn't care the teeniest little bit
> about the performance of the generated code - it just doesn't matter.
Well, sometimes it _does_ matter from a correctness standpoint. For
example, gcc without optimizations was simply unusable, because of the
lack of inlining or the lack of even trivial optimizations (ie static dead
code removal).
And a (unrelated) gcc list thread showed that even with just -O1, one
particular project had gone from 131 seconds (gcc-2.7.2) to 180 seconds
(2.95.3) to 282 seconds (3.3) to 327 seconds (the tree-ssa branch).
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH] irda: fix type of struct irda_ias_set.attribute.irda_attrib
_string.len
Original-Message-ID: <Pine.LNX.4.44.0311101900120.2881-100000@home.osdl.org>
Date: Tue, 11 Nov 2003 03:09:08 GMT
Message-ID: <fa.jht80hd.1ila297@ifi.uio.no>
On Mon, 10 Nov 2003, Linus Torvalds wrote:
>
> No, please don't.
Btw, this is a general thing with warnings that the compiler spits out.
Some compiler warnings are for perfectly ok code.
Sometimes the warning itself is fundamentally broken (the sign warnings
that gcc used to spit out), sometimes it's because the code is 100% right
for some abstract reason that the compiler can't figure out.
An example of such an "abstract reason" is exactly something like this
case, where the user really didn't _care_ too deeply about the type he was
checking, and was caring a lot more about a totally _independent_ sanity
check, which just happened to be unrelated to the type size. In this case,
having the compiler just silently notice that "oh, this can't happen,
because I know the range in this case" is ok.
And if the code is right, just re-write it in a form that avoids the
warning. So
if (a = b) {
should become
a = b;
if (a) {
because it's clearer _and_ avoids the warning (don't just blindly add a
parenthesis).
That's why I hate the "sign compare" warning of gcc so much - it warns
about things that you CANNOT sanely write in any other way. That makes
that particular warning _evil_, since it encourages people to write crap
code.
In this case, the warning is easily avoided by splitting the code up a
bit, and accepting an unnecessary cast (which hopefully the compiler may
eventually notice it unnecessary). And as the warning in general is good,
that's fine - the warning does not generally _encourage_ bad programming.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: GCC 3.4 Heads-up
Original-Message-ID: <Pine.LNX.4.58.0312252021540.14874@home.osdl.org>
Date: Fri, 26 Dec 2003 04:38:50 GMT
Message-ID: <fa.ifvvhrj.1iguaaf@ifi.uio.no>
On Thu, 25 Dec 2003, H. Peter Anvin wrote:
> >
> > Other than the constant barrage of warnings about the use of compound
> > expressions as lvalues being deprecated* (mostly because of lines 114,
> > 116, and 117 of rcupdate.h, which is included everywhere), the build
> > goes very well.
> >
> > *It also doesn't like cast or conditional expressions as lvalues.
> >
>
> Okay, that's just insane...
Actually, those language extensions (while documented for a long time) are
pretty ugly.
Some of that ugliness turns into literal bugs when C++ is used.
The cast/conditional expression as lvalue are _particularly_ ugly
extensions, since there is absolutely zero point to them. They are very
much against what C is all about, and writing something like this:
a ? b : c = d;
is something that only a high-level language person could have come up
with. The _real_ way to do this in C is to just do
*(a ? &b : &c) = d;
which is portable C, does the same thing, and has no strange semantics.
Similarly, what the _hell_ does the gcc extension
int a;
(char)a += b;
really mean? The whole extension is just braindamaged, again probably
because there were non-C people involved at some point. It's very much
against the C philosophy. I bet 99% of the people on this list have no
clue what the exact semantics of the above are.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: GCC 3.4 Heads-up
Original-Message-ID: <Pine.LNX.4.58.0312252303090.14874@home.osdl.org>
Date: Fri, 26 Dec 2003 07:10:12 GMT
Message-ID: <fa.idgfhbl.1g02aqd@ifi.uio.no>
On Fri, 26 Dec 2003, Andy Isaacson wrote:
>
> But doesn't the first one potentially let the compiler avoid spilling to
> memory, if b and c are both in registers?
Sure, and you can do it as
tmp = d;
a ? b = tmp : c = tmp;
instead if you want to. It all depends on what b/c actually are (ie maybe
they are memory-backed anyway).
The point being that there are no actual _advantages_ to being
non-standard. And there are definitely disadvantages. Lack of portability
being one, semantic confusion being another (the semantic confusion is
more visible in C++, since there lvalue'ness is more complex. But it's
visible in C too, where the gcc extensions can cause buggy programs that
_should_ give syntax errors to possibly compile to unexpected results).
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: GCC 3.4 Heads-up
Original-Message-ID: <Pine.LNX.4.58.0312261151110.14874@home.osdl.org>
Date: Fri, 26 Dec 2003 20:06:30 GMT
Message-ID: <fa.idfrijk.1h0mbic@ifi.uio.no>
On Fri, 26 Dec 2003 linux@horizon.com wrote:
>
> Applied to integer types, it *is* pretty brain damaged. But applied to
> pointer types, it makes a lot more sense.
No it doesn't.
Your example shows all the problems with the thing:
> Or consider the case when the structure doesn't have an explicit size
> and you have a big case statement for parsing it:
>
> switch (a->type) {
> case BAR:
> process_bar_chunk(((struct bar *)a)++);
> break;
Do you _really_ want to write unportable code for no reason?
This can trivially be done portably and readably by just adding a small
bit of verbiage, ie you could have
#define next_ptr(x,type) (void *)(1+(type *)(x))
and just write
process_bar_chunk(a);
a = next_ptr(a, struct bar);
or similar. Suddenly you can compile your code with any compiler you want
to, including one that maybe generates better code than gcc. Including
newer versions of gcc.
And suddenly people can look at your code, and understand what it is
doing, even if they don't know all the gcc extensions. That's _important_.
Some extensions are fairly obvious. I think the "a ? : b" one is pretty
simple, conceptually (ie you can explain it to even a novice C user
without there being any confusion). But the "cast as lvalue" definitely
isn't.
> It's well-defined and has legitimate uses.
It has no legitimate uses. The only upside of it is to avoid typing a few
extra characters, but since using macros can make the code more readable
anyway, that's not a very good argument.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: Use of floating point in the kernel
Original-Message-ID: <Pine.LNX.4.58.0401071948470.2131@home.osdl.org>
Date: Thu, 8 Jan 2004 10:09:14 GMT
Message-ID: <fa.j4skq5e.1eisk0i@ifi.uio.no>
On Wed, 7 Jan 2004, H. Peter Anvin wrote:
>
> Pekka Pietikainen wrote:
> >
> > There are a few instances of use of floating point in 2.6,
>
> Has anyone considered asking the gcc people to add an -fno-fpu (or
> -mno-fpu) option, throwing an error if any FP instructions are used?
We really should, but there really are some rare cases where it is
actually ok.
In particular, you _can_ do math, if you just do the proper
"kernel_fpu_begin()"/"kernel_fpu_end()" around it, and you have reason to
believe that you can assume a math processor exists.
Is it needed? I dunno. I'd frown on it in general, but I don't see it
being fundamentally wrong under the right circumstances.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: Linux 2.6.7-rc2
Original-Message-ID: <Pine.LNX.4.58.0405310948120.4573@ppc970.osdl.org>
Date: Mon, 31 May 2004 16:55:10 GMT
Message-ID: <fa.gtea0e8.920q9s@ifi.uio.no>
On Mon, 31 May 2004, Albert Cahalan wrote:
>
> This would be required because of the -Wno-strict-aliasing
> option. For the 2.7.xx kernels, how about we start off by
> replacing -Wno-strict-aliasing with -std=gnu99 ? It's been
> 5 years since 1999. The "restrict" keyword is useful too.
No can do.
Aliasing in gcc is so broken (_purely_ type-based and no way to avoid it
sanely in older versions) that it's not going to happen.
When we can depend on everybody having gcc-3.3+ something, and that one
properly supports the "may_alias" attribute, we may change that.
"restrict" is pretty much useless. It just weakens the already too-weak
alias rules of standard gcc.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: 2.6.7-rc2-mm1: gcc 2.95 uaccess.h warnings
Original-Message-ID: <Pine.LNX.4.58.0406010757160.14095@ppc970.osdl.org>
Date: Tue, 1 Jun 2004 15:09:06 GMT
Message-ID: <fa.irnvm0a.14k6brq@ifi.uio.no>
On Tue, 1 Jun 2004, Adrian Bunk wrote:
>
> It seems to be Linus'
>
> sparse: make x86 user pointer checks stricter.
>
> patch that causes the following warnings for every single file including
> uaccess.h when using gcc 2.95:
Yes. Apparently gcc-2.95 doesn't like to dereference a "void *" even
just inside of a "__typeof__". Which is a bit silly, since the result is a
perfectly valid _type_. It's obviously been fixed since, so I never saw
this.
(Aside - with anon unions also not working in older gcc's, I suspect we'll
start requiring gcc-3+ in the 2.7.x timeframe. I know it's a lot slower,
but apparently at least it now is starting to generate comparable code
again).
Anyway, the easiest work-around would be to just avoid derefencing the
pointer, but then you have to add strange modifiers (both "const" and
"volatile" at the same time) to make sure that gcc won't complain about
the assignment dropping any modifiers.
So the fix I'm going to check in is to just make "__chk_user_ptr()" be
something that gcc never even sees, which allows us to simplify it for the
sparse case too.
In other words, this should work for even old versions of gcc.. Just to be
anal, you should probably test on gcc-2.95 ;)
Linus
---
===== include/asm-i386/uaccess.h 1.30 vs edited =====
--- 1.30/include/asm-i386/uaccess.h Mon May 31 11:19:35 2004
+++ edited/include/asm-i386/uaccess.h Tue Jun 1 07:55:30 2004
@@ -34,10 +34,6 @@
#define segment_eq(a,b) ((a).seg == (b).seg)
-extern long not_a_user_address;
-#define check_user_ptr(x) \
- (void) ({ void __user * __userptr = (__typeof__(*(x)) *)¬_a_user_address; __userptr; })
/*
* movsl can be slow when source and dest are not both 8-byte aligned
*/
@@ -60,7 +56,7 @@
*/
#define __range_ok(addr,size) ({ \
unsigned long flag,sum; \
- check_user_ptr(addr); \
+ __chk_user_ptr(addr); \
asm("addl %3,%1 ; sbbl %0,%0; cmpl %1,%4; sbbl $0,%0" \
:"=&r" (flag), "=r" (sum) \
:"1" (addr),"g" ((int)(size)),"g" (current_thread_info()->addr_limit.seg)); \
@@ -175,7 +171,7 @@
*/
#define get_user(x,ptr) \
({ int __ret_gu,__val_gu; \
- check_user_ptr(ptr); \
+ __chk_user_ptr(ptr); \
switch(sizeof (*(ptr))) { \
case 1: __get_user_x(1,__ret_gu,__val_gu,ptr); break; \
case 2: __get_user_x(2,__ret_gu,__val_gu,ptr); break; \
@@ -294,7 +290,7 @@
#define __put_user_size(x,ptr,size,retval,errret) \
do { \
retval = 0; \
- check_user_ptr(ptr); \
+ __chk_user_ptr(ptr); \
switch (size) { \
case 1: __put_user_asm(x,ptr,retval,"b","b","iq",errret);break; \
case 2: __put_user_asm(x,ptr,retval,"w","w","ir",errret);break; \
@@ -353,7 +349,7 @@
#define __get_user_size(x,ptr,size,retval,errret) \
do { \
retval = 0; \
- check_user_ptr(ptr); \
+ __chk_user_ptr(ptr); \
switch (size) { \
case 1: __get_user_asm(x,ptr,retval,"b","b","=q",errret);break; \
case 2: __get_user_asm(x,ptr,retval,"w","w","=r",errret);break; \
===== include/asm-ppc64/uaccess.h 1.16 vs edited =====
--- 1.16/include/asm-ppc64/uaccess.h Mon May 31 10:39:25 2004
+++ edited/include/asm-ppc64/uaccess.h Tue Jun 1 07:55:30 2004
@@ -16,10 +16,6 @@
#define VERIFY_READ 0
#define VERIFY_WRITE 1
-extern long not_a_user_address;
-#define check_user_ptr(x) \
- (void) ({ void __user * __userptr = (__typeof__(*(x)) *)¬_a_user_address; __userptr; })
/*
* The fs value determines whether argument validity checking should be
* performed or not. If get_fs() == USER_DS, checking is performed, with
@@ -120,7 +116,7 @@
#define __put_user_nocheck(x,ptr,size) \
({ \
long __pu_err; \
- check_user_ptr(ptr); \
+ __chk_user_ptr(ptr); \
__put_user_size((x),(ptr),(size),__pu_err,-EFAULT); \
__pu_err; \
})
@@ -192,7 +188,7 @@
do { \
might_sleep(); \
retval = 0; \
- check_user_ptr(ptr); \
+ __chk_user_ptr(ptr); \
switch (size) { \
case 1: __get_user_asm(x,ptr,retval,"lbz",errret); break; \
case 2: __get_user_asm(x,ptr,retval,"lhz",errret); break; \
===== include/asm-sparc/uaccess.h 1.11 vs edited =====
--- 1.11/include/asm-sparc/uaccess.h Mon May 31 19:07:59 2004
+++ edited/include/asm-sparc/uaccess.h Tue Jun 1 07:55:00 2004
@@ -83,10 +83,6 @@
extern void __ret_efault(void);
-extern long not_a_user_address;
-#define check_user_ptr(x) \
- (void) ({ void __user * __userptr = (__typeof__(*(x)) *)¬_a_user_address; __userptr; })
/* Uh, these should become the main single-value transfer routines..
* They automatically use the right size if we just have the right
* pointer type..
@@ -98,12 +94,12 @@
*/
#define put_user(x,ptr) ({ \
unsigned long __pu_addr = (unsigned long)(ptr); \
-check_user_ptr(ptr); \
+__chk_user_ptr(ptr); \
__put_user_check((__typeof__(*(ptr)))(x),__pu_addr,sizeof(*(ptr))); })
#define get_user(x,ptr) ({ \
unsigned long __gu_addr = (unsigned long)(ptr); \
-check_user_ptr(ptr); \
+__chk_user_ptr(ptr); \
__get_user_check((x),__gu_addr,sizeof(*(ptr)),__typeof__(*(ptr))); })
/*
===== include/asm-sparc64/uaccess.h 1.10 vs edited =====
--- 1.10/include/asm-sparc64/uaccess.h Mon May 31 16:25:29 2004
+++ edited/include/asm-sparc64/uaccess.h Tue Jun 1 07:55:00 2004
@@ -90,10 +90,6 @@
extern void __ret_efault(void);
-extern long not_a_user_address;
-#define check_user_ptr(x) \
- (void) ({ void __user * __userptr = (__typeof__(*(x)) *)¬_a_user_address; __userptr; })
/* Uh, these should become the main single-value transfer routines..
* They automatically use the right size if we just have the right
* pointer type..
@@ -105,12 +101,12 @@
*/
#define put_user(x,ptr) ({ \
unsigned long __pu_addr = (unsigned long)(ptr); \
-check_user_ptr(ptr); \
+__chk_user_ptr(ptr); \
__put_user_nocheck((__typeof__(*(ptr)))(x),__pu_addr,sizeof(*(ptr))); })
#define get_user(x,ptr) ({ \
unsigned long __gu_addr = (unsigned long)(ptr); \
-check_user_ptr(ptr); \
+__chk_user_ptr(ptr); \
__get_user_nocheck((x),__gu_addr,sizeof(*(ptr)),__typeof__(*(ptr))); })
#define __put_user(x,ptr) put_user(x,ptr)
===== include/linux/compiler.h 1.26 vs edited =====
--- 1.26/include/linux/compiler.h Mon May 31 10:38:43 2004
+++ edited/include/linux/compiler.h Tue Jun 1 07:44:56 2004
@@ -6,11 +6,13 @@
# define __kernel /* default address space */
# define __safe __attribute__((safe))
# define __force __attribute__((force))
+extern void __chk_user_ptr(void __user *);
#else
# define __user
# define __kernel
# define __safe
# define __force
+# define __chk_user_ptr(x) (void)0
#endif
#ifdef __KERNEL__
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: using gcc built-ins for bitops?
Original-Message-ID: <Pine.LNX.4.58.0406240819300.1688@ppc970.osdl.org>
Date: Fri, 25 Jun 2004 13:04:50 GMT
Message-ID: <fa.gsu7vmh.ai6ohj@ifi.uio.no>
On Thu, 24 Jun 2004, Jakub Jelinek wrote:
> >
> > As to your primary question - is this worth doing - I don't have
> > an answer.
>
> It is, for 2 reasons:
> 1) unlike __asm, GCC knows how to schedule the instructions in the builtins
> 2) GCC will handle stuff like ffz (16) at compile time rather than runtime
I'd argue that unless the gcc code can be shown to be clearly better, it
is NOT worth doing.
Adding support for built-ins generates a noticeable maintenance burden,
since we'll still have to support older gcc's and architectures where gcc
does WORSE than we do. And quite frankly, I doubt you'll find any cases
where gcc does any better in any measurable way.
In other words, the rule about gcc builtins should be:
- use them only if they are old enough that the huge majority (possibly
all) of users get them. This is partly because gcc has frankly been
buggy, and often makes assumptions that simply may not be true for the
kernel (ie "it's ok to use library routines").
- only use them if you can show a measurable improvement. Theoretical
arguments simply don't count. Theoretical arguments is why gcc-3.x is
a lot slower than 2.95 and is apparently still not generating
appreciably better code for the kernel (and no, don't bother pointing
me to spec runs, I just don't care. The kernel is what I care about).
So far, the only case where they have been worth it is likely the
"memcpy()" stuff, and even there our previous macros were doing an almost
equivalent job (but were arguably so ugly that it was worth making the
change).
For something like ffs/popcount/etc, I do not see _any_ point to compiler
support. There just aren't any kernel uses that make it worth it. Sounds
like a total micro-optimization for some spec benchmark.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: Being more anal about iospace accesses..
Original-Message-ID: <Pine.LNX.4.58.0409151004370.2333@ppc970.osdl.org>
Date: Wed, 15 Sep 2004 17:24:01 GMT
Message-ID: <fa.gtu5tu8.bicqpk@ifi.uio.no>
On Wed, 15 Sep 2004, Jörn Engel wrote:
>
> C now supports pointer arithmetic with void*?
C doesn't. gcc does. It's a documented extension, and it acts like if it
was done on a byte.
See gcc's user guide "Extension to the C Language Family".
It's a singularly good feature.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: Being more anal about iospace accesses..
Original-Message-ID: <Pine.LNX.4.58.0409151108080.2333@ppc970.osdl.org>
Date: Wed, 15 Sep 2004 18:21:45 GMT
Message-ID: <fa.gse9tuc.a2gqpo@ifi.uio.no>
On Wed, 15 Sep 2004, Nikita Danilov wrote:
>
> Unfortunately it breaks even better identity
>
> foo *p;
>
> p + nr == (foo *)((char *)p + nr * sizeof *p)
No, gcc allows the above, by making sizeof(void) be 1.
And sane compilers would just inform the user at compile-time with a nice
readable message that he's doing something stupid.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: support of older compilers
Original-Message-ID: <Pine.LNX.4.58.0411041133210.2187@ppc970.osdl.org>
Date: Thu, 4 Nov 2004 21:06:14 GMT
Message-ID: <fa.gtt7vma.bimohm@ifi.uio.no>
On Thu, 4 Nov 2004, Adam Heath wrote:
>
> On Wed, 3 Nov 2004, Chris Wedgwood wrote:
>
> > On Wed, Nov 03, 2004 at 05:06:56PM -0600, Adam Heath wrote:
> >
> > > You can't be serious that this is a problem.
> >
> > try it, say gcc 2.95 vs gcc 4.0 ... i think last i checked the older
> > gcc was over twice as fast
>
> I didn't deny the speed difference of older and newer compilers.
>
> But why is this an issue when compiling a kernel? How often do you compile
> your kernel?
First off, for some people that is literally where _most_ of the CPU
cycles go.
Second, it's not just that the compilers are slower. Historically, new gcc
versions are:
- slower
- generate worse code
- buggier
For a _long_ time, the only reason to upgrade gcc was literally C++
support: basic C support was getting _worse_ with new compilers in pretty
much every regard.
Things seem to have improved a bit lately. The gcc-3.x series was
basically not worth it for plain C until 3.3 or so.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: support of older compilers
Original-Message-ID: <Pine.LNX.4.58.0411041353360.2187@ppc970.osdl.org>
Date: Thu, 4 Nov 2004 22:05:30 GMT
Message-ID: <fa.gudm06a.b2sp1m@ifi.uio.no>
On Thu, 4 Nov 2004, Adam Heath wrote:
> >
> > First off, for some people that is literally where _most_ of the CPU
> > cycles go.
>
> So find a fast machine. As I have already said, you don't need to compile a
> kernel for a slow machine/arch *on* a slow machine/arch.
I _have_ a fast machine. Others don't. And quite frankly, even I tend to
prioritize things like "nice and quiet" over absolute speed.
> I don't doubt these are issues. That's not what I am discussing.
Sure it is. You're complaining that developers use old versions of gcc.
They do so for a reason. Old versions of gcc are sometimes better. They
are better in many ways.
Your "use new versions of gcc even if it is slower" argument doesn't make
any _sense_. If the new versions aren't any better, why would you want to
use them?
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: support of older compilers
Original-Message-ID: <Pine.LNX.4.58.0411041544220.2187@ppc970.osdl.org>
Date: Thu, 4 Nov 2004 23:59:09 GMT
Message-ID: <fa.gtthvub.biop9h@ifi.uio.no>
On Thu, 4 Nov 2004, Adam Heath wrote:
>
> But slowness doesn't mean wrong, just by being slow.
No.
"Right" and "wrong" have _nothing_ to do with anything.
The only thing that matters is "what is the best tool". And yes,
performance _is_ an issue in selecting the best tool. It's not the only
one, but it's a major one.
You said so yourself when you claimed people should just buy faster
hardware. Again, the machine you use is just another tool. Why should you
buy a faster machine if performance doesn't matter?
I don't understand why you first dismiss performance, and then go on to
ignore all the _other_ issues I told you about too.
And your argument about "things will get fixed if you use the newer
version" is also not actually true. First off, if it ain't broke in the
older version, then things _literally_ will get fixed by not upgrading in
the first place.
And telling a developer "I'm not using your new version because it sucks
compared to the old one" is actually a perfectly valid thing to do, and is
likely to be _more_ motivational for the developer to get it fixed than
having users that just follow the newest version like stupid sheep.
There are people out there using Linux-2.0. There are probably people even
using linux-1.2. And hey, that's OK. For older machines it may really be
the right choice, especially if they are still doing the same thing they
did several years ago. The notion that you somehow "have to" upgrade to
the newest version of software is BOGUS.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: sparse segfaults
Original-Message-ID: <Pine.LNX.4.58.0411211644200.20993@ppc970.osdl.org>
Date: Mon, 22 Nov 2004 01:04:34 GMT
Message-ID: <fa.ismvog7.17ke9bp@ifi.uio.no>
On Sun, 21 Nov 2004, Jan Engelhardt wrote:
>
> And it's specific to GCC. This kinda ruins some tries to get ICC working on the
> kernel tree :)
Ahh, but Intel compiler developers are cunning, and can make (and afaik,
_did_ make) icc compatible with the good gcc extensions.
And as we all know, the definition of "good gcc extension" really is "the
kernel uses it". (Some of the good ones turned up in C99 and are thus no
longer extensions - obviously gcc wasn't the only compiler that
implemented them).
Good gcc extensions:
- the inline asm syntax (which could be better, but hey, the gcc syntax
is certainly not the worst around either)
- statement expressions (but dammit, it should have used "return" to
return the value)
- short conditionals
- symbol attributes (sections, "used", asm naming, etc)
- local labels and computed goto
- case ranges
- typeof and alignof.
- void * arithmetic
BAD gcc extensions:
- nested functions (gaah)
- extended lvalues (casts, conditionals and comma-operators as lvalues.
They are just too confusing)
- transparent unions (dammit, you could have done it with overloading
instead).
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: sparse segfaults
Original-Message-ID: <Pine.LNX.4.58.0411220812580.20993@ppc970.osdl.org>
Date: Mon, 22 Nov 2004 17:06:17 GMT
Message-ID: <fa.itnjno6.14k283s@ifi.uio.no>
On Mon, 22 Nov 2004, Jan Engelhardt wrote:
>
> >BAD gcc extensions:
>
> You don't have to use them...
We don't, generally. But they are bad even if you DON'T use them, because
they sometimes make obvious syntax errors etc much harder to debug.
For example, the "nested function" thing makes something as simple as a
missing end brace cause the error reporting to be totally off, when gcc
decides that "hey, that's ok, those other function declarations are just
nested functions in the middle of that other function". So you get
something like
file.c: lastline: parse error at end of input
even though the _real_ parse error could have been pinpointed exactly if
gcc did NOT do it's totally braindamaged nested functions. IOW, the
extension causes problems even when you don't use it.
Same goes for the "extended lvalues". They are not only insane, but they
mean that code like
(0,i) = 1;
actually compiles. Why is that a problem? Because some people (ie me) have
used constructs like this in macros to make sure that the macro is
"read-only", ie you have a situation where you don't want people to
mis-use the macro on some architecture. So having
int max_of_something;
#define MAX_SOMETHING (0,max_of_something)
is actually a nice way to make sure nobody does anything like
MAX_SOMETHING = new;
but the gcc extension means that this doesn't actually work.
(Yes, I've been bitten by this. And no, I don't see the _point_ of the
extension - does anybody actually admit to ever _using_ comma- expressions
for assignments?)
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: sparse segfaults
Original-Message-ID: <Pine.LNX.4.58.0411221047140.20993@ppc970.osdl.org>
Date: Mon, 22 Nov 2004 19:27:01 GMT
Message-ID: <fa.is6rogb.164a9bp@ifi.uio.no>
On Mon, 22 Nov 2004, Mitchell Blank Jr wrote:
>
> When I want to do that I just use:
>
> #define MAX_SOMETHING (max_of_something + 0)
Yes, I think I've done that too, and that works. My point is that the
silly comma-expression should _also_ work, and I don't see any _valid_ use
of the comma-expression as an lvalue.
I suspect (but don't have any real argument to back that up) is that the
gcc "extended lvalues" fell out as a side effect from how gcc ended up
doing some random semantic tree parsing, and it wasn't really "designed"
per se, as much as just a random implementation detail. Then somebody
noticed it, and said "cool" and documented it.
That's actually in my opinion a really good way to occasionally find a
more generic form of something - the act of noticing that an algorithm
just happens to give unintentional side effects that can actually be
mis-used. So I don't think that it's a bad way per se to add features,
especially as they then are often free (or even "negative cost", since
_not_ adding the feature would entail having to add a check against it).
And for all I know, many of the _good_ gcc features ended up being done
that way too.
But I think the (unintentional) downsides of these features are bigger
than the advantages.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: sparse segfaults
Original-Message-ID: <Pine.LNX.4.58.0411221059420.20993@ppc970.osdl.org>
Date: Mon, 22 Nov 2004 20:34:09 GMT
Message-ID: <fa.itmnood.14k693n@ifi.uio.no>
On Mon, 22 Nov 2004, Mitchell Blank Jr wrote:
>
> When gcc accepts an arbitrary algebraic expression as an lvalue I'll be
> impressed :-)
Btw, the "+0" thing is something that actually might be dropped pretty
early, and as such a compiler _might_ get it wrong just because it ended
up optimizing the expression away. So you don't need to be all that
impressed, certain trivial expressions might just disappear under some
circumstances.
Side note: the _biggest_ reason why "+0" is hard to optimize away early is
actually type handling, not the expression itself. The C type rules means
that "+0" isn't actually a no-op: it implies type expansion for small
integer types etc.
So I agree that it's unlikely to be a problem in practice, but I literally
think that the reason gcc ends up considering a comma-operator to be an
lvalue, but not a +-operator really _is_ the type-casting issues. A comma
doesn't do implicit type expansion.
What I find really strange is the ternary operator lvalue thing, though. A
ternary operator _does_ do type expansion, so that extended lvalue thing
is really quite complex for ternary ops. Try something like this:
int test(int arg)
{
char c;
int i;
return (arg ? c : i) = 1023;
}
and think about what a total disaster that is. Yes, gcc gets it right, but
dammit, what a total crock. The people who thought of this feature should
just be shot.
(Yes, it looks cool. Oh, well. The compiler can always simplify the
expression "(a ? b : c) = d" into "tmp = d ; a ? b = tmp : c = tmp", but
hey, so can the user, so what's the point? Looking at the output from
gcc, it really looks like gcc actually handles it as a special case,
rather than as the generic simplification. Scary. Scary. Scary.)
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH] Remove pointless <0 comparison for unsigned variable in
Original-Message-ID: <Pine.LNX.4.58.0411230958260.20993@ppc970.osdl.org>
Date: Tue, 23 Nov 2004 18:12:27 GMT
Message-ID: <fa.is7hood.174s93n@ifi.uio.no>
On Tue, 23 Nov 2004, Jesper Juhl wrote:
>
> Linus, would you accept patches like this?
No, please don't.
The warning is sometimes useful, but when it comes to a construct like
if (a < 0 || a > X)
the fact that "a" is unsigned does not make the construct silly. First
off, it's (a) very readable and (b) the type of "a" may not be immediately
obvious if it's a user typedef, for example.
In fact, the type of "a" might depend on the architecture, or even
compiler flags. Think about "char" - which may or may not be signed
depending on ABI and things like -funsigned-char.
In other places, it's not "unsigned" that is the problem, but the fact
that the range of a type is smaller on one architecture than another. So
you might have
inf fn(pid_t a)
{
if (a > 0xffff)
...
}
which might warn on an architecture where "pid_t" is just sixteen bits
wide. Does that make the code wrong? Hell no.
IOW, a lot of the gcc warnings are just not valid, and trying to shut gcc
up about them can break (and _has_ broken) code that was correct before.
> I probably won't be able to properly evaluate/review *all* the instances
> of this in the kernel,
It's not even that I will drop the patches, it's literally that "fixing"
the code so that gcc doesn't complain can be a BUG. We've gone through
that.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: [PATCH] Remove pointless <0 comparison for unsigned variable in
Original-Message-ID: <Pine.LNX.4.58.0411231035500.20993@ppc970.osdl.org>
Date: Tue, 23 Nov 2004 18:43:42 GMT
Message-ID: <fa.iu6jo8a.14428jq@ifi.uio.no>
On Tue, 23 Nov 2004, Jesper Juhl wrote:
>
> Shutting up gcc is not the primary goal here, the goal is/was to
> a) review the code and make sure that it is safe and correct, and fix it
> when it is not.
> b) remove comparisons that are just a waste of CPU cycles when the result
> is always true or false (in *all* cases on *all* archs).
Well, I'm convinced that (b) is unnecessary, as any compiler that notices
the range thing enough to warn will also be smart enough to just remove
the test internally.
But yes, as long as the thing is a "review and fix bugs" and not a quest
to remove warnings which may well be compiler figments, that's obviously
ok.
Linus
Newsgroups: fa.linux.kernel
From: Al Viro <viro@parcelfarce.linux.theplanet.co.uk>
Subject: Re: How to write elegant C coding
Original-Message-ID: <20050105081415.GH26051@parcelfarce.linux.theplanet.co.uk>
Date: Wed, 5 Jan 2005 08:16:42 GMT
Message-ID: <fa.n3stbal.1l1qor1@ifi.uio.no>
On Wed, Jan 05, 2005 at 03:49:07PM +0800, Coywolf Qi Hunt wrote:
> I'd say better to study compile theory and a kind of compiler source code.
Yes, gcc source definitely makes a great cautionary tale about the need of
writing elegant code and dreadful results of not doing so.
Newsgroups: fa.linux.kernel
From: Linus Torvalds <torvalds@osdl.org>
Subject: Re: removing bcopy... because it's half broken
Original-Message-ID: <Pine.LNX.4.58.0501110805560.2373@ppc970.osdl.org>
Date: Tue, 11 Jan 2005 16:15:28 GMT
Message-ID: <fa.gue3uu6.82erpg@ifi.uio.no>
On Tue, 11 Jan 2005, Bastian Blank wrote:
>
> Yes. This means IMHO that the image and every module needs to link
> against libgcc to include the required symbols. It is rather annoying to
> see modules asking for libgcc symbols.
Some architectures do that. Not all. My argument has always been that we
don't _want_ any code that gcc cannot generate.
The kernel very much on purpose does not trust gcc. There have been some
total braindamages over time, like having exception handling turned on by
default by gcc by default in plain C, and one of the reasons we noticed
was that the link wouldn't work - libgcc has the exception support, and
the kernel simply doesn't WANT that kind of crap.
It's also been useful (although at times a bit painful) to find cases
where people did stuff that simply shouldn't be done in the kernel. Things
like FP conversions, or - more commonly - 64-bit divides on hardware where
that is very slow.
It does mean that we have to know about some gcc internals ourselves, and
have our own libgcc versions for the stuff we _do_ want.
Linus
From: Linus Torvalds <torvalds@osdl.org>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH 1/19] MUTEX: Introduce simple mutex implementation
Date: Tue, 13 Dec 2005 16:17:56 UTC
Message-ID: <fa.h9416cu.i783oa@ifi.uio.no>
Original-Message-ID: <Pine.LNX.4.64.0512130812020.15597@g5.osdl.org>
On Tue, 13 Dec 2005, Andi Kleen wrote:
>
> Remove -Wdeclaration-after-statement
Please don't.
It's a coding style issue. We put our variable declarations where people
can _find_ them, not in random places in the code.
Putting variables in the middle of code only improves readability when you
have messy code.
Now, one feature that _may_ be worth it is the loop counter thing:
for (int i = 10; i; i--)
...
kind of syntax actually makes sense and is a real feature (it makes "i"
local to the loop, and can actually help people avoid bugs - you can't use
"i" by mistake after the loop).
But I think you need "--std=c99" for gcc to take that.
Linus
From: Linus Torvalds <torvalds@osdl.org>
Newsgroups: fa.linux.kernel
Subject: Re: gcc stack problem
Date: Sat, 22 Apr 2006 21:15:39 UTC
Message-ID: <fa.1s2vnCGy080lV1Zpehf7eKfRm0w@ifi.uio.no>
Original-Message-ID: <Pine.LNX.4.64.0604211823160.3701@g5.osdl.org>
On Fri, 21 Apr 2006, Albert Cahalan wrote:
>
> I reported that problem involving asmlinkage and prevent_tail_call.
It's been discussed before, although I cannot for the life of me come up
with the right magic query for google to find it ;(
Btw, the "prevent_tail_call()" thing is really badly named. It does
exactly what it says, but the problem isn't even really fundamentally
tailcalls, that just is the detail that happens to trigger the problem (but
I could imagine other situations triggering it _too_, which is why the
naming is bad).
We don't actually care about tailcalls (they're absolutely _fine_, if the
tailcall arguments are all in registers, not overwriting the caller
stack-frame), so "prevent_tail_call()" really talks about the wrong thing.
In other words, it's too tightly coupled to an implementation issue, not
to the more fundamental _conceptual_ issue, which is that the caller owns
the stackframe at that point.
> I hope I got the details right. Here it is:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27234
>
> Note comment #3, suggesting that following the ABI would
> be a better way to write the assembly.
Sure, we could just do a slower system call entry. We always knew that.
Suggesting that as the solution is pretty stupid, though. That's _the_
most timing-critical part in the whole kernel on many loads. We've
literally spent time trying to remove single cycles there, and it matters.
I'd much rather have an officially sanctioned way to do what Linux wants
to do, but in the meantime, we can (and are forced to) rely on hacks like
"prevent_tail_call()". They are hacks, and we _know_ they are hacks, but
as long as gcc maintainers don't want to help us, we don't have much
choice (although we could perhaps make the hacks a bit nicer ;).
The fact is, the "official ABI" simply isn't appropriate. In fact, we
don't use the "official ABI" _anywhere_ in the kernel any more, since we
actually end up using other gcc calling conventions (ie regparm=3).
Btw, this is not even unusual. A lot of embedded platforms have support
for magic exception/interrupt calling conventions. Gcc even supports them:
things like "__attribute__((interrupt))". This is also exactly analogous
to stdcall/cdecl/regparm/longcall/naked/sp_switch/trap_exit etc
attributes.
So gcc already has support for the fact that people sometimes need special
calling conventions. We've historically worked around it by hand instead,
since our calling convention is very _close_ to the standard one.
In fact, the calling convention we want there is _so_ close to the
standard one, that I'm not even convinced the i386 ABI really says that
the callee owns the parameter space - it may well be a local gcc decision
rather than any "official i386 ABI" issue.
Btw, we don't even need a real attribute. That would be the cleanest and
easiest way by far, but the hack mostly works, and I'd be more than
happy to just perhaps clean up the hacky "do it by hand" thing a bit.
For example, I've considered replacing the ugly "prevent_tail_call()" with
a slightly different macro that talks less about tailcalls, and talks
more about the ABI we want:
static inline long system_call_ret(long retval, void *arg)
{
asm("":
"=r" (retval),
"+m" (*(struct pt_regs *)arg)
:"0" (retval));
return retval;
}
#ifdef __x86__
#define sys_return(x, arg1) \
return system_call_ret((x), &(arg1))
#else
define sys_return(x, arg1) \
return (x)
#endif
where we'd make it extra clear that on x86 we're still _using_ the memory
pointed to by "arg1" when we return (so that any other strange gcc
optimization would also be kept from touching the call frame, not just
tailcalls).
That said, I'd much rather have a real gcc attribute. I don't _like_
having horrible hacks like the above to make gcc do what I want it to do.
And I know the gcc developers don't like it either when I force gcc to be
my biatch. It would be much better for everybody if gcc helped a bit.
Linus
From: Linus Torvalds <torvalds@osdl.org>
Newsgroups: fa.linux.kernel
Subject: Re: gcc stack problem
Date: Sat, 22 Apr 2006 21:19:44 UTC
Message-ID: <fa.AK5GIIu6+DzVgWWgtamkdek04Ao@ifi.uio.no>
Original-Message-ID: <Pine.LNX.4.64.0604211932420.3701@g5.osdl.org>
On Fri, 21 Apr 2006, Linus Torvalds wrote:
>
> For example, I've considered replacing the ugly "prevent_tail_call()" with
> a slightly different macro that talks less about tailcalls, and talks
> more about the ABI we want:
Here's a (untested!) patch to illustrate what I'm talking about.
Now, this is still the same old hacky "force gcc to do what we want", just
more geared towards _why_ why want that particular effect. And the reason
I don't like it all that much is that while it should generate the code we
want, we don't want to do this for all system calls - because some system
calls don't need it!
As mentioned, tailcalls are fine per se, it's only really a problem when
gcc generates them so that they put crud in the stack. That, in turn,
happens only when the tailcall needs more arguments than fit in registers.
And THAT only happens for some system calls, and depends on the calling
convention used inside the kernel (ie regparm-3 for normal non-asmlinkage
functions).
So if gcc had a "caller_arguments" attribute, we could just add it to the
definition of 'asmlinkage', and automatically cover all system calls. AND
it would allow tailcalls when they are appropriate and work fine. In
contrast, when we have to force it, we have to use this kind of stupid
"nail just the particular system calls".
This is why it's nice to tell the compiler about the true calling
convention. Because that leaves the compiler able to DTRT.
Linus
---
diff --git a/include/asm-i386/linkage.h b/include/asm-i386/linkage.h
index f4a6eba..a6068ac 100644
--- a/include/asm-i386/linkage.h
+++ b/include/asm-i386/linkage.h
@@ -5,7 +5,24 @@ #define asmlinkage CPP_ASMLINKAGE __attr
#define FASTCALL(x) x __attribute__((regparm(3)))
#define fastcall __attribute__((regparm(3)))
-#define prevent_tail_call(ret) __asm__ ("" : "=r" (ret) : "0" (ret))
+struct dummy { int regs[6]; };
+
+/*
+ * On x86, the caller owns the system call arguments.
+ *
+ * We tell that to gcc by telling that we still want them
+ * in memory just before the final return.
+ *
+ * Other systems might want to set error flags or
+ * something.
+ */
+#define system_call_return(ret,arg1) do { \
+ long syscall_ret_value = (ret); \
+ asm("":"=r" (syscall_ret_value), \
+ "+m" (*(struct dummy *)&(arg1)) \
+ :"0" (syscall_ret_value)); \
+ return syscall_ret_value; \
+} while (0)
#ifdef CONFIG_X86_ALIGNMENT_16
#define __ALIGN .align 16,0x90
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index c08c998..ba51ba7 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -14,8 +14,8 @@ #ifndef asmlinkage
#define asmlinkage CPP_ASMLINKAGE
#endif
-#ifndef prevent_tail_call
-# define prevent_tail_call(ret) do { } while (0)
+#ifndef system_call_return
+# define system_call_return(ret,arg1) return (ret)
#endif
#ifndef __ALIGN
diff --git a/fs/open.c b/fs/open.c
index 53ec28c..e07d8c4 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -331,25 +331,25 @@ out:
asmlinkage long sys_ftruncate(unsigned int fd, unsigned long length)
{
- long ret = do_sys_ftruncate(fd, length, 1);
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ do_sys_ftruncate(fd, length, 1),
+ fd);
}
/* LFS versions of truncate are only needed on 32 bit machines */
#if BITS_PER_LONG == 32
asmlinkage long sys_truncate64(const char __user * path, loff_t length)
{
- return do_sys_truncate(path, length);
+ system_call_return(
+ do_sys_truncate(path, length),
+ path);
}
asmlinkage long sys_ftruncate64(unsigned int fd, loff_t length)
{
- long ret = do_sys_ftruncate(fd, length, 0);
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ do_sys_ftruncate(fd, length, 0),
+ fd);
}
#endif
@@ -1099,30 +1099,24 @@ long do_sys_open(int dfd, const char __u
asmlinkage long sys_open(const char __user *filename, int flags, int mode)
{
- long ret;
if (force_o_largefile())
flags |= O_LARGEFILE;
- ret = do_sys_open(AT_FDCWD, filename, flags, mode);
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ do_sys_open(AT_FDCWD, filename, flags, mode),
+ filename);
}
EXPORT_SYMBOL_GPL(sys_open);
asmlinkage long sys_openat(int dfd, const char __user *filename, int flags,
int mode)
{
- long ret;
if (force_o_largefile())
flags |= O_LARGEFILE;
- ret = do_sys_open(dfd, filename, flags, mode);
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ do_sys_open(dfd, filename, flags, mode),
+ dfd);
}
EXPORT_SYMBOL_GPL(sys_openat);
diff --git a/kernel/exit.c b/kernel/exit.c
index f86434d..3bbc37d 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1584,8 +1584,6 @@ asmlinkage long sys_waitid(int which, pi
struct siginfo __user *infop, int options,
struct rusage __user *ru)
{
- long ret;
if (options & ~(WNOHANG|WNOWAIT|WEXITED|WSTOPPED|WCONTINUED))
return -EINVAL;
if (!(options & (WEXITED|WSTOPPED|WCONTINUED)))
@@ -1608,26 +1606,20 @@ asmlinkage long sys_waitid(int which, pi
return -EINVAL;
}
- ret = do_wait(pid, options, infop, NULL, ru);
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ do_wait(pid, options, infop, NULL, ru),
+ which);
}
asmlinkage long sys_wait4(pid_t pid, int __user *stat_addr,
int options, struct rusage __user *ru)
{
- long ret;
if (options & ~(WNOHANG|WUNTRACED|WCONTINUED|
__WNOTHREAD|__WCLONE|__WALL))
return -EINVAL;
- ret = do_wait(pid, options | WEXITED, NULL, stat_addr, ru);
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ do_wait(pid, options | WEXITED, NULL, stat_addr, ru),
+ pid);
}
#ifdef __ARCH_WANT_SYS_WAITPID
diff --git a/kernel/uid16.c b/kernel/uid16.c
index 187e2a4..c09b8dd 100644
--- a/kernel/uid16.c
+++ b/kernel/uid16.c
@@ -20,67 +20,58 @@ #include <asm/uaccess.h>
asmlinkage long sys_chown16(const char __user * filename, old_uid_t user, old_gid_t group)
{
- long ret = sys_chown(filename, low2highuid(user), low2highgid(group));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_chown(filename, low2highuid(user), low2highgid(group)),
+ filename);
}
asmlinkage long sys_lchown16(const char __user * filename, old_uid_t user, old_gid_t group)
{
- long ret = sys_lchown(filename, low2highuid(user), low2highgid(group));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_lchown(filename, low2highuid(user), low2highgid(group)),
+ filename);
}
asmlinkage long sys_fchown16(unsigned int fd, old_uid_t user, old_gid_t group)
{
- long ret = sys_fchown(fd, low2highuid(user), low2highgid(group));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_fchown(fd, low2highuid(user), low2highgid(group)),
+ fd);
}
asmlinkage long sys_setregid16(old_gid_t rgid, old_gid_t egid)
{
- long ret = sys_setregid(low2highgid(rgid), low2highgid(egid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setregid(low2highgid(rgid), low2highgid(egid)),
+ rgid);
}
asmlinkage long sys_setgid16(old_gid_t gid)
{
- long ret = sys_setgid(low2highgid(gid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setgid(low2highgid(gid)),
+ gid);
}
asmlinkage long sys_setreuid16(old_uid_t ruid, old_uid_t euid)
{
- long ret = sys_setreuid(low2highuid(ruid), low2highuid(euid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setreuid(low2highuid(ruid), low2highuid(euid)),
+ ruid);
}
asmlinkage long sys_setuid16(old_uid_t uid)
{
- long ret = sys_setuid(low2highuid(uid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setuid(low2highuid(uid)),
+ uid);
}
asmlinkage long sys_setresuid16(old_uid_t ruid, old_uid_t euid, old_uid_t suid)
{
- long ret = sys_setresuid(low2highuid(ruid), low2highuid(euid),
- low2highuid(suid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setresuid(low2highuid(ruid), low2highuid(euid), low2highuid(suid)),
+ ruid);
}
asmlinkage long sys_getresuid16(old_uid_t __user *ruid, old_uid_t __user *euid, old_uid_t __user *suid)
@@ -96,11 +87,9 @@ asmlinkage long sys_getresuid16(old_uid_
asmlinkage long sys_setresgid16(old_gid_t rgid, old_gid_t egid, old_gid_t sgid)
{
- long ret = sys_setresgid(low2highgid(rgid), low2highgid(egid),
- low2highgid(sgid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setresgid(low2highgid(rgid), low2highgid(egid), low2highgid(sgid)),
+ rgid);
}
asmlinkage long sys_getresgid16(old_gid_t __user *rgid, old_gid_t __user *egid, old_gid_t __user *sgid)
@@ -116,18 +105,16 @@ asmlinkage long sys_getresgid16(old_gid_
asmlinkage long sys_setfsuid16(old_uid_t uid)
{
- long ret = sys_setfsuid(low2highuid(uid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setfsuid(low2highuid(uid)),
+ uid);
}
asmlinkage long sys_setfsgid16(old_gid_t gid)
{
- long ret = sys_setfsgid(low2highgid(gid));
- /* avoid REGPARM breakage on x86: */
- prevent_tail_call(ret);
- return ret;
+ system_call_return(
+ sys_setfsgid(low2highgid(gid)),
+ gid);
}
static int groups16_to_user(old_gid_t __user *grouplist,
From: Linus Torvalds <torvalds@osdl.org>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 22:28:52 UTC
Message-ID: <fa.BEayQ9EkuS0NH/DNyGfMdV6zoRQ@ifi.uio.no>
On Tue, 28 Nov 2006, Jesper Juhl wrote:
>
> In kernel/sys.c::sys_prctl() the argument named 'arg2' is very clearly
> of type 'unsigned long', and when compiling with "gcc -W" gcc also warns :
> kernel/sys.c:2089: warning: comparison of unsigned expression < 0 is always false
>
> So this patch removes the test of "arg2 < 0".
No, we don't do this.
This is why we don't compile with "-W". Gcc is crap.
The fact is, if it's unsigned, it's not something that the programmer
should have to care about. We should write our code to be readable and
obviously safe, and that means that
if (x < 0 || x > MAX)
return -ERROR;
is the _right_ way to do things, without having to carry stupid context
around in our heads.
If the compiler (whose _job_ it is to carry all that context and use it to
generate good code) notices that the fact that "x" is unsigned means that
one of the tests is unnecessary, that does not make it wrong.
Gcc warns for a lot of wrong things. This is one of them.
Friends don't let friends use "-W".
Linus
From: Linus Torvalds <torvalds@osdl.org>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 23:07:56 UTC
Message-ID: <fa.o1w80AlZO3RGn6IdyI0cGaGrJR0@ifi.uio.no>
On Tue, 28 Nov 2006, Jesper Juhl wrote:
>
> > Friends don't let friends use "-W".
>
> Hehe, ok, I'll stop cleaning this stuff up then.
> Nice little hobby out the window there ;)
You might want to look at some of the other warnings gcc spits out, but
this class isn't one of them.
Other warnings we have added over the years (and that really _are_ good
warnings) have included the "-Wstrict-prototypes", and some other ones.
If you can pinpoint _which_ gcc warning flag it is that causes gcc to emit
the bogus ones, you _could_ try "-W -Wno-xyz-warning", which should cause
gcc to enable all the "other" warnings, but then not the "xyz-warning"
that causes problems.
Of course, there is often a reason why a warning is in "-W" but not in
"-Wall". Most of the time it's sign that the warning is bogus. Not always,
though - we do tend to want to be fairly strict, and Wstrict-prototypes is
an example of a _good_ warning that is not in -Wall.
Linus
From: Linus Torvalds <torvalds@osdl.org>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Wed, 29 Nov 2006 00:14:29 UTC
Message-ID: <fa.S0n9BQ6aglSBJp9Q7yIQOD4b2Q0@ifi.uio.no>
On Wed, 29 Nov 2006, Jesper Juhl wrote:
>
> I would venture that "-Wshadow" is another one of those.
I'd agree, except for the fact that gcc does a horribly _bad_ job of
-Wshadow, making it (again) totally unusable.
For example, it's often entirely interesting to hear about local variables
that shadow each other. No question about it.
HOWEVER. It's _not_ really interesting to hear about a local variable that
happens to have a common name that is also shared by a extern function.
There just isn't any room for confusion, and it's actually not even that
unusual - I tried using -Wshadow on real programs, and it was just
horribly irritating.
In the kernel, we had obvious things like local use of "jiffies" that just
make _total_ sense in a small inline function, and the fact that there
happens to be an extern declaration for "jiffies" just isn't very
interesting.
Similarly, with nested macro expansion, even the "local variable shadows
another local variable" case - that looks like it should have an obvious
warning on the face of it - really isn't always necessarily that
interesting after all. Maybe it is a bug, maybe it isn't, but it's no
longer _obviously_ bogus any more.
So I'm not convinced about the usefulness of "-Wshadow". ESPECIALLY the
way that gcc implements it, it's almost totally useless in real life.
For example, I tried it on "git" one time, and this is a perfect example
of why "-Wshadow" is totally broken:
diff-delta.c: In function 'create_delta_index':
diff-delta.c:142: warning: declaration of 'index' shadows a global declaration
(and there's a _lot_ of those). If I'm not allowed to use "index" as a
local variable and include <string.h> at the same time, something is
simply SERIOUSLY WRONG with the warning.
So the fact is, the C language has scoping rules for a reason. Can you
screw yourself by using them badly? Sure. But that does NOT mean that the
same name in different scopes is a bad thing that should be warned about.
If I wanted a language that didn't allow me to do anything wrong, I'd be
using Pascal. As it is, it turns out that things that "look" wrong on a
local level are often not wrong after all.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 08 Feb 2007 16:34:11 UTC
Message-ID: <fa.6wnj5qnl/e+ZdnLp0vqZIiDg2+M@ifi.uio.no>
On Thu, 8 Feb 2007, Jeff Garzik wrote:
>
> Just did a pull for the first time since 2.6.20, and a /megaton/ of new
> warnings have appeared, on Fedora Core 6/x86-64 "make allyesconfig".
That was due to the Makefile rule breakage. Hopefully it now _really_ is
fixed. The
CFLAGS += $(call cc-option,-Wno-pointer-sign,)
part effectively got dropped.
> All of the new warnings spew appear to be "pointers differ in signedness"
> warning. Granted, we should care about these
The thing is, I'm _really_ sad we have to use that -Wno-pointer-sign
thing. It's a real and valid warning. It finds really ugly code issues
(unlike a lot of the other things). So it got dropped by mistake, but that
is one warning that I'd actually like to resurrect (unlike a lot of other
gcc warnings that I just think are crap to begin with).
However, we've got a metric shitload of code that passes in pointers to
the wrong sign, so a lot of it would involve mindless changes that are
likely to break real things.
And the thing is, sometimes -Wpointer-sign-compare is just horribly
broken. For example, you cannot write a "strlen()" that doesn't complain
about "unsigned char *" vs "char *". And that is just a BUG.
I'm continually amazed at how totally clueless some gcc warnings are.
You'd think that the people writing them are competent. And then sometimes
they show just how totally incompetent they are, by making it impossible
to use "unsigned char" arrays together with standard functions.
So a _lot_ of the warnings are real. But with no way to shut up the ones
that aren't, it sadly doesn't matter one whit ;(
(And adding casts is just much worse than shutting up the warning
entirely)
There should be a rule about warnings: if you can't shut up stylistic
warnings on a site-by-site level without adding horribly syntactic stuff
that is worse than what the warning is all about, the warning shouldn't
exist.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 08 Feb 2007 19:54:29 UTC
Message-ID: <fa.JwqHo4sDKXLxCXLndqcL09lyA2Q@ifi.uio.no>
On Thu, 8 Feb 2007, Jan Engelhardt wrote:
>
> I generally have to agree with you about the unsigned char* vs char*. It
> is a problem of the C language that char can be signed and unsigned, and
> that people, as a result, have used it for storing
> "shorter_than_short_t"s.
>
> What C needs is a distinction between char and int8_t, rendering "char"
> an unsigned at all times basically and making "unsigned char" and
> "signed char" illegal types in turn.
No, it's really more fundamental than that.
Exactly because "char *" doesn't have a defined sign, only a TOTALLY
INCOMPETENT compiler will warn about its signedness.
The user has clearly stated "I don't care about the sign". If a compiler
complains about us passing "unsigned char *" (or, if "char" is naturally
unsigned on that platform, "signed char *") to strcmp then that compiler
IS BROKEN. Because "strcmp()" takes "char *", which simply DOES NOT HAVE a
uniquely defined sign.
That's why we can't have -Wpointer-sign on by default. The gcc warning is
simply *crap*.
If you were to have
extern int strcmp(signed char *);
and would pass *that* an "unsigned char", it would be a real and valid
sign error. But just plain "char *" isn't signed or unsigned. It's
"implementation defined", and as such, if the programmer used it, the
programmer clearly doesn't care. If he cared, he would just state the
signedness explicitly.
It really is that simple. gcc is broken. The C language isn't, it's purely
a broken compiler issue.
The other things in the kernel I'd be willing to fix up. But I simply
refuse to work around a known-buggy warning.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 08 Feb 2007 21:38:34 UTC
Message-ID: <fa.9L8ch/tLrHvKJ6fAH+mnGpPhA7Y@ifi.uio.no>
On Thu, 8 Feb 2007, Jan Engelhardt wrote:
>
> Thank you for this insight, I don't usually read standards, only RFCs :)
> Uh, does that also apply to the longer types, int, long etc.? I hope not.
No. An "int" or "long" is always signed.
But bitfields, enums or "char" can - and do - have signedness that depends
on the architecture and/or compiler writers whims.
So if you do
enum my_enum {
orange = 1,
blue = 2,
green = 3
};
you don't know if "enum my_enum" is signed or not. The compiler could
decide to use "int". Or it could decide to use "unsigned char" for it. You
simply don't know.
Same goes for
struct dummy {
int flag:1;
} a_variable;
which could make "a_variable.d" be either signed or unsigned. In the
absense of an explicit "signed" or "unsigned" by the programmer, you
really won't even _know_ whether "flag" can contain the values {0,1} or
{-1,0}.
Normally you wouldn't ever even care. A one-bit bitfield would only ever
really be tested against 0, but it really is possible that when you assign
"1" to that value, and read it back, it will read back -1. Try it.
Those are the three only types I can think of, but the point is, they
really don't have any standard-defined sign. It's up to the compiler.
Now, passing a "pointer to bitfield" is impossible, and if you pass a
pointer to an enum you'd better *always* use the same enum anyway (because
not only is the signedness implementation-defined, the _size_ of the enum
is also implementation-defined), so in those two cases, -Wpointer-sign
doesn't hurt.
But in the case of "char", it really is insane. It's doubly insane exactly
because the C standard defines a lot of standard functions that take "char
*", and that make tons of sense with unsigned chars too, and you may have
100% good reasons to use "unsigned char" for many of them.
> >It really is that simple. gcc is broken. The C language isn't, it's purely
> >a broken compiler issue.
>
> Maybe you could send in a patch to gcc that fixes the issue?
I've butted heads with too many broken gcc decisions. I've occasionally
used to try to discuss these kinds of things on the gcc mailing lists, but
I got too fed up with language lawyers that said "the standard _allows_ us
to be total *ssholes, so stop complaining". It seemed to be an almost
universal defense.
(And yes, the standard allows any warnings at all. So technically, gcc can
warn about whatever hell it wants. It doesn't make it a good idea).
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 08 Feb 2007 23:38:26 UTC
Message-ID: <fa.rQaVFvOiCp4BAwBTdNamxFfibs4@ifi.uio.no>
On Thu, 8 Feb 2007, David Rientjes wrote:
>
> And a compiler that makes a_variable.flag unsigned would be brain-dead
> because "int" is always signed.
No, making bitfields unsigned is actually usually a good idea. It allows
you to often generate better code, and it actually tends to be what
programmers _expect_. A lot of people seem to be surprised to hear that a
one-bit bitfield actually often encodes -1/0, and not 0/1.
So unsigned bitfields are not only traditional K&R, they are also usually
_faster_ (which is probably why they are traditional K&R - along with
allowing "char" to be unsigned by default). Don't knock them. It's much
better to just remember that bitfields simply don't _have_ any standard
sign unless you specify it explicitly, than saying "it should be signed
because 'int' is signed".
I will actually argue that having signed bit-fields is almost always a
bug, and that as a result you should _never_ use "int" at all. Especially
as you might as well just write it as
signed a:1;
if you really want a signed bitfield.
So I would really recommend that you never use "int a:<bits>" AT ALL,
because there really is never any good reason to do so. Do it as
unsigned a:3;
signed b:2;
but never
int c:4;
because the latter really isn't sensible.
"sparse" will actually complain about single-bit signed bitfields, and it
found a number of cases where people used that "int x:1" kind of syntax.
Just don't do it.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Fri, 09 Feb 2007 00:43:23 UTC
Message-ID: <fa.3Mdxp84Tr+L5++LDigPmzF1RBw8@ifi.uio.no>
On Thu, 8 Feb 2007, David Rientjes wrote:
>
> Your struct:
>
> struct dummy {
> int flag:1;
> } a_variable;
>
> should expect a_variable.flag to be signed, that's what the int says.
No it's not.
You just don't understand the C language.
And if you don't understand the C language, you can't say "that's what the
int says". It says no such thing.
The C language clearly says that bitfields have implementation-defined
types. So when you see
struct dummy {
int flag:1;
} a_variable;
if you don't read that as "oh, the sign of 'flag' is implementation-
defined", then you simply aren't reading it right.
Is it "intuitive"? Nobody ever really called C an _intuitive_ language. C
has a lot of rules that you simply have to know. The bitfield sign rule is
just one such rule.
> There is no special case here with regard to type.
Sure there is. Read the spec.
I don't understand why you are arguing. YOU ARE WRONG.
Bitfields simply have implementation-defined signedness. As do enums. As
does "char". It really is that simple.
The *real* special case is actually "int" and "long". In many ways, the
fact that those *do* have a well-specified signedness is actually the
exception rather than the rule.
Most C types don't, and some you can't even tell (do pointers generate
"signed" or "unsigned" comparisons? I'll argue that a compiler that
generates signed comparisons for them is broken, but it tends to be
something you can only see with a standards- conforming proghram if you
can allocate memory across the sign boundary, which may or may not be
true..)
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Fri, 09 Feb 2007 01:00:01 UTC
Message-ID: <fa.hVil9VZqUdhpn0YnrmKYFBV+HJs@ifi.uio.no>
On Thu, 8 Feb 2007, Linus Torvalds wrote:
>
> The C language clearly says that bitfields have implementation-defined
> types.
Btw, this isn't even some "theoretical discussion". LOTS of compilers do
unsigned bitfields. It may even be the majority. There may (or may not) be
some correlation with what the default for "char" is, I haven't looked
into it.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Fri, 09 Feb 2007 01:11:48 UTC
Message-ID: <fa.tKhLJu63MhyaKUu5vrxPvhFsq8s@ifi.uio.no>
On Thu, 8 Feb 2007, David Rientjes wrote:
>
> Maybe you should read my first post, we're talking about gcc's behavior
> here, not the C standard.
Give it up, David.
Even gcc DOES DIFFERENT THINGS! Have you even read the docs?
By default it is treated as signed int but this may be changed by
the -funsigned-bitfields option.
So even for gcc, it's just a default. I think you can even change the
default in the spec file.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Fri, 09 Feb 2007 15:49:23 UTC
Message-ID: <fa.1TXAidaaEyGbTROyWwfS65/zs8w@ifi.uio.no>
On Thu, 8 Feb 2007, David Rientjes wrote:
>
> Yes, I read the 4.1.1 docs:
>
> By default, such a bit-field is signed, because this is
> consistent: the basic integer types such as int are signed
> types.
Ok, so the gcc people have added some language. So what? The fact is, that
language has absolutely nothing to do with the C language, and doesn't
change anything at all.
> That is the whole basis for my argument, when you declare something "int,"
> most programmers would consider that to be SIGNED regardless of whether it
> is 32 bits, 13 bits, or 1 bit.
And MY argument is that YOUR argument is CRAP.
The fact is, signs of bitfields, chars and enums aren't well-defined.
That's a FACT.
Your argument makes no sense. It's like arguing against "gravity", or like
arguing against the fact that the earth circles around the sun. It us
STUPID to argue against facts.
Your argument is exactly the same argument that people used to say that
the earth is the center of the universe: "because it makes sense that
we're the center".
The fact that "most programmers" or "it makes sense" doesn't make anything
at all true. Only verifiable FACTS make something true.
There's a great saying: "Reality is that which, when you stop believing in
it, doesn't go away." - Philip K Dick.
So wake up and smell the coffee: reality is that bitfields don't have a
specified sign. It's just a FACT. Whether you _like_ it or not is simply
not relevant.
The same is true of "char". You can argue that it should be signed by
default, since "short", "int" and "long" are signed. But THAT IS NOT TRUE.
Arguing against reality really isn't very smart.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 08 Feb 2007 22:03:45 UTC
Message-ID: <fa.flw8JTinXGkfIFTyhT08uPDIiRo@ifi.uio.no>
On Thu, 8 Feb 2007, Linus Torvalds wrote:
>
> But THE CALLER CANNOT AND MUST NOT CARE! Because the sign of "char" is
> implementation-defined, so if you call "strcmp()", you are already
> basically saying: I don't care (and I _cannot_ care) what sign you are
> using.
Let me explain it another way.
Say you use
signed char *myname, *yourname;
if (strcmp(myname,yourname) < 0)
printf("Ha, I win!\n")
and you compile this on an architecture where "char" is signed even
without the explicit "signed".
What should happen?
Should you get a warning? The types really *are* the same, so getting a
warning sounds obviously insane. But on the other hand, if you really care
about the sign that strcmp() uses internally, the code is wrong *anyway*,
because with another compiler, or with the *same* compiler on another
architecture or some other compiler flags, the very same code is buggy.
In other words, either you should get a warning *regardless* of whether
the sign actually matches or not, or you shouldn't get a warning at all
for the above code. Either it's buggy code, or it isn't.
Warning only when the sign doesn't _happen_ to match is crap. In that
case, it's not a warning about bad code, it's a warning about a bad
*compiler*.
My suggestion is that if you *really* care about the sign so much that you
want the sign warning, make it really obvious to the compiler. Don't ever
call functions that have implicit signs. Make even "int" arguments (which
is well-defined in its sign) use "signed int", and then you can make the
compiler warn if anybody ever passes it an "unsigned int".
Never mind even a pointer - if somebody actually took the time and effort
to spell out "signed int" in a function prototype, and you pass that
function an unsigned integer, maybe a warning is perfectly fine. Clearly
the programmer really cared, and if he didn't care about the sign that
much, he could have used just "int".
Conversely, if somebody has a function with a "unsigned int" prototype,
and you pass it a regular "int", a compiler shouldn't complain, because an
"int" will just silently promote to unsigned. But perhaps the programmer
passes it something that he had _explicitly_ marked with "signed int".
Would it make sense to warn then? Makes sense to me.
And no, none of this is about "strict C standards". All of it is about
"what makes sense". It simply doesn't make sense to complain about the
sign of "char", because it's not something that has a very hard
definition. Similarly, you shouldn't complain about regular "int"
conversions, because they are normal, and the standard defines them, but
maybe you can take a hint when the programmer gives you a hint by doing
something that is "obviously unnecessary", like explicitly saying that
"signed int" thing.
Just an idea.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Mon, 12 Feb 2007 16:27:16 UTC
Message-ID: <fa.b0/AuPSY9jjXpODFE6lHbgcR58c@ifi.uio.no>
On Mon, 12 Feb 2007, Sergei Organov wrote:
>
> Why strlen() should be allowed to be called with an incompatible pointer
> type? My point is that gcc should issue *different warning*, -- the same
> warning it issues here:
I agree that "strlen()" per se isn't different.
The issue is not that the warning isn't "technically correct". IT IS.
Nobody should ever argue that the warning isn't "correct". I hope people
didn't think I argued that.
I've argued that the warning is STUPID. That's a totally different thing.
I can say totally idiotic things in perfectly reasonable English grammar
and spelling. Does that make the things I say "good"? No.
The same is true of this gcc warning. It's technically perfectly
reasonable both in English grammar and spelling (well, as far as
any compiler warning ever is) _and_ in "C grammar and spelling" too.
But being grammatically correct does not make it "smart". IT IS STILL
STUPID.
Can people not see the difference between "grammatically correct" and
"intelligent"? People on the internet seem to have often acquired the
understanding that "bad grammar and spelling" => "stupid", and yes, there
is definitely some kind of correlation there. But as any logician and
matematician hopefully knows, "a => b" does NOT imply "!a => !b".
Some people think that "warnings are always good". HELL NO!
A warning is only as good as
(a) the thing it warns about
(b) the thing you can do about it
And THAT is the fundamental problem with that *idiotic* warning. Yes, it's
technically correct. Yes, it's "proper C grammar". But if you can't get
over the hump of realizing that there is a difference between "grammar"
and "intelligent speech", you shouldn't be doing compilers.
So the warning sucks because:
- the thing it warns about (passing "unsigned char" to something that
doesn't specify a sign at all!) is not something that sounds wrong in
the first place. Yes, it's unsigned. But no, the thing it is passed to
didn't specify that it wanted a "signed" thing in the first place. The
"strlen()" function literally says
"I want a char of indeterminate sign"!
which implies that strlen really doesn't care about the sign. The same
is true of *any* function that takes a "char *". Such a function
doesn't care, and fundamentally CANNOT care about the sign, since it's
not even defined!
So the warning fails the (a) criterion. The warning isn't valid,
because the thing it warns about isn't a valid problem!
- it _also_ fails the (b) criterion, because quite often there is nothing
you can do about it. Yes, you can add a cast, but adding a cast
actually causes _worse_ code (but the warning is certainly gone). But
that makes the _other_ argument for the warning totally point-less: if
the reason for the warning was "bad code", then having the warning is
actively BAD, because the end result is actually "worse code".
See? The second point is why it's important to also realize that there is
a lot of real and valid code that actually _does_ pass "strlen()" an
unsigned string. There are tons of reasons for that to happen: the part of
the program that _does_ care wants to use a "unsigned char" array, because
it ends up doing things like "isspace(array[x])", and that is not
well-defined if you use a "char *" array.
So there are lots of reasons to use "unsigned char" arrays for strings.
Look it up. Look up any half-way reasonable man-page for the "isspace()"
kind of functions, and if they don't actually explicitly say that you
should use unsigned characters for it, those man-pages are crap. Because
those functions really *are* defined in "int", but it's the same kind of
namespace that "getchar()" works in (ie "unsigned char" + EOF, where EOF
_usually_ is -1, although other values are certainly technically legal
too).
So:
- in practice, a lot of "good programming" uses "unsigned char" pointers
for doing strings. There are LOTS of reasons for that, but "isspace()"
and friends is the most obvious one.
- if you can't call "strlen()" on your strings without the compiler
warning, there's two choices: the compiler warning is CRAP, or your
program is bad. But as I just showed you, "unsigned char *" is actually
often the *right* thing to use for string work, so it clearly wasn't
the program that was bad.
So *please* understand:
- yes, the warning is "correct" from a C grammatical standpoint
- the warning is STILL CRAP, because grammar isn't the only thing about a
computer language. Sane usage is MUCH MORE important than any grammar.
Thus ends the sacred teachings of Linus "always right" Torvalds. Go and
ponder these words, and please send me all your money (certified checks
only, please - sending small unmarked bills is against USPS rules) to show
your support of the holy church of good taste.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Tue, 13 Feb 2007 23:22:03 UTC
Message-ID: <fa.hyS7shmJjZzkas+x7uPw2yZlZIk@ifi.uio.no>
On Tue, 13 Feb 2007, Sergei Organov wrote:
>
> Sorry, what do you do with "variable 'xxx' might be used uninitialized"
> warning when it's false? Turn it off? Annotate the source? Assign fake
> initialization value? Change the compiler so that it does "the effort"
> for you? Never encountered false positive from this warning?
The thing is, that warning is at least easy to shut up.
You just add a fake initialization. There isn't any real downside.
In contrast, to shut up the idiotic pointer-sign warning, you have to add
a cast.
Now, some people seem to think that adding casts is a way of life, and
think that C programs always have a lot of casts. That's NOT TRUE. It's
actually possible to avoid casts, and good C practice will do that to
quite a high degree, because casts in C are *dangerous*.
A language that doesn't allow arbitrary type-casting is a horrible
language (because it doesn't allow you to "get out" of the language type
system), and typecasts in C are really important. But they are an
important feature that should be used with caution, and as little as
possible, because they (by design, of course) break the type rules.
Now, since the _only_ reason for the -Wpointer-sign warning in the first
place is to warn about breaking type rules, if the way to shut it up is to
break them EVEN MORE, then the warning is obviously totally broken. IT
CAUSES PEOPLE TO WRITE WORSE CODE! Which was against the whole point of
having the warning in the first place.
This is why certain warnings are fine. For example, the warning about
if (a=b)
..
is obviously warning about totally valid C code, but it's _still_ a fine
warning, because it's actually very easy to make that warning go away AND
IMPROVE THE CODE at the same time. Even if you actually meant to write the
assignment, you can write it as
if ((a = b) != 0)
..
or
a = b;
if (a)
..
both of which are actually more readable.
But if you have
unsigned char *mystring;
len = strlen(mystring);
then please tell me how to fix that warning without making the code
*worse* from a type-safety standpoint? You CANNOT. You'd have to cast it
explicitly (or assing it through a "void *" variable), both of which
actually MAKE THE TYPE-CHECKING PROBLEM WORSE!
See? The warning made no sense to begin with, and it warns about something
where the alternatives are worse than what it warned about.
Ergo. It's a crap warning.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 15 Feb 2007 15:58:00 UTC
Message-ID: <fa./C+CbqoiGqw9ss8aRNmvscNLJv0@ifi.uio.no>
On Thu, 15 Feb 2007, Sergei Organov wrote:
>
> Yes, there is. There are at least 2 drawbacks. Minor one is
> initialization eating cycles. Major one is that if later I change the
> code and there will appear a pass that by mistake uses those fake value,
> I won't get the warning anymore. The latter drawback is somewhat similar
> to the drawbacks of explicit type casts.
I actually agree - it would clearly be *nicer* if gcc was accurate with
the warning. I'm just saying that usually the patch to shut gcc up is
quite benign.
But yes, it basically disabled the warning, and just *maybe* the warning
was valid, and it disabled it by initializing something to the wrong
value.
That's why warnings with false positives are often much worse than not
having the warning at all: you start dismissing them, and not even
bothering to fix them "properly". That's especially true if there _isn't_
a proper fix.
I personally much prefer a compiler that doesn't warn a lot, but *when* it
warns, it's 100% on the money. I think that compilers that warn about
things that "may" be broken are bogus. It needs to be iron-clad, with no
question about whether the warning is appropriate!
Which is, of course, why we then shut up some gcc warnings, and which gets
us back to the start of the discussion.. ;)
> [I'd, personally, try to do code reorganization instead so that it
> becomes obvious for the compiler that the variable can't be used
> uninitialized. Quite often it makes the code better, at least it was my
> experience so far.]
It's true that it sometimes works that way. Not always, though.
The gcc "uninitialized variable" thing is *usually* a good sign that the
code wasn't straightforward for the compiler to follow, and if the
compiler cannot follow it, then probably neither can most programmers. So
together with the fact that it's not at least _syntactically_ ugly to shut
up, it's not a warning I personally object to most of the time (unlike,
say, the pointer-sign one ;)
> [Did I already say that I think it's wrong warning to be given in this
> particular case, as the problem with the code is not in signedness?]
>
> 1. Don't try to hide the warning, at least not immediately, -- consider
> fixing the code first. Ah, sorry, you've already decided for yourself
> that the warning is idiotic, so there is no reason to try to...
This is actually something we've tried.
The problem with that approach is that once you have tons of warnings that
are "expected", suddenly *all* warnings just become noise. Not just the
bogus one. It really *is* a matter of: warnings should either be 100%
solid, or they should not be there at all.
And this is not just about compiler warnings. We've added some warnings of
our own (well, with compiler help, of course): things like the
"deprecated" warnings etc. I have often ended up shutting them up, and one
reason for that is that yes, I have literally added bugs to the kernel
that the compiler really *did* warn about, but because there were so many
other pointless warnings, I didn't notice!
I didn't make that up. I forget what the bug was, but it literally wasn't
more than a couple of months ago that I passed in the wrong type to a
function, and the compiler warned about it in big letters. It was just
totally hidden by all the other warning crap.
> 2. If you add a cast, it's not in contrast, it's rather similar to fake
> initialization above as they have somewhat similar drawback.
I agree that it's "unnecessary code", and in many ways exactly the same
thing. I just happen to believe that casts tend to be a lot more dangerous
than extraneous initializations. Casts have this nasty tendency of hiding
*other* problems too (ie you later change the thing you cast completely,
and now the compiler doesn't warn about a *real* bug, because the cast
will just silently force it to a wrong type).
And yeah, that may well be a personal hangup of mine. A lot of people
think casts in C are normal. Me, I actually consider C to be a type-safe
language (!) but it's only type-safe if you are careful, and avoiding
casts is one of the rules.
Others claim that C isn't type-safe at all, and I think it comes from
them having been involved in projects where people didn't have the same
"casts are good, but only when you use them really really carefully".
> > But if you have
> >
> > unsigned char *mystring;
> >
> > len = strlen(mystring);
> >
> > then please tell me how to fix that warning without making the code
> > *worse* from a type-safety standpoint? You CANNOT. You'd have to cast it
> > explicitly (or assing it through a "void *" variable), both of which
> > actually MAKE THE TYPE-CHECKING PROBLEM WORSE!
>
> Because instead of trying to fix the code, you insist on hiding the
> warning.
No. I'm saying that I have *reasons* that I need my string to be unsigned.
That makes your first "fix" totally bogus:
> There are at least two actual fixes:
>
> 1. Do 'char *mystring' instead, as otherwise compiler can't figure out
> that you are going to use 'mystring' to point to zero-terminated
> array of characters.
And the second fix is a fix, but it is, again, worse than the warning:
> 2. Use "len = ustrlen(mystring)" if you indeed need something like C
> strings
Sure, I can do that, but the upside is what?
Getting rid of a warning that was bogus to begin with?
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 15 Feb 2007 19:03:40 UTC
Message-ID: <fa.V6quRYU3js+PpjJVZTPrsiq7eU4@ifi.uio.no>
On Thu, 15 Feb 2007, Sergei Organov wrote:
>
> I agree that if the warning has no true positives, it sucks. The problem
> is that somehow I doubt it has none. And the reasons for the doubt are:
Why do you harp on "no true positives"?
That's a pointless thing. You can make *any* warning have "true
positives". My personal favorite is the unconditional warning:
warning: user is an idiot
and I _guarantee_ you that it has a lot of true positives.
It's the "no false negatives" angle you should look at.
THAT is what matters. The reason we don't see a lot of warnings about
idiot users is not that people don't do stupid things, but that
*sometimes* they actually don't do something stupid.
Yeah, I know, it's far-fetched, but still.
In other words, you're barking up *exactly* the wrong tree. You're looking
at it the wrong way around.
Think of it this way: in science, a theory is proven to be bad by a single
undeniable fact just showing that it's wrong.
The same is largely true of a warning. If the warning sometimes happens
for code that is perfectly fine, the warning is bad.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: About GCC4 Optimization
Date: Sun, 25 Mar 2007 04:28:38 UTC
Message-ID: <fa.HN/7hEahpxR74lNaqt4THx4pfG0@ifi.uio.no>
On Sun, 25 Mar 2007, yuan cooper wrote:
>
> during my work, I found there is a bug with GCC4 O2 optimization.
Technically, it's a misfeature of gcc4, not a bug.
The C language allows for type-based alias detection, and gcc notices that
a "float *" cannot ever alias with a "unsigned long *", so it decides to
not even do the loads and stores..
Now, there's two things wrong with this picture:
- gcc is being an ass. type-based alias detection should happen only as a
last resort, and gcc should know and notice that *despite* the types
being different, they definitely alias.
So what gcc does may be technically legal, but it's still a horribly
bad thing to do. Sadly, some gcc people seem to care more about "letter
of the law" than "sanity and quality of implementation".
- as a result, you should always compile any kernel stuff with
"-fno-strict-aliasing", which should turn this off. If it *still*
happens with that flag, then it is indeed a compiler bug.
> float ftmp;
> unsigned long tmp;
> ftmp = 1.0/1024.0;
> tmp = *(unsigned long *)(&ftmp);
> tmp = (tmp >> 11) && 0xFFF;
>
> if optimization level is O2, gcc will MOV eax to tmp, but current eax has a random value.
> -O is ok and gcc3 with O2 is ok too.
That said, you really _really_ shouldn't be doing FP in the kernel anyway.
Linus
From: Al Viro <viro@ftp.linux.org.uk>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH 16/16] fix handling of integer constant expressions
Date: Sun, 24 Jun 2007 18:45:09 UTC
Message-ID: <fa.55kJz0prOSHcU8RGEXmJ+Xhm6BA@ifi.uio.no>
On Sun, Jun 24, 2007 at 08:18:52PM +0200, Segher Boessenkool wrote:
> >#define _IOC_TYPECHECK(t) \
> > ((sizeof(t) == sizeof(t[1]) && \
> > sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
> > sizeof(t) : __invalid_size_argument_for_IOC)
> >poisoning _IOW() et.al., so those who do something like
> >
> >static const char *v4l1_ioctls[] = {
> > [_IOC_NR(VIDIOCGCAP)] = "VIDIOCGCAP",
> >
> >run into trouble.
>
> >The only reason that doesn't break gcc to hell and back is
> >that gcc has unfixed bugs in that area.
>
> If I understand correctly what bugs you are talking about,
> most (all?) of those were solved in the dark ages already
> (i.e., the 3.x series).
Alas, no. gcc is amazingly (and inconsistently) sloppy about the
things it accepts as integer constant expressions.
> >It certainly is not a valid C
>
> Why not? Nothing in the C standard says all your externs
> have to be defined in some other translation unit you link
> with AFAIK.
It's not about externs. It's about things like
unsigned n;
int a[] = {[n - n + n - n] = 1};
And yes, gcc does eat that. With -pedantic -std=c99, at that.
However,
unsigned n;
int a[] = {[n + n - n - n] = 1};
gets you error: nonconstant array index in initializer
And that's 4.1, not 3.x...
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: [git patches 1/2] warnings: attack valid cases spotted by
Date: Wed, 18 Jul 2007 02:58:32 UTC
Message-ID: <fa.tkhtwE+e9F03/OMSt0Cgx8TD4+0@ifi.uio.no>
On Tue, 17 Jul 2007, Roland Dreier wrote:
> So setting a variable to something meaningless (guaranteeing that a
> garbage value is used in case of a bug) just to shut up a warning makes
> no sense -- it's no safer than leaving the code as is.
Wrong.
It's safer for two reasons:
- now everybody will see the *same* behaviour
- the "meaningless value" is guaranteed to not be a security leak
but the whole "shut up bogus warnings" is the best reason.
So it *is* safer than leaving the code as-is.
Of course, usually the best approach is to rewrite the code to be simpler,
so that even gcc sees that something is obviously initialized. Sadly,
people seldom do the right thing, and sometimes gcc just blows incredibly
hard.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: [git patches 1/2] warnings: attack valid cases spotted by
Date: Tue, 17 Jul 2007 22:27:09 UTC
Message-ID: <fa.YCEAlOvqvTrzfD3mT0RqlP8HRFk@ifi.uio.no>
On Tue, 17 Jul 2007, Andrew Morton wrote:
> (rofl, look at that mess: it was utterly impractical, unrealistic and
> stupid for gcc to go and UTFify the compiler output. Sigh. LANG=C, guys).
Yeah, I absolutely *detest* how gcc does idiotic quoting just because you
happen to be in a UTF-8 locale. There's no reason for it what-so-ever, and
I don't understand the mindset of whoever did that crap.
They should use a nice ASCII tick-mark ('), not some fancy quotation
marks.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH] make atomic_t volatile on all architectures
Date: Sun, 12 Aug 2007 06:11:47 UTC
Message-ID: <fa.D4/HwdurI9XNIUeAbUpYHZias+s@ifi.uio.no>
On Sun, 12 Aug 2007, Segher Boessenkool wrote:
>
> Note that last line.
Segher, how about you just accept that Linux uses gcc as per reality, and
that sometimes the reality is different from your expectations?
"+m" works. We use it. It's better than the alternatives. Pointing to
stale documentation doesn't change anything.
The same is true of accesses through a volatile pointer. The kernel has
done that for a *loong* time (all MMIO IO is done that way), and it's
totally pointless to say that "volatile" isn't guaranteed to do what it
does. It works, and quite frankly, if it didn't work, it would be a gcc
BUG.
And again, this is not a "C standard" issue. It's a "sane implementation"
issue. Linux expects the compiler to be sane. If it isn't, that's not our
problem. gcc *is* sane, and I don't see why you constantly act as if it
wasn't.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Thu, 25 Oct 2007 23:33:37 UTC
Message-ID: <fa.KQvOr/FGHynbgGXSB9HW4X+JB6I@ifi.uio.no>
On Fri, 26 Oct 2007, Andi Kleen wrote:
>
> No it can't (at least not on x86) as I have explained in the rest of the mail
> you conveniently snipped.
I "conveniently snipped it" because it was pointless.
"adc" or "cmov" has nothing what-so-ever to do with it. If some routine
returns 0-vs-1 and gcc then turns "if (routine()) x++" into
"x+=routine()", what does that have to do with adc or cmov?
The fact is, these kinds of optimizations are *bogus* and they are
dangerous.
Now, it's equally true that we probably don't have those kinds of patterns
in the kernel, and we'll probably not hit it, but wouldn't it be much
better to make sure that compilers shouldn't do that?
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Thu, 25 Oct 2007 23:58:08 UTC
Message-ID: <fa./PjdIECW7NH9tvfZFBS9vQifre0@ifi.uio.no>
On Fri, 26 Oct 2007, Andi Kleen wrote:
>
> The conditional add/sub using carry trick is not generally bogus.
> But for registers it's a fine optimization.
For registers it's fine. For memory, it's a disaster. It's more than just
dirty cachelines and introducing race conditions, it's also about
protection and dirty pages.
So even in user space, to even be correct in the first place, the compiler
would need to make sure that the variable is writable at all (or you might
take a SIGSEGV), but I guess that gcc just assumes it is, at least for
globals (or gcc could depend on seeing *other* writes that are done
unconditionally).
More likely, the compiler people don't even care, because "the C standard
doesn't specify that behaviour" - ie things like write-protected memory or
garbage collection based on dirty/accessed bits are outside the scope of
what the language specifies. Much less things like pthreads or other
synchronization primitives in threads.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Thu, 25 Oct 2007 14:56:48 UTC
Message-ID: <fa.ZrwwpON7aSAL6jPUXYb1t0BQY6c@ifi.uio.no>
On Thu, 25 Oct 2007, Nick Piggin wrote:
>
> Andi spotted this exchange on the gcc list. I don't think he's
> brought it up here yet, but it worries me enough that I'd like
> to discuss it.
Are you surprised?
The gcc developers seem to have had a total disregard for what people
want or need, and every time some code generation issue comes up, there's
a lot of people on the list that do language-lawyering, rather than admit
that there might be a problem.
It's happened before, it will happen again. I don't think it's true of all
gcc developers (or even most, I hope), but it's common enough. For some
reason, compiler developers seem to be far enough removed from "real life"
that they have a tendency to talk in terms of "this is what the spec says"
rather than "this is a problem".
Happily, at least in this kind of situation, threading is a real issue for
other projects than just the kernel, so maybe it gets solved properly.
But I have to admit that for the last five years or so, I've really wanted
some other compiler team to come up with a good open-source compiler.
Exactly due to issues like this (Q: "Gcc creates bogus code that doesn't
work!" A: "It's not bogus, it's technically allowed by the language specs
that don't talk about xyz, the fact that it doesn't work isn't our
problem").
I think the OpenBSD people decided to actually do something about this,
and I suspect it had *nothing* to do with license issues, and everything
to do with these kinds of problems. I wish them all the luck, although
personally I think LLVM is a much more interesting project.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Fri, 26 Oct 2007 15:09:44 UTC
Message-ID: <fa.jPlqUnR/e7PS8SPHgJQdZO5+hTg@ifi.uio.no>
On Fri, 26 Oct 2007, Bart Van Assche wrote:
> On 10/25/07, Linus Torvalds <> wrote:
> >
> > The gcc developers seem to have had a total disregard for what people
> > want or need, and every time some code generation issue comes up, there's
> > a lot of people on the list that do language-lawyering, rather than admit
> > that there might be a problem.
>
> Please make a proposal for how gcc should be modified instead of just
> shooting on the gcc people -- the root cause here is the way the C/C++
> memory model is defined. (Note: I'm not in any way involved in gcc
> development.)
Note that I'm very ambivalent about gcc. I think it's a *great* compiler.
I have my doubts about some of the things it does, but hey, it is an old
project, it has accumulated cruft over time, and cleaning things up is
often almost impossible.
And bugs happen. I'm not complaining about that at all either, even if
sometimes a compiler bug is damn frustrating.
And the fact is, most of the gcc developers are great.
So my basic worry about gcc is in fact none of the individual technical
problems themselves - those can be fixed. No, the problem I've seen in gcc
is that _some_ of the developers seem to be total d*ckheads when it comes
to "language definition", and seem to think that it's more important to
read the language spec like a lawyer than it is to solve actual user
problems.
And that has come up before. It has nothing to do with this particular
"gcc doesn't create thread-safe code" issue. We had the exact same issue
with gcc creating totally unusable code due to having a fundamentally
braindead memory aliasing model. The language-lawyering people basically
could push their *idiotic* model onto gcc, just by quoting the standard,
and not caring about actual users at all.
And that's a problem. In the kernel, we've historically always cared a lot
about POSIX and SuS, but while conforming to standards have been primary
goals since pretty much day one (ie I asked around about POSIX before the
very first release, and it's how I met some suckers^Wupstanding citizens
to try those early kernels), it has *always* taken a back seat to things
like compatibility with existing apps.
The gcc lists seem to often get to the point where people quote the
standard, and that's that. In that environment, the paper standard (that
hass *nothing* to do with reality) trumps any other argument. "What we do
is _allowed_ by the standard" seems to be a good argument, even if it
breaks real code and there is no sane way to avoid doing it.
And I really don't think it's everybody. At all. But I think it's the case
that sometimes it's easier to quote the standard than write good code, and
so gcc lists have more people quoting the papers than trying to fix some
problem.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Fri, 26 Oct 2007 22:25:24 UTC
Message-ID: <fa.Y3WwI6Dccocs5a7KRYgXCffe3T8@ifi.uio.no>
On Fri, 26 Oct 2007, Giacomo Catenazzi wrote:
>
> So we have the great opportunity to change the standard, then
> gcc will change ;-)
I see the smiley, but sadly, new standards take ten years or more to
mature. Which means that even if the upcoming one is "perfect", things
will be wrong with it, if only because people will have new usage
scenarios where the standard simply isn't relevant or that it otherwise
just doesn't address, and that then gets us back to the same issues
somewhere else.
So it would be much better if developers just didn't think the standard
trumped "real and existing code and problems", and shot down the language
lawyers (and don't get me wrong - it's not just in gcc, btw. We _have_ had
some of the same behavior in the kernel, although I will argue that our
"backwards compatibility trumps pretty much everything else" rules at
least solves _some_ of the problems).
Standards are just papers. Yes, they're important, but they are definitely
not more important than anything else, and they are a lot _less_ important
than some people seem to think. Gcc has done more for programming by being
a de-facto standard and widely available, than the _paper_ standards often
ever do!
It's also sad that a lot of these things seem to be done in the name of
optimizing code, and then in many cases it drives people *away* from using
that optimizer for anything but benchmarking.
In the kernel, we historically used to try for extreme optimizations,
these days we spend more time tuning the optimizations _down_ because they
aren't optimizations at all (ie using -Os instead of -O2), or they were
buggy enough that we have to explicitly disable them (aliasing,
"unit-at-a-time" etc).
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Fri, 26 Oct 2007 16:30:21 UTC
Message-ID: <fa.k4lMmch1x0DcfRWoZSYiHPrhvJM@ifi.uio.no>
On Fri, 26 Oct 2007, Linus Torvalds wrote:
>
>
> On Fri, 26 Oct 2007, Bart Van Assche wrote:
> >
> > You can find my proposal to improve gcc here:
> > http://gcc.gnu.org/ml/gcc/2007-10/msg00465.html
>
> Btw, I think this is fine per se, but putting "__attribute__((acquire))"
> on the functions that acquire a lock does seem to be problematic, in that
> quite often you might well want to inline those things. How would you
> handle that?
Thinking some more about this, you really have two cases:
- full functions taking/releasing locks (possibly conditionally, ie
with something like trylock and/or based on argument values).
You simply *cannot* require these to be marked, because the locking may
have been done indirectly. Yes, you can mark things like
"pthread_mutex_trylock()" as being an acquire-function, but the fact
is, users will then wrap these things in *other* functions, and return
their return values.
Ergo: a compiler *must* assume that a function call that it
didn't inline involves locking. There's no point in adding some
gcc-specific attributes to system header files, because it's not going
to fix anything in any portable program.
- inline assembly (together with, potentially, compiler primitives).
That's the only other way to reliably do locking from C.
This one gcc could certainly extend on. But would there really be any
upside? It would be easier/better to say that inline assembly (at least
if it clobbers memory or is volatile) has the same serialization issues
as a function call.
So the fact is, any compiler that turns
if (conditional)
x++;
into an unconditional write to x (where 'x' is potentially visible to the
outside - global visibility or has had its address taken) is just broken.
No ifs, buts or maybes about it. You simply cannot do that optimization,
because there is no way for the compiler to know whether the conditional
implies that you hold a lock or not.
Linus
From: Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups: fa.linux.kernel
Subject: Re: Is gcc thread-unsafe?
Date: Fri, 26 Oct 2007 17:26:26 UTC
Message-ID: <fa.QOxdywPqXQZhYfjoZmci7Y/EX5I@ifi.uio.no>
On Fri, 26 Oct 2007, Andrew Haley wrote:
>
> Bart Van Assche writes:
>
> > Andrew, do you know whether gcc currently contains any optimization
> > that interchanges the order of accesses to non-volatile variables
> > and function calls ?
>
> It sure does.
Note that doing so is perfectly fine.
But only for local variables that haven't had their addresses taken.
The fact is, those kinds of variables really *are* special. They are
provably not accessible from any other context, and re-ordering them (or
doing anything AT ALL to them - the most basic and very important
optimization is caching them in registers, of course) is always purely an
internal compiler issue.
But if gcc re-orders functions calls with *other* memory accesses, gcc is
totally broken. I doubt it does that. It would break on all but the most
trivial programs, and it would be a clear violation of even standard C.
HOWEVER: the bug that started this thread isn't even "reordering
accesses", it's *adding* accesses that weren't there (and please don't mix
this up with "volatile", since volatile is a totally unrelated issue and
has nothing what-so-ever to do with anything).
Linus
From: "H. Peter Anvin" <hpa@zytor.com>
Newsgroups: fa.linux.kernel
Subject: Re: [PATCH] x86: do not expose CONFIG_BSWAP to userspace
Date: Wed, 28 Jan 2009 22:16:45 UTC
Message-ID: <fa.g0xAldV+LxJRgkaDmnri7UwIha0@ifi.uio.no>
Harvey Harrison wrote:
> I'm afraid my knowledge of gcc compiler flags for various models is
> lacking, I used i486 as suggested, just wanted to make sure I understood
> you corectly.
You did, but I misremembered... instead of having the __i386__,
__i486__, __i586__, __i686__ being an additive chain as would make
sense, gcc just has __i386__ plus whichever corresponds to the -march=
option. I keep forgetting this because it's just so incredibly dumb.
Bloody hell. This really f*cks thing up.
What's worse, they seem to simply be adding new options, so at this
point you'd actually need something like:
# if defined(__i486__) || defined(__i586__) || defined(__i686__) || \
defined(__core2__) || defined(__k8__) || defined(__amdfam10__)
Worse, there isn't any kind of macro that can be used to compare for a
negative (i.e. not i386).
This obviously is screaming to be abstracted away into a header of its
own, but it really can't be done cleanly as far as I can tell because of
this particular piece of major gcc braindamage.
So, one ends up doing something like:
#ifdef __i486__
# define __CPU_HAVE_BSWAP
#endif
#ifdef __i586__
# define __CPU_HAVE_BSWAP
#endif
.. and so on, and have to keep this up to date with the latest
inventions of the gcc people. *Sob.*
-hpa
Index
Home
About
Blog