Bug 40698 - Incorrect code generation when compiling c++ source
Summary: Incorrect code generation when compiling c++ source
Status: RESOLVED DUPLICATE of bug 21920
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.3.3
: P3 major
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-09 13:13 UTC by jacob navia
Modified: 2009-07-09 15:35 UTC (History)
34 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
This is the cpp source needed to reproduce the bug (716 bytes, text/plain)
2009-07-09 13:17 UTC, jacob navia
Details
Needed for linking the cpp code (387 bytes, text/plain)
2009-07-09 13:18 UTC, jacob navia
Details
script that demonstrates the bug (262 bytes, text/plain)
2009-07-09 13:18 UTC, jacob navia
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jacob navia 2009-07-09 13:13:08 UTC
This bug appears in ALL version of gcc that we tested since gcc 4.1.2

If you see the assembly generated by the cpp source using the
-DNOT_WORKING
option

you will see:
main:
	leal	4(%esp), %ecx
	andl	$-16, %esp
	pushl	-4(%ecx)
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%esi
	pushl	%ebx
	pushl	%ecx
	subl	$92, %esp
	movl	-20(%ebp), %eax
	movl	$0, -44(%ebp)
/*
Here is the bug. The compiler generates code to read
*location -16(%ebp) before anything was written to it!
*/
	movl	-16(%ebp), %edx
	leal	-44(%ebp), %esi
	movl	$0, -40(%ebp)
---------------------------------------------------------
Attached are 3 files:
(1) The cpp source
(2) an associated assembly file needed to link. That code is not
   the source of the bug since the bug is in the calling procedure
(3) A shell script exercising the bug and a workaround that masks
   the bug.

This is the code that produces the bug:
------------------------------------------------------------------------------
bug.cpp File (1)
------------------------------------------------------------------------------
#include <stdio.h>
#include <sys/types.h>

extern "C" {
    extern int64_t __cmpxchg8bytes(volatile int64_t *destination, volatile int64_t exchange, volatile int64_t compare);
}

#if	defined(WORKING)

// there is no error by using the union of data
class TwoPointers
{
public:
    TwoPointers() { u1.s1.First = NULL; u1.s1.Second = NULL; }

    void* GetFirst() const     { return  u1.s1.First; } 
    void* GetSecond() const    { return  u1.s1.Second; } 
    void SetFirst(void* ptr)   {  u1.s1.First = ptr; } 
    void SetSecond(void* ptr)  {  u1.s1.Second = ptr; }

    union unionData {
        struct structData1 {
            void* volatile First;
            void* volatile Second;
        } s1;
        long long longlongData;
    } u1;
};

__inline TwoPointers InterlockedExchange(TwoPointers* Target, TwoPointers Value)
{
    long long retVal = __cmpxchg8bytes(( long long *)Target, *((long long *) (&Value.u1.longlongData)), *(long long *) (&Target->u1.longlongData) ) ; 
    return ( *(TwoPointers *) &retVal ) ;
}

#else	// defined(WORKING)

// this generates wrong code
class TwoPointers
{
public:
    TwoPointers() { First = NULL; Second = NULL; }

    void* GetFirst() const     { return First; } 
    void* GetSecond() const    { return Second; } 
    void SetFirst(void* ptr)   { First = ptr; } 
    void SetSecond(void* ptr)  { Second = ptr; }

    void* volatile First;
    void* volatile Second;
};

__inline TwoPointers InterlockedExchange(TwoPointers* Target, TwoPointers Value)
{
    long long retVal = __cmpxchg8bytes(( long long *)Target, *((long long *) (&Value)), *(long long *) (&Target) ) ; 
    return ( *(TwoPointers *) &retVal ) ;
}

#endif	// defined(WORKING)

int main()
{
    unsigned long long i = 0xdeaddeaddeaddeadll ;
    unsigned long long j = 0xbeafbeafbeafbeafll ;
    TwoPointers a ;
    TwoPointers b ;

    a.SetFirst( (void *) 0xdeaddead ) ;
    a.SetSecond( ( void *) 0xdeaddead ) ;

    b.SetFirst( (void *) 0xbeafbeaf ) ;
    b.SetSecond( (void *) 0xbeafbeaf ) ;

    InterlockedExchange( &a, b ) ;

    if ( a.GetFirst() == b.GetFirst() && a.GetSecond() == b.GetSecond() ) {
        printf("Succeeded\n");
    } else {
        printf("Failed\n");
        printf("It should be 0x%016llx\n", *( (long long *)&b) ) ;
        printf("Real data is 0x%016llx\n", *( (long long *)&a) ) ;
    }
}
-------------------------------------------------------------------------
i386xadd.s File (2)
-------------------------------------------------------------------------
/* BEGIN */
.global __cmpxchg8bytes
	.type __cmpxchg8bytes,@function
