Re: gcc miscompiling duff's device (probaby two different bugs)

[Pjotr Kourzanov <peter dot kourzanov at xs4all dot nl>]

On Tue, 2010-03-02 at 10:24 +0000, Andrew Haley wrote: 
> On 03/02/2010 09:38 AM, Peter Kourzanov wrote:
> 
> >   I have the following variation on Duff's device that seems to 
> > mis-compile on all GCC versions I can access within a minute (that 
> > is gcc-3.{3,4}, gcc-4.{1,2,3,4} on x86 and gcc-4.3.2 on x86_64). The 
> > symptoms are as follows:
> > 
> > $ gcc-4.4 -o duffbug duffbug.c ; ./duffbug
> > { heïï3)
> > { hello world ! }
> > 
> >   As you can observe in the difference between duff4_works() and 
> > duff4_fails(), apparently due to for-loop initializer being externalized
> > vs. specified as the first for-loop expression. It doesn't matter if the
> > 'case 0' is  labeling the for-loop, or the first statement in the 
> > for-loop in case of duff4_works() of course. However, older gcc-3.x do
> > give a warning though if the 'case 0' labels the first statement for 
> > duff4_fails(), since the first expression in the for-loop is then
> > inaccessible. All gcc-4.x versions don't warn, even when supplied with
> > the -Wall flag (which is wrong, hence this *first* bug):
> 
> So, your claim is that gcc should warn about the for loop initializer
> being unreachable.  is that correct?

Exactly. Just like what gcc-3.x does, even without the -Wall flag.

> 
> > $ gcc-4.4 -Wall -o duffbug duffbug.c ; ./duffbug
> > $ gcc-3.4 -Wall -o duffbug duffbug.c ; ./duffbug
> > duffbug.c: In function `duff4_fails':
> > duffbug.c:28: warning: unreachable code at beginning of switch statement
> > 
> >   I think the compiler is generating wrong code for duff4_fails() when
> > 'case 0' labels the for-loop. It somehow skips the first for-loop
> > expression, just as if 'case 0' pointed to the first statement in the
> > for-loop (hence this *second* bug). Haven't checked the assembly
> > though...
> 
> I don't understand.  In what way is the code gcc generates wrong?
> 
> int duff4_fails(char * dst,const char * src,const size_t n)
> {
>   const size_t rem=n % 4, a=rem + (!rem)*4;
>   char * d=dst+=a;
>   const char * s=src+=a;
>   /* gcc bug? dst+=n; */
>   
>     switch (rem) {
>     case 0:  for(dst+=n;d<dst;d+=4,s+=4) {
>     /*case 0:*/	d[-4]=s[-4];
>     case 3:	d[-3]=s[-3];
>     case 2:	d[-2]=s[-2];
>     case 1:	d[-1]=s[-1];
> 	      }
>     }
> 	return 0;
> }
> The first time around the loop the initializer (d+=n) is jumped around, so
> d == dst.  At the end of the loop, d+=4, so d > dst.  Therefore the loop
> exits.

  And its wrong since it shouldn't jump around the initializer.  The
following two snippets exhibit the same behaviour:

> case 0:  for(dst+=n;d<dst;d+=4,s+=4) {
> /*case 0:*/	d[-4]=s[-4];

> /*case 0:*/  for(dst+=n;d<dst;d+=4,s+=4)  {
> case 0:	d[-4]=s[-4];

  Which is wrong IMHO.

Kind regards,

Pjotr