Invalid compilation without -fno-strict-aliasing

Invalid compilation without -fno-strict-aliasing

[Jean Tourrilhes]

	Hi,

	It looks like a compiler bug to me...
	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).
	Code (from linux/include/net/iw_handler.h) :
--------------------------------------------
static inline char *
iwe_stream_add_event(char *	stream,		/* Stream of events */
		     char *	ends,		/* End of stream */
		     struct iw_event *iwe,	/* Payload */
		     int	event_len)	/* Real size of payload */
{
	/* Check if it's possible */
	if((stream + event_len) < ends) {
		iwe->len = event_len;
		memcpy(stream, (char *) iwe, event_len);
		stream += event_len;
	}
	return stream;
}
--------------------------------------------
	IMHO, the compiler should have enough context to know that the
reordering is dangerous. Any suggestion to make this simple code more
bullet proof is welcomed.

	Have fun...

	Jean

Re: Invalid compilation without -fno-strict-aliasing

[Horst von Brand]

Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> 	It looks like a compiler bug to me...
> 	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).
> 	Code (from linux/include/net/iw_handler.h) :
> --------------------------------------------
> static inline char *
> iwe_stream_add_event(char *	stream,		/* Stream of events */
> 		     char *	ends,		/* End of stream */
> 		     struct iw_event *iwe,	/* Payload */
> 		     int	event_len)	/* Real size of payload */
> {
> 	/* Check if it's possible */
> 	if((stream + event_len) < ends) {
> 		iwe->len = event_len;
> 		memcpy(stream, (char *) iwe, event_len);
> 		stream += event_len;
> 	}
> 	return stream;
> }
> --------------------------------------------
> 	IMHO, the compiler should have enough context to know that the
> reordering is dangerous. Any suggestion to make this simple code more
> bullet proof is welcomed.

The compiler is free to assume char *stream and struct iw_event *iwe point
to separate areas of memory, due to strict aliasing.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: Invalid compilation without -fno-strict-aliasing

[Falk Hueffner]

Horst von Brand <vonbrand@inf.utfsm.cl> writes:

> Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > 	if((stream + event_len) < ends) {
> > 		iwe->len = event_len;
> > 		memcpy(stream, (char *) iwe, event_len);
> > 		stream += event_len;
> > 	}
> > 	return stream;
> > }
> 
> The compiler is free to assume char *stream and struct iw_event *iwe
> point to separate areas of memory, due to strict aliasing.

The relevant paragraph of the C99 standard is:

An object shall have its stored value accessed only by an lvalue
expression that has one of the following types:

-- a type compatible with the effective type of the object,
-- a qualified version of a type compatible with the effective type of
   the object,
-- a type that is the signed or unsigned type corresponding to the
   effective type of the object,
-- a type that is the signed or unsigned type corresponding to a
   qualified version of the effective type of the object,
-- an aggregate or union type that includes one of the aforementioned
   types among its members (including, recursively, a member of a
   subaggregate or contained union), or
-- a character type.

I can't really spot any lvalue here that might violate this rule.  It
would be nice if somebody could report a bug with a testcase.

-- 
	Falk

Re: Invalid compilation without -fno-strict-aliasing

[Horst von Brand]

Falk Hueffner <falk.hueffner@student.uni-tuebingen.de> said:
> Horst von Brand <vonbrand@inf.utfsm.cl> writes:
> > Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > > 	if((stream + event_len) < ends) {
> > > 		iwe->len = event_len;
> > > 		memcpy(stream, (char *) iwe, event_len);
> > > 		stream += event_len;
> > > 	}
> > > 	return stream;
> > > }
> > 
> > The compiler is free to assume char *stream and struct iw_event *iwe
> > point to separate areas of memory, due to strict aliasing.
> 
> The relevant paragraph of the C99 standard is:
> 
> An object shall have its stored value accessed only by an lvalue
> expression that has one of the following types:
[...]
> -- a character type.

(char *) gives you a (pointer to) a character type.

> I can't really spot any lvalue here that might violate this rule.  It
> would be nice if somebody could report a bug with a testcase.

stream and (char *) iwe
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: Invalid compilation without -fno-strict-aliasing

[Daniel Jacobowitz]

On Wed, Feb 26, 2003 at 05:47:48PM -0300, Horst von Brand wrote:
> Falk Hueffner <falk.hueffner@student.uni-tuebingen.de> said:
> > Horst von Brand <vonbrand@inf.utfsm.cl> writes:
> > > Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > > > 	if((stream + event_len) < ends) {
> > > > 		iwe->len = event_len;
> > > > 		memcpy(stream, (char *) iwe, event_len);
> > > > 		stream += event_len;
> > > > 	}
> > > > 	return stream;
> > > > }
> > > 
> > > The compiler is free to assume char *stream and struct iw_event *iwe
> > > point to separate areas of memory, due to strict aliasing.
> > 
> > The relevant paragraph of the C99 standard is:
> > 
> > An object shall have its stored value accessed only by an lvalue
> > expression that has one of the following types:
> [...]
> > -- a character type.
> 
> (char *) gives you a (pointer to) a character type.
> 
> > I can't really spot any lvalue here that might violate this rule.  It
> > would be nice if somebody could report a bug with a testcase.
> 
> stream and (char *) iwe

Stream is not the same storage as iwe, so this is hardly the issue. 
Writes to stream don't affect iwe.  The problem was the assignment to
iwe->len being moved after the access, according to the report.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Invalid compilation without -fno-strict-aliasing

[Jakub Jelinek]

On Wed, Feb 26, 2003 at 03:57:54PM -0500, Daniel Jacobowitz wrote:
> > > > > 	if((stream + event_len) < ends) {
> > > > > 		iwe->len = event_len;
> > > > > 		memcpy(stream, (char *) iwe, event_len);
> > > > > 		stream += event_len;
> > > > > 	}
> > > > > 	return stream;

> > > > The compiler is free to assume char *stream and struct iw_event *iwe
> > > > point to separate areas of memory, due to strict aliasing.

No.

> Stream is not the same storage as iwe, so this is hardly the issue. 
> Writes to stream don't affect iwe.  The problem was the assignment to
> iwe->len being moved after the access, according to the report.

GCC can do that with -fstrict-aliasing:
All calls to this inline function are done with constant event_len.
E.g. in case of event_len = IW_EV_UINT_LEN, memcpy macro expands to
__constant_memcpy which does:
                        *(unsigned long *)to = *(const unsigned long *)from;
                        *(1+(unsigned long *)to) = *(1+(const unsigned long *)from);
