[Bug tree-optimization/52430] New: [4.4 Regression] firefox miscompilation

[Bug tree-optimization/52430] New: [4.4 Regression] firefox miscompilation

[jakub at gcc dot gnu.org]

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

             Bug #: 52430
           Summary: [4.4 Regression] firefox miscompilation
    Classification: Unclassified
           Product: gcc
           Version: 4.4.6
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jakub@gcc.gnu.org
                CC: hubicka@gcc.gnu.org, jamborm@gcc.gnu.org


The following file is miscompiled on x86_64-linux with

-quiet -fPIC  -fno-rtti -pedantic -fno-exceptions -fstack-protector --param
ssp-buffer-size=4 -m64 -mtune=generic -fpermissive -fno-exceptions
-fno-strict-aliasing -fshort-wchar -ffunction-sections -fdata-sections -Os
-freorder-blocks -fomit-frame-pointer -fpreprocessed dombindings.ii

It was "fixed" or fixed for real, not clear, by a huge
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147852
commit which I think is unlikely backportable, at least not in full.

The problem seems to be in IPA-CP/IPA-* decisions, the growStorageBy method is
called in several places in this TU with constant 1, so IPA-CP decides to clone
things, but in the end clones just calculateNewCapacity (with implied
lengthInc=1), but doesn't clone growStorageBy, eventhough the call to
calculateNewCapacity from it call the clone that assumes lengthInc is 1.
For that TU this isn't a problem, but growStorageBy is public linkonce
function,
and when mixing it with other TUs that call growStorageBy with other
parameters, if this one wins, they ignore their last parameter and grow just by
1 instead of the desired amount.
but ends up actually cloning just

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[jakub at gcc dot gnu.org]

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-29 09:47:17 UTC ---
Created attachment 26779
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26779
dombindings.ii.bz2

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.4.7

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-02-29
         AssignedTo|unassigned at gcc dot       |jamborm at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #2 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-29 15:26:30 UTC ---
Mine, the problem is that ipcp_need_redirect_p does not redirect back
the problematic call because n_cloning_candidates == 0.  That seems
bogus, I'm investigating why it happens.

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[jamborm at gcc dot gnu.org]

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

--- Comment #3 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-29 16:45:04 UTC ---
Created attachment 26787
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26787
Proposed untested fix

n_cloning_candidates is zero because ipcp_initialize_node_lattices
thinks that growStorageBy does not need to be preserved because it
only checks node->needed, which is false.  In 4.5, we use
cgraph_only_called_directly_p for this purpose which also tests
node->local.externally_visible, which is what the attached patch,
currently under bootstrap and testing, does too.

With the patch at and -Os, we do not clone calculateNewCapacity and
the problem therefore does not occur.

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2012-03/msg00023.htm
                   |                            |l

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-03-01 12:57:24 UTC ---
Patch passed bootstrap and testing and I have posted it to the mailing list:

http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00023.html

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[jamborm at gcc dot gnu.org]

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

--- Comment #5 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-03-05 12:50:33 UTC ---
Author: jamborm
Date: Mon Mar  5 12:50:29 2012
New Revision: 184928

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184928
Log:
2012-03-05  Martin Jambor  <mjambor@suse.cz>

    PR tree-optimization/52430
    * ipa-cp.c (ipcp_initialize_node_lattices): Also consider
    node->local.externally_visible as needed.


Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/ipa-cp.c

[Bug tree-optimization/52430] [4.4 Regression] firefox miscompilation

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

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

--- Comment #6 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-03-05 16:05:23 UTC ---
Fixed.