__cmpxchg8bytes:
	pushl %ebp
	movl %esp, %ebp
/*
* stack frame layout
* =============
* old (8 byes)
* new (8 bytes)
* ptr (4 bytes)
* rip (4 bytes)
* old ebp
* ============ # ebp and esp point here
*/

/*
* Save ecx, ebx CMPXCHG8B uses them. Save esi, this code uses it.
*/
	pushl %ecx
	pushl %ebx
	pushl %esi

/* move ptr to esi */
	movl 8(%ebp), %esi

/* move new to ecx:ebx; low order word at bottom, high word at top */
	movl 16(%ebp), %ecx
	movl 12(%ebp), %ebx

/* move old to edx:eax; low order word at bottom, high word at top */
	movl 24(%ebp), %edx
	movl 20(%ebp), %eax

/* magic word */
	lock cmpxchg8b (%esi)

/* restore regs and stack */
	popl %esi
	popl %ebx
	popl %ecx
	leave
	ret

/* END */
--------------------------------------------------------------------
File 3: runit.sh
This script demonstrates the bug
--------------------------------------------------------------------
#!/bin/sh

program="bug"
cmd_as="/usr/local/gcc-4.1.2/bin/gcc -O2 -o i386xadd.o -c i386xadd.s"
echo $cmd_as
$cmd_as

compiler="/usr/local/gcc-4.1.2/bin/g++"

for flags in NOT_WORKING WORKING
do

    echo
    echo "********** Compile and run with flag $flags turned on  **********"

    options=" -march=i686 -O3 i386xadd.o -lgcc -lgcc_s -lstdc++"
    cmd="$compiler $options -D$flags $program.cpp -o $program"
    echo $cmd
    $cmd

    echo
    ./$program
    echo

done
Comment 1 jacob navia 2009-07-09 13:17:21 UTC
Created attachment 18167 [details]
This is the cpp source needed to reproduce the bug
Comment 2 jacob navia 2009-07-09 13:18:15 UTC
Created attachment 18168 [details]
Needed for linking the cpp code
Comment 3 jacob navia 2009-07-09 13:18:54 UTC
Created attachment 18169 [details]
script that demonstrates the bug
Comment 4 Jakub Jelinek 2009-07-09 13:50:56 UTC
The code has a bunch of aliasing violations, so is invalid.

*** This bug has been marked as a duplicate of 21920 ***
Comment 5 jacob navia 2009-07-09 14:34:55 UTC
What do you mean by "aliasing violations" ???
Why would that code be invalid without any warning from the compiler?

It took us a lot of work for finding this bug and reporting it to you
can you explain?

Thanks 
Comment 6 Richard Biener 2009-07-09 15:26:13 UTC
g++-4.1 -S t.C -O2 -m32 -Wall
t.C: In function ‘TwoPointers InterlockedExchange(TwoPointers*, TwoPointers)’:
t.C:60: warning: dereferencing type-punned pointer will break strict-aliasing rules
t.C:60: warning: dereferencing type-punned pointer will break strict-aliasing rules
t.C:61: warning: dereferencing type-punned pointer will break strict-aliasing rules
t.C: In function ‘int main()’:
t.C:85: warning: dereferencing type-punned pointer will break strict-aliasing rules
t.C:86: warning: dereferencing type-punned pointer will break strict-aliasing rules
t.C:68: warning: unused variable ‘i’
t.C:69: warning: unused variable ‘j’


*** This bug has been marked as a duplicate of 21920 ***
Comment 7 Manuel López-Ibáñez 2009-07-09 15:35:25 UTC
@Jacob,

The C/C++ standards have some precise rules of what can and cannot be done about pointer casts.

See http://gcc.gnu.org/onlinedocs/gcc-4.4.0/gcc/Optimize-Options.html#index-fstrict_002daliasing-749

See also the documentation of -Wstrict-aliasing.

This is warned by GCC by using -Wall.

manu@localhost:~$ gcc --version
gcc (GCC) 4.2.4 (Ubuntu 4.2.4-1ubuntu4)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

manu@localhost:~$ g++ -Wall  -O2 pr40698.c -c
pr40698.c: In function ‘TwoPointers InterlockedExchange(TwoPointers*, TwoPointers)’:
pr40698.c:55: warning: dereferencing type-punned pointer will break strict-aliasing rules
pr40698.c:55: warning: dereferencing type-punned pointer will break strict-aliasing rules
pr40698.c:56: warning: dereferencing type-punned pointer will break strict-aliasing rules
pr40698.c: In function ‘int main()’:
pr40698.c:80: warning: dereferencing type-punned pointer will break strict-aliasing rules
pr40698.c:81: warning: dereferencing type-punned pointer will break strict-aliasing rules
pr40698.c:63: warning: unused variable ‘i’
pr40698.c:64: warning: unused variable ‘j’


If you have more questions, please do ask.