Bug 28073 - Type-punned pointer passed as function parameter generates bad assembly sequence
Type-punned pointer passed as function parameter generates bad assembly sequence
Status: RESOLVED DUPLICATE of bug 21920
Product: gcc
Classification: Unclassified
Component: c
4.1.0
: P3 normal
: ---
Assigned To: Not yet assigned to anyone
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-17 14:31 UTC by Jeffrey Sorensen
Modified: 2006-06-19 21:04 UTC (History)
63 users (show)

See Also:
Host: x86_64-linux-gnu
Target: x86_64-linux-gnu
Build: x86_64-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments
warn for bad casts hiding type-punning (7.79 KB, patch)
2006-06-19 21:04 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeffrey Sorensen 2006-06-17 14:31:02 UTC
The following test code
-- begin --
typedef struct {
  int val;
} Foo;

int func(long longPtr) {
  Foo *pFoo = *(Foo **)&longPtr; /* BAD! */
  /* Foo *pFoo = (Foo *)longPtr; If you do this instead it works */
  return pFoo->val;
}

int main(int argc, const char *argv[]) {
  Foo foo;
  foo.val = 1;

  return func((long)(&foo));
}
-- end --
When compiled with -O2 (which implies -fstrict-aliasing) on the x86_64 architectures changes the generated assembly sequence
<       movl    (%rdi), %eax
<       movq    %rdi, -8(%rsp)
--
>       movq    -8(%rsp), %rax
>       movl    (%rax), %eax

Comparing to other architectures, for example, i686 - the -O2 generated code
with -fstrict-aliasing and -fno-strict-aliasing is identical.  The code
generated with strict aliasing on the x86_64 is pretty much nonsense and
in the case of this test program will result in garbage from the stack being
returned from main (or, possibly, a seg fault)

Compare x86_64
$ gcc -O2 -Wall badcase.c; ./a.out; echo $?
badcase.c: In function ‘func’:
badcase.c:9: warning: dereferencing type-punned pointer will break strict-aliasing rules
76
$ gcc -O2 -fno-strict-aliasing -Wall badcase.c; ./a.out; echo $?
1
with i686
$ gcc -O2 -Wall badcase.c; ./a.out; echo $?
badcase.c: In function `func':
badcase.c:9: warning: dereferencing type-punned pointer will break strict-aliasing rules
1

Please note that putting in any diagnostic code into func (for example a 
print statement) will "fix" it because it changes the determination of aliasing.
This makes this particular interaction extra hard to spot.

After some considerable debate with my colleagues about the nature of the code and its use of type-punning to convert a long into a pointer, I decided to submit this as a bug because (1) google reveals that a great many distribution builders are adding -fno-strict-aliasing to get their distributions building and working on x86_64 and (2) this ugly construct is the exact type of code that the current version of the (somewhat) popular swig library wrapper generates.  I intend to open a bug against SWIG as well as there is no good reason why a code generator should generate code like this.

Some of us believe that this code violates the standard (although how specifically they cannot say) and, thus, the compiler is under no obligation to compile it correctly.  Even so, in this particular case, it would be better not to generate the eroneous instruction sequence.  Hopefully it will be reasonably easy to pin down under which conditions the instruction sequence is changed, for this particular architecture, and perhaps this might point the way to a more fundamental bug.

$ gcc -v
Using built-in specs.
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-libgcj-multifile --enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre --with-cpu=generic --host=x86_64-redhat-linux
Thread model: posix
gcc version 4.1.0 20060304 (Red Hat 4.1.0-3)
Comment 1 Andrew Pinski 2006-06-17 14:56:23 UTC
  Foo *pFoo = *(Foo **)&longPtr; /* BAD! */
The above statement accesses a "long" as a "Foo*" which does violate aliasing rules.

*** This bug has been marked as a duplicate of 21920 ***
Comment 2 Jeffrey Sorensen 2006-06-19 16:44:11 UTC
Changing just one line of the test program to the (AFAIK) legal C code.  By casting through void *, we are addressing Andrew's concerns about violating the C rules. 

  Foo *pFoo = *(Foo **) ((void *)&longPtr); /* // BAD! */

eliminates the type-punned warning, even at the highest possible warning level, and continues to generate code the results in a bad return value.  This test case illustrates that this problem is actually worse than we originally thought, as now incorrect code is generated without any warning.

$ gcc -Wstrict-aliasing=2 -Wall -O2 badcase.c
$ ./a.out ; echo $?
76
$ gcc -Wstrict-aliasing=2 -Wall -O2 -fno-strict-aliasing badcase.c
$ ./a.out ; echo $?
1
Comment 3 Andrew Pinski 2006-06-19 16:54:23 UTC
(In reply to comment #2)
> Changing just one line of the test program to the (AFAIK) legal C code.  By
> casting through void *, we are addressing Andrew's concerns about violating the
> C rules. 
> 
>   Foo *pFoo = *(Foo **) ((void *)&longPtr); /* // BAD! */

No you are still accessing a long as a "Foo*", the intermediate types does not change a thing.

The cast to "void*" is designed to get rid of the warning but does not get rid of the undefinedness of the code.

*** This bug has been marked as a duplicate of 21920 ***
Comment 4 Daniel Berlin 2006-06-19 18:55:21 UTC
Subject: Re:  Type-punned pointer passed as function parameter
 generates bad assembly sequence

sorenj at us dot ibm dot com wrote:
> ------- Comment #2 from sorenj at us dot ibm dot com  2006-06-19 16:44 -------
> Changing just one line of the test program to the (AFAIK) legal C code.  By
> casting through void *, we are addressing Andrew's concerns about violating the
> C rules. 

No you aren't.
The only thing that matters is what the type of the dereferenced pointer
is, not the intermediate casts.

For example,

int *foo
float b;
float *c;

b = 5.0
foo = (int*)&b
c = (float *)foo
printf("%f\n", *c);


is legal.


> 
>   Foo *pFoo = *(Foo **) ((void *)&longPtr); /* // BAD! */

Still not legal.

> 
> eliminates the type-punned warning, even at the highest possible warning level,
> and continues to generate code the results in a bad return value.  This test
> case illustrates that this problem is actually worse than we originally
> thought, as now incorrect code is generated without any warning.

We can't issue warnings in every case because it is impossible to detect
every case.

We could probably issue a warning in this case.
Comment 5 Richard Biener 2006-06-19 21:04:55 UTC
Created attachment 11706 [details]
warn for bad casts hiding type-punning

At suse we used the attached patch to teach packagers not "fix" strict aliasing warnings by introducing a (void *) cast inbetween.  Sort of a compromise between too much false positives and catching all evil use.