iwe->len is unsigned short and the code writes the value as
unsigned short (iwe->len = 8) and then reads the same memory as unsigned
long (something = *(const unsigned long *)(char *)iwe). But those two types
cannot alias and thus gcc can reorder them.

To fix that, __constant_memcpy would have to access the data through
union, ir you could as well forget about __constant_memcpy and use
__builtin_memcpy where gcc will take care about the constant copying.

	Jakub

Re: Invalid compilation without -fno-strict-aliasing

[Linus Torvalds]

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

Re: Invalid compilation without -fno-strict-aliasing

[Daniel Jacobowitz]

On Thu, Feb 27, 2003 at 07:30:03PM +0000, Linus Torvalds wrote:
> 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.

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'
     Accesses to objects with types with this attribute are not
     subjected to type-based alias analysis, but are instead assumed to
     be able to alias any other type of objects, just like the `char'
     type.  See `-fstrict-aliasing' for more information on aliasing
     issues.

     Example of use:

          typedef short __attribute__((__may_alias__)) short_a;

          int
          main (void)
          { 
            int a = 0x12345678;
            short_a *b = (short_a *) &a;

            b[1] = 0;

            if (a == 0x12345678)
              abort();

            exit(0);
          }

     If you replaced `short_a' with `short' in the variable
     declaration, the above program would abort when compiled with
     `-fstrict-aliasing', which is on by default at `-O2' or above in
     recent GCC versions.


So you define a typedef for unsigned long which has the __may_alias__
attribute, and you go to town writing memcpy inline with that type
instead of a normal unsigned long.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Invalid compilation without -fno-strict-aliasing

[Linus Torvalds]

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

Re: Invalid compilation without -fno-strict-aliasing

[Daniel Jacobowitz]

On Thu, Feb 27, 2003 at 12:00:46PM -0800, Linus Torvalds wrote:
> 
> 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.

We could work around both of these: disable the sign compare warning,
and use check_gcc to set a high number for -finline-limit...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Invalid compilation without -fno-strict-aliasing

[Linus Torvalds]

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

Re: Invalid compilation without -fno-strict-aliasing

[H. Peter Anvin]

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

Re: Invalid compilation without -fno-strict-aliasing

[Anton Blanchard]

 
> 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).

Yep, the instruction count used for inlining seems to be calculated too
early to be useful. Things like sigorsets look huge until all the
redundant cases get optimised away.

Also in gcc 3.2 -Winline was broken, I really hope that gets fixed. We
need it to tune -finline-limit and to catch all those stupidly large
inline functions.

Anton

Re: Invalid compilation without -fno-strict-aliasing

[Jean Tourrilhes]

On Wed, Feb 26, 2003 at 04:38:10PM +0100, Horst von Brand wrote:
> Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > 	It looks like a compiler bug to me...
> > 	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).
> > 	Code (from linux/include/net/iw_handler.h) :
> > --------------------------------------------
> > static inline char *
> > iwe_stream_add_event(char *	stream,		/* Stream of events */
> > 		     char *	ends,		/* End of stream */
> > 		     struct iw_event *iwe,	/* Payload */
> > 		     int	event_len)	/* Real size of payload */
> > {
> > 	/* Check if it's possible */
> > 	if((stream + event_len) < ends) {
> > 		iwe->len = event_len;
> > 		memcpy(stream, (char *) iwe, event_len);
> > 		stream += event_len;
> > 	}
> > 	return stream;
> > }
> > --------------------------------------------
> > 	IMHO, the compiler should have enough context to know that the
> > reordering is dangerous. Any suggestion to make this simple code more
> > bullet proof is welcomed.
> 
> The compiler is free to assume char *stream and struct iw_event *iwe point
> to separate areas of memory, due to strict aliasing.

	Which is true and which is not the problem I'm complaining about.
	Have fun...

	Jean

Re: Invalid compilation without -fno-strict-aliasing

[Horst von Brand]

Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> On Wed, Feb 26, 2003 at 04:38:10PM +0100, Horst von Brand wrote:
> > Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > > 	It looks like a compiler bug to me...
> > > 	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).
> > > 	Code (from linux/include/net/iw_handler.h) :
> > > --------------------------------------------
> > > static inline char *
> > > iwe_stream_add_event(char *	stream,		/* Stream of events */
> > > 		     char *	ends,		/* End of stream */
> > > 		     struct iw_event *iwe,	/* Payload */
> > > 		     int	event_len)	/* Real size of payload */
> > > {
> > > 	/* Check if it's possible */
> > > 	if((stream + event_len) < ends) {
> > > 		iwe->len = event_len;
> > > 		memcpy(stream, (char *) iwe, event_len);
> > > 		stream += event_len;
> > > 	}
> > > 	return stream;
> > > }
> > > --------------------------------------------
> > > 	IMHO, the compiler should have enough context to know that the
> > > reordering is dangerous. Any suggestion to make this simple code more
> > > bullet proof is welcomed.
> > 
> > The compiler is free to assume char *stream and struct iw_event *iwe point
> > to separate areas of memory, due to strict aliasing.
> 
> 	Which is true and which is not the problem I'm complaining about.

... the compiler thus goes and reorders the frobbing of the variables, as
they are (assumed) separate.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: Invalid compilation without -fno-strict-aliasing

[Daniel Phillips]

On Wednesday 26 February 2003 22:07, Horst von Brand wrote:
> Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > On Wed, Feb 26, 2003 at 04:38:10PM +0100, Horst von Brand wrote:
> > > Jean Tourrilhes <jt@bougret.hpl.hp.com> said:
> > > > 	It looks like a compiler bug to me...
> > > > 	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).
> > > > 	Code (from linux/include/net/iw_handler.h) :
> > > > --------------------------------------------
> > > > static inline char *
> > > > iwe_stream_add_event(char *	stream,		/* Stream of events */
> > > > 		     char *	ends,		/* End of stream */
> > > > 		     struct iw_event *iwe,	/* Payload */
> > > > 		     int	event_len)	/* Real size of payload */
> > > > {
> > > > 	/* Check if it's possible */
> > > > 	if((stream + event_len) < ends) {
> > > > 		iwe->len = event_len;
> > > > 		memcpy(stream, (char *) iwe, event_len);
> > > > 		stream += event_len;
> > > > 	}
> > > > 	return stream;
> > > > }
> > > > --------------------------------------------
> > > > 	IMHO, the compiler should have enough context to know that the
> > > > reordering is dangerous. Any suggestion to make this simple code more
> > > > bullet proof is welcomed.
> > >
> > > The compiler is free to assume char *stream and struct iw_event *iwe
> > > point to separate areas of memory, due to strict aliasing.
> >
> > 	Which is true and which is not the problem I'm complaining about.
>
> ... the compiler thus goes and reorders the frobbing of the variables, as
> they are (assumed) separate.

