pointer post-incr problem with GCC 3.3.3 ??

pointer post-incr problem with GCC 3.3.3 ??

[Ron Flory]

hi-

  I believe I may have found an optimizer issue with x86 gcc C++
3.3.3 in the postfix-incr / assignment operation(s).

  I have searched the gcc bug tracking database for this issue and
have been unable to located an existing report of this issue.

------------------------------

background:

gcc -v :

Reading specs from /usr/lib/gcc-lib/i386-redhat-linux/3.3.3/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man 
--infodir=/usr/share/info --enable-shared --enable-threads=posix 
--disable-checking --disable-libunwind-exceptions --with-system-zlib 
--enable-__cxa_atexit --host=i386-redhat-linux
Thread model: posix
gcc version 3.3.3 20040412 (Red Hat Linux 3.3.3-7)

  This is gcc/c++ included with Fedora Core 2, Running on a
SMP P-III, Asus MB.

------------------------------

  The compilation line I used, w/ switches is:

gcc -o maze -O6 maze.cpp -lstdc++

----------------

Given two trivialized code fragments:

#1:
       while (*ptr)
       {
          *ptr = toupper(*ptr);

          ptr++;
       }

#2:
       while (*ptr)
          *(ptr++) = toupper(*ptr);

  I would assert these two code frags are functionally identical.

  Frag #1 appears to compile and operate correctly at all optimizer
settings, however frag #2 produces incorrect results for any
optimizer setting greater than "-O" (i.e., no optimization or -O
operate correctly).

  This is a simple 'strupr()' body, which should capitalize any
ascii c-string in-place.  If frag #2 is compiled with optimization
greater than "-O", the string "-abc" is altered to be "ABC" instead
of "-ABC", which becomes a string shelf-left operation- NOT the
desired effect.

-------------------------------------

  analysis of generated code:

// ***************************************
// ***** compiled with -O : works OK *****
// ***************************************

00001f38 <_Z6struprPc>:

void  strupr(char *ptr)
{
  1f38: 55              push   %ebp
  1f39: 89 e5           mov    %esp,%ebp
  1f3b: 56              push   %esi
  1f3c: 53              push   %ebx
  1f3d: 8b 75 08        mov    0x8(%ebp),%esi
if (ptr)
  1f40: 85 f6           test   %esi,%esi
  1f42: 74 1e           je     1f62 <_Z6struprPc+0x2a>
    while (*ptr)
  1f44: 80 3e 00        cmpb   $0x0,(%esi)
  1f47: 74 19           je     1f62 <_Z6struprPc+0x2a>
       *(ptr++) = toupper(*ptr);
  1f49: 83 ec 0c        sub    $0xc,%esp
  1f4c: 0f be 06        movsbl (%esi),%eax  // deref *ptr
  1f4f: 50              push   %eax
  1f50: 89 f3           mov    %esi,%ebx    // backup 'ptr' for later
  1f52: 46              inc    %esi         // inc 'ptr'
  1f53: e8 fc ff ff ff  call   1f54 <_Z6struprPc+0x1c>
  1f58: 88 03           mov    %al,(%ebx)   // store at pre-inc 'ptr'
  1f5a: 83 c4 10        add    $0x10,%esp
  1f5d: 80 3e 00        cmpb   $0x0,(%esi)
  1f60: 75 e7           jne    1f49 <_Z6struprPc+0x11>
}
  1f62: 8d 65 f8        lea    0xfffffff8(%ebp),%esp
  1f65: 5b              pop    %ebx
  1f66: 5e              pop    %esi
  1f67: 5d              pop    %ebp
  1f68: c3              ret
  1f69: 90              nop

  The key points in this snippet are:

  1f4c: load char from *ptr
  1f50: make temp copy of 'ptr' for result, simulate post-incr
  1f52: incr 'ptr' in place, not used again in this iteration
  1f58: save result using tmp 'ptr' value, prior to post-incr

  The generated code is correct.

------------------------------------

// ***********************************
// ***** compiled with -O6 : BUG *****
// ***********************************

00001ff0 <_Z6struprPc>:

