Bug 36862 - -O2 creates bad code with different target architectures
Summary: -O2 creates bad code with different target architectures
Alias: None
Product: Red Hat Linux
Classification: Retired
Component: gcc   
(Show other bugs)
Version: 7.1
Hardware: i386
OS: Linux
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: David Lawrence
URL: http://sourceforge.net/projects/sidplay2
Depends On:
TreeView+ depends on / blocked
Reported: 2001-04-20 17:26 UTC by Michael Schwendt
Modified: 2007-04-18 16:32 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2001-04-20 17:27:37 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
PSID.ii (86.20 KB, text/plain)
2001-04-20 17:27 UTC, Michael Schwendt
no flags Details

Description Michael Schwendt 2001-04-20 17:26:30 UTC
From Bugzilla Helper:
User-Agent: Mozilla/4.77 [en] (X11; U; Linux 2.4.2-2 i686; Nav)

$ gcc -v
Reading specs from /usr/lib/gcc-lib/i386-redhat-linux/2.96/specs
gcc version 2.96 20000731 (Red Hat Linux 7.1 2.96-81)

is generating bad code depending on which target architecture and level of
optimization is used.

Discovered with the latest release of:

When building the three i386 packages with

  rpm --recompile

the resulting player executable is unable to detect input files and throws
an error. When building three i586 packages with

  rpm --recompile --target=i586

the player loads input files without errors but doesn't play any sound
(audio output is zero except for a few milli-secs).


With -O0 the code compiles without errors.


I've managed to find a first short file that does not build correctly with
-O2. It's 193 lines short (PSID.cpp), its *.ii version is 3720 lines.

Almost the same code is in libsidplay-1, too, and builds fine there with
the default RPM_OPT_FLAGS. I see the author has converted to use AC99 types
(uint_least8_t, uint_least16_t, etc.) in many places.

Also, the code does nothing more than examining a file buffer in memory,
checking a 32-bit magic number in the first four octets, and reading
several big-endian values and character strings. It makes use of a few
inline (!) functions for big/little-endian conversion.


Clear case.

ESI register points to beginning of memory buffer which contains:
0x50, 0x53, 0x49, 0x44 (= 'P','S','I','D')

    jbe .L4
    movb    3(%esi), %al                ; big-endian conversion
    movb    (%esi), %bl                 ; read four single bytes
    movb    1(%esi), %cl                ; 3(ESI)
    movb    2(%esi), %dl

     ; at this point we have
     ; AL = 0x44
     ; BL = 0x50
     ; CL = 0x53
     ; DL = 0x49

    movb    %dl, -15(%ebp)
    movb    %al, -16(%ebp)
    movb    %cl, -18(%ebp)
    movb    %bl, -17(%ebp)

     ; at this point we have
     ; -18(EBP) = 0x53, 0x50, 0x44, 0x49 (= 0x49445053)
     ; ought to be:  0x44, 0x49, 0x53, 0x50 (= 0x50534944)
     ; the current implementation of big-endian conversion
     ; consists of heavily inlined functions that swap 16-bit
     ; words and 32-bit dwords
     ; the -O2 optimizer clearly messed up the 16-bit low- and
     ; high-word

    movw    -18(%ebp), %ax
    cmpl    $1347635524, -16(%ebp)      ; = 0x50534944 = "PSID" ?
    movw    %ax, -14(%ebp)
    jne .L4

    ; the comparison fails, because the value is 1229213779

    leal    4(%esi), %eax
    movl    %eax, -48(%ebp)
    movb    4(%esi), %dl
    movb    1(%eax), %al
    movb    %al, -20(%ebp)
    movb    %dl, -19(%ebp)
    cmpw    $2, -20(%ebp)
    jbe .L3
    movl    $_sidtune_unknown, (%edi)

Reproducible: Always
Steps to Reproduce:
See description.  Depending on rpm's --target=XYZ  argument , the compiler
produces different code for some other (yet unknown) file.

1. cd libsidplay-2.0.7 ; CXXFLAGS="-O0 -Wall -s -v -save-temps" configure
2. cd src/sidtune ; make clean ; make PSID.o
3. (the same with -O2 ......)

Comment 1 Michael Schwendt 2001-04-20 17:27:33 UTC
Created attachment 15884 [details]

Comment 2 Jakub Jelinek 2001-04-24 15:26:59 UTC
The source looks like huge pile of type-punning, see `info gcc' on
Compiling with -fno-strict-aliasing should help here, although fixing the
source would be much better.
Particularly e.g. with
inline void endian_32hi16 (unsigned int &dword, unsigned short word)
  ((unsigned short *) &dword)[1] = word;
compiler is free to give you on next access to dword either dword with second
16bits equal to word or unmodified dword.

Note You need to log in before you can comment on or make changes to this bug.