Actually, the compiler appears to view &iwe->len and (char *) iwe as 
separate, which I find surprising.

Regards,

Daniel

Re: Invalid compilation without -fno-strict-aliasing

[Linus Torvalds]

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

Re: Invalid compilation without -fno-strict-aliasing

[Albert Cahalan]

Jean Tourrilhes writes:

> It looks like a compiler bug to me...
> 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).
> Code (from linux/include/net/iw_handler.h) :
> --------------------------------------------
> static inline char *
> iwe_stream_add_event(char * stream,  /* Stream of events */
>        char * ends,  /* End of stream */
>        struct iw_event *iwe, /* Payload */
>        int event_len) /* Real size of payload */
> {
>  /* Check if it's possible */
>  if((stream + event_len) < ends) {
>   iwe->len = event_len;
>   memcpy(stream, (char *) iwe, event_len);
>   stream += event_len;
>  } return stream;
> }
> --------------------------------------------
> IMHO, the compiler should have enough context to
> know that the reordering is dangerous. Any suggestion
> to make this simple code more bullet proof is welcomed.
>
> Have fun...

Since (char*) is special, I agree that it's a bug.
In any case, a warning sure would be nice!

Now for the fun. Pass iwe->len into this
macro before the memcpy, and all should be well.

#define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))

Like this:

   iwe->len = event_len;
   FORCE_TO_MEM(iwe->len);
   memcpy(stream, (char *) iwe, event_len);

Re: Invalid compilation without -fno-strict-aliasing

[Jean Tourrilhes]

On Tue, Feb 25, 2003 at 11:33:09PM -0500, Albert Cahalan wrote:
> Jean Tourrilhes writes:
> 
> > It looks like a compiler bug to me...
> > 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).
> > Code (from linux/include/net/iw_handler.h) :
> > --------------------------------------------
> > static inline char *
> > iwe_stream_add_event(char * stream,  /* Stream of events */
> >        char * ends,  /* End of stream */
> >        struct iw_event *iwe, /* Payload */
> >        int event_len) /* Real size of payload */
> > {
> >  /* Check if it's possible */
> >  if((stream + event_len) < ends) {
> >   iwe->len = event_len;
> >   memcpy(stream, (char *) iwe, event_len);
> >   stream += event_len;
> >  } return stream;
> > }
> > --------------------------------------------
> > IMHO, the compiler should have enough context to
> > know that the reordering is dangerous. Any suggestion
> > to make this simple code more bullet proof is welcomed.
> >
> > Have fun...
> 
> Since (char*) is special, I agree that it's a bug.
> In any case, a warning sure would be nice!
> 
> Now for the fun. Pass iwe->len into this
> macro before the memcpy, and all should be well.
> 
> #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> 
> Like this:
> 
>    iwe->len = event_len;
>    FORCE_TO_MEM(iwe->len);
>    memcpy(stream, (char *) iwe, event_len);

	I'll try that, that sounds absolutely clever (but I only
understand half of it).
	Thanks a lot !

	Jean

Re: Invalid compilation without -fno-strict-aliasing

[Richard B. Johnson]

On Wed, 26 Feb 2003, Jean Tourrilhes wrote:

> On Tue, Feb 25, 2003 at 11:33:09PM -0500, Albert Cahalan wrote:
> > Jean Tourrilhes writes:
> > 
> > > It looks like a compiler bug to me...
> > > 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).
> > > Code (from linux/include/net/iw_handler.h) :
> > > --------------------------------------------
> > > static inline char *
> > > iwe_stream_add_event(char * stream,  /* Stream of events */
> > >        char * ends,  /* End of stream */
> > >        struct iw_event *iwe, /* Payload */
> > >        int event_len) /* Real size of payload */
> > > {
> > >  /* Check if it's possible */
> > >  if((stream + event_len) < ends) {
> > >   iwe->len = event_len;
> > >   memcpy(stream, (char *) iwe, event_len);
> > >   stream += event_len;
> > >  } return stream;
> > > }
> > > --------------------------------------------
> > > IMHO, the compiler should have enough context to
> > > know that the reordering is dangerous. Any suggestion
> > > to make this simple code more bullet proof is welcomed.
> > >
> > > Have fun...
> > 
> > Since (char*) is special, I agree that it's a bug.
> > In any case, a warning sure would be nice!
> > 
> > Now for the fun. Pass iwe->len into this
> > macro before the memcpy, and all should be well.
> > 
> > #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> > 
> > Like this:
> > 
> >    iwe->len = event_len;
> >    FORCE_TO_MEM(iwe->len);
> >    memcpy(stream, (char *) iwe, event_len);
> 
> 	I'll try that, that sounds absolutely clever (but I only
> understand half of it).
> 	Thanks a lot !
> 
> 	Jean
> -

This does absoultely nothing with egcs-2.91.66. I also modified

 #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
                                           |________ this to "memory"

and it still does nothing. The result of gcc -O2 -S -o xxx xxx.c just
shows:

#AP
#NOAP

With no code in-between.

I also changed it to:
 #define FORCE_TO_MEM(x) __asm__ __volatile__(""::"r"(&(x)))
to no avail.

What's up?


Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: Invalid compilation without -fno-strict-aliasing

[Daniel Jacobowitz]

