[Bug c/52734] New: Incorrect optimization of uClibc sbrk()

[Bug c/52734] New: Incorrect optimization of uClibc sbrk()

[michael at talamasca dot ocis.net]

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

             Bug #: 52734
           Summary: Incorrect optimization of uClibc sbrk()
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: michael@talamasca.ocis.net
              Host: i386-pc-linux-gnu
            Target: i386-pc-linux-gnu
             Build: i386-pc-linux-gnu


Created attachment 27005
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27005
Simplified code analogous to the sbrk problem

GCC 4.7.0 miscompiles uClibc 0.9.33 producing an unusable library.  The problem
is in uClibc's sbrk() implementation -- GCC optimizes away sbrk's attempt to
save a global variable before a called function changes it.  Basically, that
function (brk) is wrongly treated as an __attribute__((pure)) function.

The above text is to help anyone searching for the bug.  I'm attaching a
distilled case showing the same problem.  The function ccc() is supposed to
return the value of bbb before the second call to aaa(), but when compiled with
-Os it returns the value of bbb afterwards.

This is on i386-pc-linux-uclibc.  GCC 4.6.3 did not have the problem.

[Bug c/52734] Incorrect optimization of uClibc sbrk()

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2012-03-27
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-27 07:35:27 UTC ---
The testcase does not reproduce the issue for me on i?86-linux.  Are you
sure the testcase reproduces the issue for you?  Can you attach the wrong
assembly that is produced and the output of the compiler command producing
it when you append -v to it?

[Bug c/52734] Incorrect optimization of uClibc sbrk()

[mikpe at it dot uu.se]

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

--- Comment #2 from Mikael Pettersson <mikpe at it dot uu.se> 2012-03-27 07:42:46 UTC ---
Created attachment 27008
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27008
adjusted test case

This test case (adjusted to __builtin_abort() on error) fails with
gcc-4.7-20120324 with -Os/-O2/-O3 on x86_64-pc-linux-gnu, but works with
-O0/-O1, and works with gcc-4.6.3 and any -O option.

[Bug c/52734] Incorrect optimization of uClibc sbrk()

[mikpe at it dot uu.se]

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

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

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

--- Comment #3 from Mikael Pettersson <mikpe at it dot uu.se> 2012-03-27 07:45:49 UTC ---
Here's the bogus assembly code for ccc() with gcc-4.7 and -Os:

        .globl  ccc
        .type   ccc, @function
ccc:
.LFB1:
        .cfi_startproc
        call    aaa
        testl   %eax, %eax
        je      .L3
.L5:
        movl    bbb(%rip), %eax
        ret
.L3:
        call    aaa
        testl   %eax, %eax
        je      .L5
        xorl    %eax, %eax
        ret
        .cfi_endproc
.LFE1:
        .size   ccc, .-ccc

Note how it fails to read bbb into a local before the second call to aaa.

[Bug c/52734] Incorrect optimization of uClibc sbrk()

[mikpe at it dot uu.se]

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

--- Comment #4 from Mikael Pettersson <mikpe at it dot uu.se> 2012-03-27 07:53:52 UTC ---
Not x86-specific, gcc-4.7 fails on arm-linux-gnueabi and m68k-linux too.

[Bug c/52734] Incorrect optimization of uClibc sbrk()

[michael at talamasca dot ocis.net]

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

--- Comment #5 from michael at talamasca dot ocis.net <michael at talamasca dot ocis.net> 2012-03-27 07:57:02 UTC ---
Created attachment 27009
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27009
Typescript of Michael running Mikael's version of the testcase

[Bug c/52734] Incorrect optimization of uClibc sbrk()

[michael at talamasca dot ocis.net]

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

--- Comment #6 from michael at talamasca dot ocis.net <michael at talamasca dot ocis.net> 2012-03-27 08:05:06 UTC ---
Created attachment 27011
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27011
Assembly output of Mikael's testcase, from Michael's computer

Here's the assembly output you requested.  The "-v" data is in the typescript
file added separately.

I've used the modified version of the case submitted by Mikael, rather than my
original file.

Notice that "bbb" appears in the assembly only twice.  The first time is the
increment in Mikael's aaa implementation, and the second is on a code path that
goes straight to the end of the function.  Nowhere in the code is the load of
ddd from bbb.

