[Bug c++/53707] New: compiler generates wrong code

[Bug c++/53707] New: compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

             Bug #: 53707
           Summary: compiler generates wrong code
    Classification: Unclassified
           Product: gcc
           Version: 4.6.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: galtgendo@o2.pl


Using built-in specs.
COLLECT_GCC=gcc-4.6.3
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/i686-pc-linux-gnu/4.6.3/lto-wrapper
Target: i686-pc-linux-gnu
Configured with:
/mnt/workbox/builds/portage/sys-devel/gcc-4.6.3/work/gcc-4.6.3/configure
--prefix=/usr --bindir=/usr/i686-pc-linux-gnu/gcc-bin/4.6.3
--includedir=/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include
--datadir=/usr/share/gcc-data/i686-pc-linux-gnu/4.6.3
--mandir=/usr/share/gcc-data/i686-pc-linux-gnu/4.6.3/man
--infodir=/usr/share/gcc-data/i686-pc-linux-gnu/4.6.3/info
--with-gxx-include-dir=/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4
--host=i686-pc-linux-gnu --build=i686-pc-linux-gnu --disable-altivec
--disable-fixed-point --with-ppl --with-cloog --disable-ppl-version-check
--with-cloog-include=/usr/include/cloog-ppl --enable-lto --enable-nls
--without-included-gettext --with-system-zlib --disable-werror
--enable-secureplt --disable-multilib --enable-libmudflap --disable-libssp
--enable-libgomp
--with-python-dir=/share/gcc-data/i686-pc-linux-gnu/4.6.3/python
--enable-checking=release --disable-libgcj --with-arch=i686 --enable-objc-gc
--enable-languages=c,c++,objc,fortran --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-targets=all
--with-bugurl=http://bugs.gentoo.org/ --with-pkgversion='Gentoo 4.6.3 p1.3,
pie-0.5.1' --disable-libstdcxx-pch
Thread model: posix
gcc version 4.6.3 (Gentoo 4.6.3 p1.3, pie-0.5.1)
g++-4.6.3 -std=gnu++0x -O3 -fomit-frame-pointer -march=native -c tools.cpp -o
ui-tools.o
(without preprocessor switches, as they won't matter for preprocessed sources)
no warnings with the above line (though quite a few with -Wall, but not in the
affected part of code, it would seem)

The attached preprocessed sources produce *right* code, to get the *wrong*
code, remove 'string test(n);' (which should be a no-op there) from the
'for(unsigned n = 0; n < Codes; n++)' around line 54977.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #1 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-06-17 13:03:27 UTC ---
Created attachment 27642
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27642
-Wall warnings

Attaching '-Wall' warnings for completeness.

Note, that gcc 4.7.1 generates good code regardless of the presence of the
added line.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #2 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-06-17 13:05:20 UTC ---
Created attachment 27643
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27643
preprocessed sources triggering the bug

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

Rafał Mużyło <galtgendo at o2 dot pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27643|0                           |1
        is obsolete|                            |

--- Comment #3 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-06-17 13:08:59 UTC ---
Created attachment 27644
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27644
preprocessed sources triggering the bug (compressed)

[Bug c++/53707] compiler generates wrong code

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2012-06-18
      Known to work|                            |4.7.1
     Ever Confirmed|0                           |1
      Known to fail|                            |4.6.3

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-18 08:55:36 UTC ---
It's hard to analyze a miscompile with a compile-time only testcase.  Please
try to produce a testcase that can be linked and executed and that fails
when miscompiled and succeeds when not.

Also please try the 4.6.4 release.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #5 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-06-18 09:46:17 UTC ---
The wrong code here seems to be 'n' treated as a constant - there was a bug
(regarding freeciv, IIRC) of a similar case for an older gcc version.

As for gcc 4.6.4, when will it be released ?

[Bug c++/53707] compiler generates wrong code

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-18 09:58:04 UTC ---
(In reply to comment #5)
> The wrong code here seems to be 'n' treated as a constant - there was a bug
> (regarding freeciv, IIRC) of a similar case for an older gcc version.
> 
> As for gcc 4.6.4, when will it be released ?

GCC 4.6.4 is already released.

Btw, I see

private:
  enum : unsigned { Codes = 128 };
  struct Cheat {
    string code;
    string desc;
  };
  Cheat cheat[Codes];

and supposedly that array is being accessed.  I have no idea what the
'enum : unsigned { Codes = 128 }' syntax is about.  Is the bug going
away if you change that to use C++98 only features (like for example
using a const unsigned int Codes = 128;)

[Bug c++/53707] compiler generates wrong code

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-18 09:58:58 UTC ---
(In reply to comment #6)
> (In reply to comment #5)
> > The wrong code here seems to be 'n' treated as a constant - there was a bug
> > (regarding freeciv, IIRC) of a similar case for an older gcc version.
> > 
> > As for gcc 4.6.4, when will it be released ?
> 
> GCC 4.6.4 is already released.

Err no, my fault.  Looking at the wrong place.

[Bug c++/53707] compiler generates wrong code

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-18 10:04:36 UTC ---
Btw, when removing the 'string test(n)' line the function gets inlined and
eliminated, so that is not of much help.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #9 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-06-18 10:43:50 UTC ---
The code is not mine and it's about as convoluted (if not more) as freeciv was
(that was the *initial* part of bug 39333 - the upstream workaround was
http://svn.gna.org/viewcvs/freeciv/trunk/client/gui-gtk-2.0/wldlg.c?r1=13570&r2=13617).
Anyway, changing enum to static const doesn't help.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #10 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-06-18 11:13:20 UTC ---
Now, for an interesting note:
if instead of 'string test(n);' I put 'printf("%d\n", n);', not only the bug
*still* happens, but the result if funny:
first a '0' gets printed, but after that only '1' does.

So, it seems n++ of the loop is done only once.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #11 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-07-02 19:38:29 UTC ---
OK, I don't know if it's related, but 
https://bugs.gentoo.org/show_bug.cgi?id=424475
and its follow-up
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11845

This code:
int
find_stack_direction ()
{
  static char *addr = 0;
  auto char dummy;
  if (addr == 0)
    {
      addr = &dummy;
      return find_stack_direction ();
    }
  else
    return (&dummy > addr) ? 1 : -1;
}

int
main ()
{
  return find_stack_direction () < 0;
}

works with '-O2' and gcc 4.6.3, breaks on the same compiler with '-O3'
works with both '-O2' and '-O3' on 4.7.1.

On an interesting note, it works with both '-O2' and '-O3' on 4.5.3 too.

[Bug c++/53707] compiler generates wrong code

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #12 from Mikael Pettersson <mikpe at it dot uu.se> 2012-07-02 20:00:34 UTC ---
(In reply to comment #11)
> OK, I don't know if it's related, but 
> https://bugs.gentoo.org/show_bug.cgi?id=424475
> and its follow-up
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11845
> 
> This code:
> int
> find_stack_direction ()
> {
>   static char *addr = 0;
>   auto char dummy;
>   if (addr == 0)
>     {
>       addr = &dummy;
>       return find_stack_direction ();
>     }
>   else
>     return (&dummy > addr) ? 1 : -1;
> }
> 
> int
> main ()
> {
>   return find_stack_direction () < 0;
> }
> 
> works with '-O2' and gcc 4.6.3, breaks on the same compiler with '-O3'
> works with both '-O2' and '-O3' on 4.7.1.
> 
> On an interesting note, it works with both '-O2' and '-O3' on 4.5.3 too.

This one is not a C++ issue.  Please move it to a new PR.  I can reproduce it
with 4.6-20120629 on x86_64-linux, but not with 4.7-20120630 or 4.8-20120701.

[Bug c++/53707] compiler generates wrong code

[galtgendo at o2 dot pl]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #13 from Rafał Mużyło <galtgendo at o2 dot pl> 2012-07-02 20:25:12 UTC ---
As I said, it might not be related, but it's a bit odd, that both gcc versions
agree *and* it's also about inlining.

[Bug c++/53707] compiler generates wrong code

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-07-03 05:02:18 UTC ---
The http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707#c11 testcase is invalid.
In C (or C++) you can compare only pointers into the same object (or one past
the last byte of the object).
You need to cast the pointers to uintptr_t and do the comparisons in that type.

[Bug c++/53707] compiler generates wrong code

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #15 from Mikael Pettersson <mikpe at it dot uu.se> 2012-07-03 09:56:33 UTC ---
Created attachment 27734
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27734
cleaned up test case

This cleaned up version of the small C test case should be free of any
technical problems, I hope.  Gives the same result for -O2 and -O3 with 4.4.7,
4.5.4, 4.7.1, and 4.8.0 (all on x86_64-linux), but with 4.6.4 it returns
different results for -O2 and -O3.

Weirdly enough, changing the comparison operator from ">" to "<" makes 4.6.4
return the same result for both -O2 and -O3.

[Bug c++/53707] compiler generates wrong code

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-07-03 13:09:01 UTC ---
The code as written might be considered as undefined.

[Bug c++/53707] compiler generates wrong code

[schwab@linux-m68k.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #17 from Andreas Schwab <schwab@linux-m68k.org> 2012-07-03 14:05:06 UTC ---
When the call to find_stack_direction is inlined into itself there is no
requirement on how the two instances of here are allocated, so their addresses
have no defined relative order.

[Bug c++/53707] compiler generates wrong code

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

--- Comment #18 from Mikael Pettersson <mikpe at it dot uu.se> 2012-07-03 15:04:56 UTC ---
Right, I understood that the C test case invoked implementation-defined
behaviour, but inlining does invalidate any assumptions it might have made. 
Attaching an "__attribute__((__noinline__))" to find_stack_direction seems to
make it work also with gcc-4.6.4 -O3.

[Bug c++/53707] compiler generates wrong code

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53707

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |INVALID

--- Comment #19 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-04 09:22:10 UTC ---
Invalid.