On Wed, Feb 26, 2003 at 01:23:19PM -0500, Richard B. Johnson wrote:
> On Wed, 26 Feb 2003, Jean Tourrilhes wrote:
> 
> > On Tue, Feb 25, 2003 at 11:33:09PM -0500, Albert Cahalan wrote:
> > > Jean Tourrilhes writes:
> > > 
> > > > It looks like a compiler bug to me...
> > > > 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).
> > > > Code (from linux/include/net/iw_handler.h) :
> > > > --------------------------------------------
> > > > static inline char *
> > > > iwe_stream_add_event(char * stream,  /* Stream of events */
> > > >        char * ends,  /* End of stream */
> > > >        struct iw_event *iwe, /* Payload */
> > > >        int event_len) /* Real size of payload */
> > > > {
> > > >  /* Check if it's possible */
> > > >  if((stream + event_len) < ends) {
> > > >   iwe->len = event_len;
> > > >   memcpy(stream, (char *) iwe, event_len);
> > > >   stream += event_len;
> > > >  } return stream;
> > > > }
> > > > --------------------------------------------
> > > > IMHO, the compiler should have enough context to
> > > > know that the reordering is dangerous. Any suggestion
> > > > to make this simple code more bullet proof is welcomed.
> > > >
> > > > Have fun...
> > > 
> > > Since (char*) is special, I agree that it's a bug.
> > > In any case, a warning sure would be nice!
> > > 
> > > Now for the fun. Pass iwe->len into this
> > > macro before the memcpy, and all should be well.
> > > 
> > > #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> > > 
> > > Like this:
> > > 
> > >    iwe->len = event_len;
> > >    FORCE_TO_MEM(iwe->len);
> > >    memcpy(stream, (char *) iwe, event_len);
> > 
> > 	I'll try that, that sounds absolutely clever (but I only
> > understand half of it).
> > 	Thanks a lot !
> > 
> > 	Jean
> > -
> 
> This does absoultely nothing with egcs-2.91.66. I also modified
> 
>  #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
>                                            |________ this to "memory"
> 
> and it still does nothing. The result of gcc -O2 -S -o xxx xxx.c just
> shows:
> 
> #AP
> #NOAP
> 
> With no code in-between.
> 
> I also changed it to:
>  #define FORCE_TO_MEM(x) __asm__ __volatile__(""::"r"(&(x)))
> to no avail.
> 
> What's up?

That's "working".  Does it prevent the reordering?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Invalid compilation without -fno-strict-aliasing

[Richard B. Johnson]

On Wed, 26 Feb 2003, Daniel Jacobowitz wrote:

> On Wed, Feb 26, 2003 at 01:23:19PM -0500, Richard B. Johnson wrote:
> > On Wed, 26 Feb 2003, Jean Tourrilhes wrote:
> > 
> > > On Tue, Feb 25, 2003 at 11:33:09PM -0500, Albert Cahalan wrote:
> > > > Jean Tourrilhes writes:
> > > > 
> > > > > It looks like a compiler bug to me...
> > > > > 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).
> > > > > Code (from linux/include/net/iw_handler.h) :
> > > > > --------------------------------------------
> > > > > static inline char *
> > > > > iwe_stream_add_event(char * stream,  /* Stream of events */
> > > > >        char * ends,  /* End of stream */
> > > > >        struct iw_event *iwe, /* Payload */
> > > > >        int event_len) /* Real size of payload */
> > > > > {
> > > > >  /* Check if it's possible */
> > > > >  if((stream + event_len) < ends) {
> > > > >   iwe->len = event_len;
> > > > >   memcpy(stream, (char *) iwe, event_len);
> > > > >   stream += event_len;
> > > > >  } return stream;
> > > > > }
> > > > > --------------------------------------------
> > > > > IMHO, the compiler should have enough context to
> > > > > know that the reordering is dangerous. Any suggestion
> > > > > to make this simple code more bullet proof is welcomed.
> > > > >
> > > > > Have fun...
> > > > 
> > > > Since (char*) is special, I agree that it's a bug.
> > > > In any case, a warning sure would be nice!
> > > > 
> > > > Now for the fun. Pass iwe->len into this
> > > > macro before the memcpy, and all should be well.
> > > > 
> > > > #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> > > > 
> > > > Like this:
> > > > 
> > > >    iwe->len = event_len;
> > > >    FORCE_TO_MEM(iwe->len);
> > > >    memcpy(stream, (char *) iwe, event_len);
> > > 
> > > 	I'll try that, that sounds absolutely clever (but I only
> > > understand half of it).
> > > 	Thanks a lot !
> > > 
> > > 	Jean
> > > -
> > 
> > This does absoultely nothing with egcs-2.91.66. I also modified
> > 
> >  #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> >                                            |________ this to "memory"
> > 
> > and it still does nothing. The result of gcc -O2 -S -o xxx xxx.c just
> > shows:
> > 
> > #AP
> > #NOAP
> > 
> > With no code in-between.
> > 
> > I also changed it to:
> >  #define FORCE_TO_MEM(x) __asm__ __volatile__(""::"r"(&(x)))
> > to no avail.
> > 
> > What's up?
> 
> That's "working".  Does it prevent the reordering?

It was supposed to force x, which may be cached in a register,
to be written to memory __now__. It doesn't seem to do anything.
I think FORCE_TO_MEM() needs to claim that it uses most all the
registers. That will make sure that any register values get
written to their final memory locations.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: Invalid compilation without -fno-strict-aliasing

[Daniel Jacobowitz]