[Bug rtl-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|WAITING                     |NEW
          Component|c                           |rtl-optimization
      Known to work|                            |4.6.3
   Target Milestone|---                         |4.7.1
            Summary|Incorrect optimization of   |[4.7/4.8 Regression]
                   |uClibc sbrk()               |Incorrect optimization of
                   |                            |uClibc sbrk()

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-27 08:24:27 UTC ---
Confirmed.

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tom at codesourcery dot com
          Component|rtl-optimization            |tree-optimization

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-27 09:21:51 UTC ---
Caused by tail merging.  -fno-tree-tail-merge fixes it.

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

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

--- Comment #9 from vries at gcc dot gnu.org 2012-03-27 13:05:09 UTC ---
Blocks 3 and 5, with successor 6 are considered equal and merged.
...
  # BLOCK 3 freq:6102
  # PRED: 2 [61.0%]  (true,exec)
  # VUSE <.MEMD.1734_10>
  dddD.1710_3 = bbbD.1703;
  goto <bb 6>;
  # SUCC: 6 [100.0%]  (fallthru,exec)

  # BLOCK 5 freq:2378
  # PRED: 4 [61.0%]  (false,exec)
  # SUCC: 6 [100.0%]  (fallthru,exec)

  # BLOCK 6 freq:10000
  # PRED: 3 [100.0%]  (fallthru,exec) 7 [100.0%]  (fallthru) 5 [100.0%] 
(fallthru,exec)
  # dddD.1710_1 = PHI <dddD.1710_3(3), 0(7), dddD.1710_4(5)>
  # .MEMD.1734_8 = PHI <.MEMD.1734_10(3), .MEMD.1734_11(7), .MEMD.1734_11(5)>
  # VUSE <.MEMD.1734_8>
  return dddD.1710_1;
  # SUCC: EXIT [100.0%] 
...

Tail merge considers 2 blocks equal if the effect at the tail is equal,
meaning:
- the sequence of side effects produced by each block is equal
- the value phis are equal

There are no side effects in block 3 and 5, and the phi alternatives of
dddD.1710_1 for 3 (dddD.1710_3)  and 5 (dddD.1710_4)  are proven equal by gvn.

The problem is that changing the (4->5) edge into a (4->3) edge changes the
value of dddD.1710_3, because block 4 contains a store that affects the load in
block 3.
...
  # BLOCK 4 freq:3898
  # PRED: 2 [39.0%]  (false,exec)
  # VUSE <.MEMD.1734_10>
  dddD.1710_4 = bbbD.1703;
  # .MEMD.1734_11 = VDEF <.MEMD.1734_10>
  # USE = nonlocal null 
  # CLB = nonlocal null 
  D.1724_5 = aaaD.1705 ();
  if (D.1724_5 != 0)
    goto <bb 7>;
  else
    goto <bb 5>;
  # SUCC: 7 [39.0%]  (true,exec) 5 [61.0%]  (false,exec)
...

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[vries at gcc dot gnu.org]

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

--- Comment #10 from vries at gcc dot gnu.org 2012-03-27 13:19:12 UTC ---
Created attachment 27014
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27014
Tentative patch

The key element of this patch is:
...
+  if (gvn_used && vuse1 != vuse2)
+    return;
+
...

If gvn was use to prove two bbs equal, and the incoming vuses are not equal,
don't do the substitution. This is a bit too restrictive, I'm pondering about a
more precise fix.

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[vries at gcc dot gnu.org]

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

--- Comment #11 from vries at gcc dot gnu.org 2012-03-27 17:04:54 UTC ---
> Created attachment 27014 [details]
> Tentative patch

Bootstrapped and reg-tested on x86_64, no issues found.

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[joerg.jungermann at gmx dot de]

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

joerg.jungermann at gmx dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joerg.jungermann at gmx dot
                   |                            |de

--- Comment #12 from joerg.jungermann at gmx dot de 2012-04-10 22:40:41 UTC ---
We at Freetz[1] had serveral problems compiling a running uclibc 0.9.3[23] with
gcc 4.7.0. -fno-tree-tail-merge did fix this, so I applied the patch from #c10.

[1] http://freetz.org/ticket/1752

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[sedat.dilek at gmail dot com]

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

--- Comment #13 from Sedat Dilek <sedat.dilek at gmail dot com> 2012-04-10 22:57:15 UTC ---
Just some remarks:

Freetz is an OSS project supporting router (platforms) of a popular German
company. The target-system is mostly MIPS(EL). The majority of our developers
build on AMD64 hosts (32bit-toolchain).
The full patchset for gcc-4.7.0 can be found in [1].
Jörg provided his adapted/backported patch in [2].
Thank you for testing!

[0] http://freetz.org/
[1] http://freetz.org/browser/trunk/toolchain/make/target/gcc/4.7.0/
[2] http://freetz.org/attachment/ticket/1752/r8889-gcc-4.7-fix.patch

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[rguenth at gcc dot gnu.org]

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

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-11 10:12:24 UTC ---
(In reply to comment #11)
> > Created attachment 27014 [details]
> > Tentative patch
> 
> Bootstrapped and reg-tested on x86_64, no issues found.

Can you post it for review then?

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[vries at gcc dot gnu.org]

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

--- Comment #15 from vries at gcc dot gnu.org 2012-04-12 16:25:28 UTC ---
Created attachment 27142
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27142
pr52734.2.patch

Another tentative patch. Also not precise, but a simpler approach.

Now testing.

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #16 from vries at gcc dot gnu.org 2012-04-13 19:32:27 UTC ---
Fixed in trunk: r186418
Fixed in 4.7 branch: r186424

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[sedat.dilek at gmail dot com]

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

--- Comment #17 from Sedat Dilek <sedat.dilek at gmail dot com> 2012-04-13 19:38:24 UTC ---
Thanks, Tom!

[Bug tree-optimization/52734] [4.7/4.8 Regression] Incorrect optimization of uClibc sbrk()

[jackie.rosen at hushmail dot com]

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

Jackie Rosen <jackie.rosen at hushmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jackie.rosen at hushmail dot com

--- Comment #18 from Jackie Rosen <jackie.rosen at hushmail dot com> ---
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.