void  strupr(char *ptr)
{
  1ff0: 55              push   %ebp
  1ff1: 89 e5           mov    %esp,%ebp
  1ff3: 56              push   %esi
  1ff4: 53              push   %ebx
  1ff5: 8b 75 08        mov    0x8(%ebp),%esi
if (ptr)
  1ff8: 85 f6           test   %esi,%esi
  1ffa: 74 05           je     2001 <_Z6struprPc+0x11>
    while (*ptr)
  1ffc: 80 3e 00        cmpb   $0x0,(%esi)
  1fff: 75 07           jne    2008 <_Z6struprPc+0x18>
       *(ptr++) = toupper(*ptr);
}
  2001: 8d 65 f8        lea    0xfffffff8(%ebp),%esp
  2004: 5b              pop    %ebx
  2005: 5e              pop    %esi
  2006: 5d              pop    %ebp
  2007: c3              ret
  2008: 89 f3           mov    %esi,%ebx   // make tmp copy of 'ptr'
  200a: 83 ec 0c        sub    $0xc,%esp
  200d: 46              inc    %esi        // incr ptr
  200e: 0f be 16        movsbl (%esi),%edx // deref 'ptr' AFTER incr
  2011: 52              push   %edx
  2012: e8 fc ff ff ff  call   2013 <_Z6struprPc+0x23>
  2017: 83 c4 10        add    $0x10,%esp
  201a: 88 03           mov    %al,(%ebx)
  201c: 80 3e 00        cmpb   $0x0,(%esi)
  201f: 75 e7           jne    2008 <_Z6struprPc+0x18>
  2021: eb de           jmp    2001 <_Z6struprPc+0x11>

  The key points in this snippet are:

  2008: make temp copy of 'ptr' for result, simulate post-incr
  200d: incr 'ptr' in place
        BELOW IS AN ERROR
  200e: load char from *ptr, but 'ptr' has already been incr
        ABOVE IS AN ERROR
  201a: save result using tmp 'ptr' value, prior to post-incr

  The problem with this snippet is that 'ptr' is derefed
after it was incr'd at 200d.  There are two solutions to
this error:

  1. use ebx as the src ptr at 200e instead of esi, which
     has already been 'post-incremented' early.
  2. move the 'movsbl' instr ahead of the 'incr' at
     200d.

---------------------------------------------

  Operations of the form *(ptr++) are fundamental to C/C++, and the
increment should be applied only after the assignment (pointer deref),
but it is apparently it is occurring too early...  I am somewhat
surprised this has gotten past the compiler verification test suites
that should be run during testing and prior to release.

  I may try to build the most recent gnu C/C++ from sources to
see if the problem is still present- I have not done this in a
few years; hope it goes well.

ron flory

Re: pointer post-incr problem with GCC 3.3.3 ??

[Graham Stott]

Hi,

>           *(ptr++) = toupper(*ptr);
> 
>   I would assert these two code frags are functionally identical.
>
Nope your assertion is wrong.

Read up on "sequence points" and you'll undersrand why. 

Graham 

Re: pointer post-incr problem with GCC 3.3.3 ??

[Falk Hueffner]

Graham Stott <graham.stott@btinternet.com> writes:

>>           *(ptr++) = toupper(*ptr);
>> 
>>   I would assert these two code frags are functionally identical.
>>
> Nope your assertion is wrong.
>
> Read up on "sequence points" and you'll undersrand why. 

I don't see this. Can you please explain in detail where the violation
is?

-- 
	Falk

Re: pointer post-incr problem with GCC 3.3.3 ??

[Falk Hueffner]

Falk Hueffner <hueffner@informatik.uni-tuebingen.de> writes:

> Graham Stott <graham.stott@btinternet.com> writes:
>
>>>           *(ptr++) = toupper(*ptr);
>>> 
>>>   I would assert these two code frags are functionally identical.
>>>
>> Nope your assertion is wrong.
>>
>> Read up on "sequence points" and you'll undersrand why. 
>
> I don't see this. Can you please explain in detail where the violation
> is?

Actually, I think I see it now. "ptr" has its value read before the
modification of "ptr", without an intervening sequence point, and not
only to determine the value to be stored in "ptr".

-- 
	Falk

Re: pointer post-incr problem with GCC 3.3.3 ??

[Andreas Schwab]

Falk Hueffner <hueffner@informatik.uni-tuebingen.de> writes:

> Graham Stott <graham.stott@btinternet.com> writes:
>
>>>           *(ptr++) = toupper(*ptr);
>>> 
>>>   I would assert these two code frags are functionally identical.
>>>
>> Nope your assertion is wrong.
>>
>> Read up on "sequence points" and you'll undersrand why. 
>
> I don't see this. Can you please explain in detail where the violation
> is?

Order of evaluation of operator arguments is unspecified, as well as the
exact point (before the next sequence point) when side effects take place.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."