On Wed, Feb 26, 2003 at 02:40:27PM -0500, Richard B. Johnson wrote:
> On Wed, 26 Feb 2003, Daniel Jacobowitz wrote:
> 
> > On Wed, Feb 26, 2003 at 01:23:19PM -0500, Richard B. Johnson wrote:
> > > On Wed, 26 Feb 2003, Jean Tourrilhes wrote:
> > > 
> > > > On Tue, Feb 25, 2003 at 11:33:09PM -0500, Albert Cahalan wrote:
> > > > > Jean Tourrilhes writes:
> > > > > 
> > > > > > It looks like a compiler bug to me...
> > > > > > 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).
> > > > > > Code (from linux/include/net/iw_handler.h) :
> > > > > > --------------------------------------------
> > > > > > static inline char *
> > > > > > iwe_stream_add_event(char * stream,  /* Stream of events */
> > > > > >        char * ends,  /* End of stream */
> > > > > >        struct iw_event *iwe, /* Payload */
> > > > > >        int event_len) /* Real size of payload */
> > > > > > {
> > > > > >  /* Check if it's possible */
> > > > > >  if((stream + event_len) < ends) {
> > > > > >   iwe->len = event_len;
> > > > > >   memcpy(stream, (char *) iwe, event_len);
> > > > > >   stream += event_len;
> > > > > >  } return stream;
> > > > > > }
> > > > > > --------------------------------------------
> > > > > > IMHO, the compiler should have enough context to
> > > > > > know that the reordering is dangerous. Any suggestion
> > > > > > to make this simple code more bullet proof is welcomed.
> > > > > >
> > > > > > Have fun...
> > > > > 
> > > > > Since (char*) is special, I agree that it's a bug.
> > > > > In any case, a warning sure would be nice!
> > > > > 
> > > > > Now for the fun. Pass iwe->len into this
> > > > > macro before the memcpy, and all should be well.
> > > > > 
> > > > > #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> > > > > 
> > > > > Like this:
> > > > > 
> > > > >    iwe->len = event_len;
> > > > >    FORCE_TO_MEM(iwe->len);
> > > > >    memcpy(stream, (char *) iwe, event_len);
> > > > 
> > > > 	I'll try that, that sounds absolutely clever (but I only
> > > > understand half of it).
> > > > 	Thanks a lot !
> > > > 
> > > > 	Jean
> > > > -
> > > 
> > > This does absoultely nothing with egcs-2.91.66. I also modified
> > > 
> > >  #define FORCE_TO_MEM(x) asm volatile(""::"r"(&(x)))
> > >                                            |________ this to "memory"
> > > 
> > > and it still does nothing. The result of gcc -O2 -S -o xxx xxx.c just
> > > shows:
> > > 
> > > #AP
> > > #NOAP
> > > 
> > > With no code in-between.
> > > 
> > > I also changed it to:
> > >  #define FORCE_TO_MEM(x) __asm__ __volatile__(""::"r"(&(x)))
> > > to no avail.
> > > 
> > > What's up?
> > 
> > That's "working".  Does it prevent the reordering?
> 
> It was supposed to force x, which may be cached in a register,
> to be written to memory __now__. It doesn't seem to do anything.
> I think FORCE_TO_MEM() needs to claim that it uses most all the
> registers. That will make sure that any register values get
> written to their final memory locations.

If so it wouldn't be inside the #APP/#NOAPP markers.  You didn't answer
my other question: was X in memory at the time?

You should be using something like __asm__ __volatile__ (""::"m"(x))
anyway.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Invalid compilation without -fno-strict-aliasing

[Richard B. Johnson]

On Wed, 26 Feb 2003, Daniel Jacobowitz wrote:
[SNIPPED...]

> > It was supposed to force x, which may be cached in a register,
> > to be written to memory __now__. It doesn't seem to do anything.
> > I think FORCE_TO_MEM() needs to claim that it uses most all the
> > registers. That will make sure that any register values get
> > written to their final memory locations.
> 
> If so it wouldn't be inside the #APP/#NOAPP markers.  You didn't answer
> my other question: was X in memory at the time?

It was in %ebx register and didn't go back to NNN(%esp) where
it came from. Like I said, it did do anything.

> 
> You should be using something like __asm__ __volatile__ (""::"m"(x))
> anyway.
> 

Yep. Probably.


Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: Invalid compilation without -fno-strict-aliasing

[Albert Cahalan]

On Wed, 2003-02-26 at 15:19, Richard B. Johnson wrote:
> On Wed, 26 Feb 2003, Daniel Jacobowitz wrote:

>>> It was supposed to force x, which may be cached in a register,
>>> to be written to memory __now__. It doesn't seem to do anything.
>>> I think FORCE_TO_MEM() needs to claim that it uses most all the
>>> registers. That will make sure that any register values get
>>> written to their final memory locations.
>>
>> If so it wouldn't be inside the #APP/#NOAPP markers.  You didn't
>> answer my other question: was X in memory at the time?
>
> It was in %ebx register and didn't go back to NNN(%esp) where
> it came from. Like I said, it did do anything.
>
>> You should be using something like __asm__ __volatile__ (""::"m"(x))
>> anyway.
>
> Yep. Probably.

I wrote that from my memory and gcc documentation. With none
of my 3 compilers did I see the sample code failing, so I
wasn't able to perform a useful test. There may be some little
detail I missed.

I've had the code working, with gcc 3.0 I'd guess, on both
PowerPC and something strange. The code relies on "r" being
a suitable register; this might only be true for common RISC
architectures. So this is semi-portable code, able to be used
where the gcc porter followed the suggestion to use "r" as
the normal sort of register. Since x86 is notably lacking in
the concept of general purpose registers, the above might not
apply.

Add :"memory" (a clobber) on the end if you want to be brutal.
This may produce slower code though.

The basic idea is to convince gcc that some assembly code
will access the data via a pointer. Probably somebody else
has already posted a fixed version.

With a recent gcc, look into the __may_alias__ attribute.

This kind of stuff sure is poorly documented, isn't it?
Has anybody ever seen a reasonable explanation of the new
C99 restrict keyword, including scope interactions? How
about a list of gcc bugs in this area? Even with a recent
gcc, control over aliasing is really crude. I ought to be
able to specify a list of types and objects that do or do
not alias, with each other or with the first one specified.

Invalid compilation without -fno-strict-aliasing

[Yubin Ruan]

Does anyone have any idea why this thread

    https://lkml.org/lkml/2003/2/25/270

is related to strict-aliasing? To me, a compiler barrier like this will fix it:

    if((stream + event_len) < ends) {
        iwe->len = event_len;
        barrier();
        memcpy(stream, (char *) iwe, event_len);
        stream += event_len;
    }

Yubin

Re: Invalid compilation without -fno-strict-aliasing

[Paul E. McKenney]

On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
> Does anyone have any idea why this thread
> 
>     https://lkml.org/lkml/2003/2/25/270

Hmmm...  This is quite the blast from the past.  Compilers have changed
a bit in the last 14 years.  Nevertheles...

> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
> 
>     if((stream + event_len) < ends) {
>         iwe->len = event_len;
>         barrier();
>         memcpy(stream, (char *) iwe, event_len);
>         stream += event_len;
>     }

As with many bugs, there are a number of ways to fix this one.  I suggest taking
a look at the documentation for -no-strict-alias.  This stackoverflow URL might
not be a bad place to start:

https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers

								Thanx, Paul

Re: Invalid compilation without -fno-strict-aliasing

[Yubin Ruan]

Thanks Paul,

2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
>> Does anyone have any idea why this thread
>>
>>     https://lkml.org/lkml/2003/2/25/270
>
> Hmmm...  This is quite the blast from the past.  Compilers have changed
> a bit in the last 14 years.  Nevertheles...

Unfortunately I got several old servers with GCC 3.x so I have to keep
an eye on these things (and worse, I am one of those guys who like to
home-brew some locking primitives...)

>> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
>>
>>     if((stream + event_len) < ends) {
>>         iwe->len = event_len;
>>         barrier();
>>         memcpy(stream, (char *) iwe, event_len);
>>         stream += event_len;
>>     }
>
> As with many bugs, there are a number of ways to fix this one.  I suggest taking
> a look at the documentation for -no-strict-alias.  This stackoverflow URL might
> not be a bad place to start:
>
> https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers

Basically I understand what aliasing means (for the C/C++ Languages)
and I know what a strict aliasing rule it is. I just cannot see why a
strict aliasing rule will cause this reordering problem...

Yubin

Re: Invalid compilation without -fno-strict-aliasing

[Paul E. McKenney]

On Fri, Nov 03, 2017 at 09:33:48PM +0800, Yubin Ruan wrote:
> Thanks Paul,
> 
> 2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> > On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
> >> Does anyone have any idea why this thread
> >>
> >>     https://lkml.org/lkml/2003/2/25/270
> >
> > Hmmm...  This is quite the blast from the past.  Compilers have changed
> > a bit in the last 14 years.  Nevertheles...
> 
> Unfortunately I got several old servers with GCC 3.x so I have to keep
> an eye on these things (and worse, I am one of those guys who like to
> home-brew some locking primitives...)
> 
> >> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
> >>
> >>     if((stream + event_len) < ends) {
> >>         iwe->len = event_len;
> >>         barrier();
> >>         memcpy(stream, (char *) iwe, event_len);
> >>         stream += event_len;
> >>     }
> >
> > As with many bugs, there are a number of ways to fix this one.  I suggest taking
> > a look at the documentation for -no-strict-alias.  This stackoverflow URL might
> > not be a bad place to start:
> >
> > https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers
> 
> Basically I understand what aliasing means (for the C/C++ Languages)
> and I know what a strict aliasing rule it is. I just cannot see why a
> strict aliasing rule will cause this reordering problem...

OK.  Then please tell me why exactly you believe that strict aliasing does
not allow the compiler to reverse the order of the first two statements
in the body of the above "if" statement.

							Thanx, Paul

Re: Invalid compilation without -fno-strict-aliasing

[Yubin Ruan]

2017-11-03 22:03 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Fri, Nov 03, 2017 at 09:33:48PM +0800, Yubin Ruan wrote:
>> Thanks Paul,
>>
>> 2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> > On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
>> >> Does anyone have any idea why this thread
>> >>
>> >>     https://lkml.org/lkml/2003/2/25/270
>> >
>> > Hmmm...  This is quite the blast from the past.  Compilers have changed
>> > a bit in the last 14 years.  Nevertheles...
>>
>> Unfortunately I got several old servers with GCC 3.x so I have to keep
>> an eye on these things (and worse, I am one of those guys who like to
>> home-brew some locking primitives...)
>>
>> >> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
>> >>
>> >>     if((stream + event_len) < ends) {
>> >>         iwe->len = event_len;
>> >>         barrier();
>> >>         memcpy(stream, (char *) iwe, event_len);
>> >>         stream += event_len;
>> >>     }
>> >
>> > As with many bugs, there are a number of ways to fix this one.  I suggest taking
>> > a look at the documentation for -no-strict-alias.  This stackoverflow URL might
>> > not be a bad place to start:
>> >
>> > https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers
>>
>> Basically I understand what aliasing means (for the C/C++ Languages)
>> and I know what a strict aliasing rule it is. I just cannot see why a
>> strict aliasing rule will cause this reordering problem...
>
> OK.  Then please tell me why exactly you believe that strict aliasing does
> not allow the compiler to reverse the order of the first two statements
> in the body of the above "if" statement.

Indeed, the compiler is free to reorder those two statements and
strict aliasing does not prevent the compiler from doing so. But, from
my perspective, this reordering issue might just come from some other
optimization rules (so we have to use barrier()), not because of the
strict aliasing rule.

Hmm... am I missing anything?

Yubin

Re: Invalid compilation without -fno-strict-aliasing

[Paul E. McKenney]

On Fri, Nov 03, 2017 at 10:12:48PM +0800, Yubin Ruan wrote:
> 2017-11-03 22:03 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> > On Fri, Nov 03, 2017 at 09:33:48PM +0800, Yubin Ruan wrote:
> >> Thanks Paul,
> >>
> >> 2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> >> > On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
> >> >> Does anyone have any idea why this thread
> >> >>
> >> >>     https://lkml.org/lkml/2003/2/25/270
> >> >
> >> > Hmmm...  This is quite the blast from the past.  Compilers have changed
> >> > a bit in the last 14 years.  Nevertheles...
> >>
> >> Unfortunately I got several old servers with GCC 3.x so I have to keep
> >> an eye on these things (and worse, I am one of those guys who like to
> >> home-brew some locking primitives...)
> >>
> >> >> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
> >> >>
> >> >>     if((stream + event_len) < ends) {
> >> >>         iwe->len = event_len;
> >> >>         barrier();
> >> >>         memcpy(stream, (char *) iwe, event_len);
> >> >>         stream += event_len;
> >> >>     }
> >> >
> >> > As with many bugs, there are a number of ways to fix this one.  I suggest taking
> >> > a look at the documentation for -no-strict-alias.  This stackoverflow URL might
> >> > not be a bad place to start:
> >> >
> >> > https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers
> >>
> >> Basically I understand what aliasing means (for the C/C++ Languages)
> >> and I know what a strict aliasing rule it is. I just cannot see why a
> >> strict aliasing rule will cause this reordering problem...
> >
> > OK.  Then please tell me why exactly you believe that strict aliasing does
> > not allow the compiler to reverse the order of the first two statements
> > in the body of the above "if" statement.
> 
> Indeed, the compiler is free to reorder those two statements and
> strict aliasing does not prevent the compiler from doing so. But, from
> my perspective, this reordering issue might just come from some other
> optimization rules (so we have to use barrier()), not because of the
> strict aliasing rule.
> 
> Hmm... am I missing anything?

When you tell gcc -no-strict-aliasing (which is a non-standard extension,
not part of the C language), the compiler is required to allow for the
fact that two pointers of different types might well reference the same
location(s).  In this case, the compiler cannot reverse the statements,
just as it would not be allowed to reverse the order of equivalent
non-pointer statements.

							Thanx, Paul

Re: Invalid compilation without -fno-strict-aliasing

[Yubin Ruan]

Thanks Paul,

2017-11-03 22:38 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Fri, Nov 03, 2017 at 10:12:48PM +0800, Yubin Ruan wrote:
>> 2017-11-03 22:03 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> > On Fri, Nov 03, 2017 at 09:33:48PM +0800, Yubin Ruan wrote:
>> >> Thanks Paul,
>> >>
>> >> 2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> >> > On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
>> >> >> Does anyone have any idea why this thread
>> >> >>
>> >> >>     https://lkml.org/lkml/2003/2/25/270
>> >> >
>> >> > Hmmm...  This is quite the blast from the past.  Compilers have changed
>> >> > a bit in the last 14 years.  Nevertheles...
>> >>
>> >> Unfortunately I got several old servers with GCC 3.x so I have to keep
>> >> an eye on these things (and worse, I am one of those guys who like to
>> >> home-brew some locking primitives...)
>> >>
>> >> >> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
>> >> >>
>> >> >>     if((stream + event_len) < ends) {
>> >> >>         iwe->len = event_len;
>> >> >>         barrier();
>> >> >>         memcpy(stream, (char *) iwe, event_len);
>> >> >>         stream += event_len;
>> >> >>     }
>> >> >
>> >> > As with many bugs, there are a number of ways to fix this one.  I suggest taking
>> >> > a look at the documentation for -no-strict-alias.  This stackoverflow URL might
>> >> > not be a bad place to start:
>> >> >
>> >> > https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers
>> >>
>> >> Basically I understand what aliasing means (for the C/C++ Languages)
>> >> and I know what a strict aliasing rule it is. I just cannot see why a
>> >> strict aliasing rule will cause this reordering problem...
>> >
>> > OK.  Then please tell me why exactly you believe that strict aliasing does
>> > not allow the compiler to reverse the order of the first two statements
>> > in the body of the above "if" statement.

I think I understand your point now: if two pointers of different
types might reference the same location, then the compiler cannot
reverse the statements (just as it is not allowed to reverse the above
"iwe->len " and "memcpy" statements).

But why is this? After all, the compiler does not know the context.

Thanks,
Yubin

>> Indeed, the compiler is free to reorder those two statements and
>> strict aliasing does not prevent the compiler from doing so. But, from
>> my perspective, this reordering issue might just come from some other
>> optimization rules (so we have to use barrier()), not because of the
>> strict aliasing rule.
>>
>> Hmm... am I missing anything?
>
> When you tell gcc -no-strict-aliasing (which is a non-standard extension,
> not part of the C language), the compiler is required to allow for the
> fact that two pointers of different types might well reference the same
> location(s).  In this case, the compiler cannot reverse the statements,
> just as it would not be allowed to reverse the order of equivalent
> non-pointer statements.
>
>                                                         Thanx, Paul
>

Re: Invalid compilation without -fno-strict-aliasing

[Paul E. McKenney]

On Sat, Nov 04, 2017 at 07:26:27AM +0800, Yubin Ruan wrote:
> Thanks Paul,
> 
> 2017-11-03 22:38 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> > On Fri, Nov 03, 2017 at 10:12:48PM +0800, Yubin Ruan wrote:
> >> 2017-11-03 22:03 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> >> > On Fri, Nov 03, 2017 at 09:33:48PM +0800, Yubin Ruan wrote:
> >> >> Thanks Paul,
> >> >>
> >> >> 2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> >> >> > On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
> >> >> >> Does anyone have any idea why this thread
> >> >> >>
> >> >> >>     https://lkml.org/lkml/2003/2/25/270
> >> >> >
> >> >> > Hmmm...  This is quite the blast from the past.  Compilers have changed
> >> >> > a bit in the last 14 years.  Nevertheles...
> >> >>
> >> >> Unfortunately I got several old servers with GCC 3.x so I have to keep
> >> >> an eye on these things (and worse, I am one of those guys who like to
> >> >> home-brew some locking primitives...)
> >> >>
> >> >> >> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
> >> >> >>
> >> >> >>     if((stream + event_len) < ends) {
> >> >> >>         iwe->len = event_len;
> >> >> >>         barrier();
> >> >> >>         memcpy(stream, (char *) iwe, event_len);
> >> >> >>         stream += event_len;
> >> >> >>     }
> >> >> >
> >> >> > As with many bugs, there are a number of ways to fix this one.  I suggest taking
> >> >> > a look at the documentation for -no-strict-alias.  This stackoverflow URL might
> >> >> > not be a bad place to start:
> >> >> >
> >> >> > https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers
> >> >>
> >> >> Basically I understand what aliasing means (for the C/C++ Languages)
> >> >> and I know what a strict aliasing rule it is. I just cannot see why a
> >> >> strict aliasing rule will cause this reordering problem...
> >> >
> >> > OK.  Then please tell me why exactly you believe that strict aliasing does
> >> > not allow the compiler to reverse the order of the first two statements
> >> > in the body of the above "if" statement.
> 
> I think I understand your point now: if two pointers of different
> types might reference the same location, then the compiler cannot
> reverse the statements (just as it is not allowed to reverse the above
> "iwe->len " and "memcpy" statements).
> 
> But why is this? After all, the compiler does not know the context.

It is just part of the contract between the compiler and the
developer.

1.	If you don't specify -no-strict-alias, then you as the developer
	are responsible for making sure that no two pointers of different
	types reference the same location at the same time.  If you fail
	in this responsibility, as the above example code failed, then
	the compiler is within its rights to assume that two pointers
	of different type cannot possibly reference the same location.
	The compiler is thus free to reorder any such references.
	(And there is one exception to this rule to allow certain I/O
	functions to work correctly.)

2.	If you -do- specify -no-strict-alias, then the responsibility
	falls on the compiler.	If the compiler cannot prove that a given
	pair of pointers reference different locations, then the compiler
	if forced to assume that they might reference the same location.
	In this case, the compiler is forbidden from reordering the
	above code.

Why have two approaches?

If you are writing heavy-duty numerical/vector/matrix code, #1 works
really well, allowing pointers to overlapping portions of matrices
but still allowing the compiler to undertake valuable optimizations
that reorder code rather freely.

If you are writing the Linux kernel, there is so much use of pointers
to different types referencing the same data that it the optimizations
are often destructive.

So that is one reason why both options are available.  There are
probably others.

							Thanx, Paul

> Thanks,
> Yubin
> 
> >> Indeed, the compiler is free to reorder those two statements and
> >> strict aliasing does not prevent the compiler from doing so. But, from
> >> my perspective, this reordering issue might just come from some other
> >> optimization rules (so we have to use barrier()), not because of the
> >> strict aliasing rule.
> >>
> >> Hmm... am I missing anything?
> >
> > When you tell gcc -no-strict-aliasing (which is a non-standard extension,
> > not part of the C language), the compiler is required to allow for the
> > fact that two pointers of different types might well reference the same
> > location(s).  In this case, the compiler cannot reverse the statements,
> > just as it would not be allowed to reverse the order of equivalent
> > non-pointer statements.
> >
> >                                                         Thanx, Paul
> >
> 

Re: Invalid compilation without -fno-strict-aliasing

[Yubin Ruan]

Thanks Paul,

2017-11-04 7:55 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
> On Sat, Nov 04, 2017 at 07:26:27AM +0800, Yubin Ruan wrote:
>> Thanks Paul,
>>
>> 2017-11-03 22:38 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> > On Fri, Nov 03, 2017 at 10:12:48PM +0800, Yubin Ruan wrote:
>> >> 2017-11-03 22:03 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> >> > On Fri, Nov 03, 2017 at 09:33:48PM +0800, Yubin Ruan wrote:
>> >> >> Thanks Paul,
>> >> >>
>> >> >> 2017-11-03 20:27 GMT+08:00 Paul E. McKenney <paulmck@linux.vnet.ibm.com>:
>> >> >> > On Fri, Nov 03, 2017 at 09:54:15AM +0800, Yubin Ruan wrote:
>> >> >> >> Does anyone have any idea why this thread
>> >> >> >>
>> >> >> >>     https://lkml.org/lkml/2003/2/25/270
>> >> >> >
>> >> >> > Hmmm...  This is quite the blast from the past.  Compilers have changed
>> >> >> > a bit in the last 14 years.  Nevertheles...
>> >> >>
>> >> >> Unfortunately I got several old servers with GCC 3.x so I have to keep
>> >> >> an eye on these things (and worse, I am one of those guys who like to
>> >> >> home-brew some locking primitives...)
>> >> >>
>> >> >> >> is related to strict-aliasing? To me, a compiler barrier like this will fix it:
>> >> >> >>
>> >> >> >>     if((stream + event_len) < ends) {
>> >> >> >>         iwe->len = event_len;
>> >> >> >>         barrier();
>> >> >> >>         memcpy(stream, (char *) iwe, event_len);
>> >> >> >>         stream += event_len;
>> >> >> >>     }
>> >> >> >
>> >> >> > As with many bugs, there are a number of ways to fix this one.  I suggest taking
>> >> >> > a look at the documentation for -no-strict-alias.  This stackoverflow URL might
>> >> >> > not be a bad place to start:
>> >> >> >
>> >> >> > https://stackoverflow.com/questions/23848188/strict-aliasing-rule-and-char-pointers
>> >> >>
>> >> >> Basically I understand what aliasing means (for the C/C++ Languages)
>> >> >> and I know what a strict aliasing rule it is. I just cannot see why a
>> >> >> strict aliasing rule will cause this reordering problem...
>> >> >
>> >> > OK.  Then please tell me why exactly you believe that strict aliasing does
>> >> > not allow the compiler to reverse the order of the first two statements
>> >> > in the body of the above "if" statement.
>>
>> I think I understand your point now: if two pointers of different
>> types might reference the same location, then the compiler cannot
>> reverse the statements (just as it is not allowed to reverse the above
>> "iwe->len " and "memcpy" statements).
>>
>> But why is this? After all, the compiler does not know the context.
>
> It is just part of the contract between the compiler and the
> developer.
>
> 1.      If you don't specify -no-strict-alias, then you as the developer
>         are responsible for making sure that no two pointers of different
>         types reference the same location at the same time.  If you fail
>         in this responsibility, as the above example code failed, then
>         the compiler is within its rights to assume that two pointers
>         of different type cannot possibly reference the same location.
>         The compiler is thus free to reorder any such references.
>         (And there is one exception to this rule to allow certain I/O
>         functions to work correctly.)
>
> 2.      If you -do- specify -no-strict-alias, then the responsibility
>         falls on the compiler.  If the compiler cannot prove that a given
>         pair of pointers reference different locations, then the compiler
>         if forced to assume that they might reference the same location.
>         In this case, the compiler is forbidden from reordering the
>         above code.
>
> Why have two approaches?
>
> If you are writing heavy-duty numerical/vector/matrix code, #1 works
> really well, allowing pointers to overlapping portions of matrices
> but still allowing the compiler to undertake valuable optimizations
> that reorder code rather freely.

Much more clear with this valuable insight!

So, without -no-strict-aliasing(that is, enforcing strict aliasing),
the compiler is within its right to assume that the "iwe" and "stream"
above point to different area of memory, and thus can reorder those
two statements rather freely.

I bet that with a more modern compiler, it can recognize the context
because there is a "cha *" type cast there, which indicates that those
two pointers may point to the same address.

Thank you very much;-)
Yubin

> If you are writing the Linux kernel, there is so much use of pointers
> to different types referencing the same data that it the optimizations
> are often destructive.
>
> So that is one reason why both options are available.  There are
> probably others.
>
>                                                         Thanx, Paul
>
>> Thanks,
>> Yubin
>>
>> >> Indeed, the compiler is free to reorder those two statements and
>> >> strict aliasing does not prevent the compiler from doing so. But, from
>> >> my perspective, this reordering issue might just come from some other
>> >> optimization rules (so we have to use barrier()), not because of the
>> >> strict aliasing rule.
>> >>
>> >> Hmm... am I missing anything?
>> >
>> > When you tell gcc -no-strict-aliasing (which is a non-standard extension,
>> > not part of the C language), the compiler is required to allow for the
>> > fact that two pointers of different types might well reference the same
>> > location(s).  In this case, the compiler cannot reverse the statements,
>> > just as it would not be allowed to reverse the order of equivalent
>> > non-pointer statements.
>> >
>> >                                                         Thanx, Paul
>> >
>>
>