Bug 52430 - [4.4 Regression] firefox miscompilation
Summary: [4.4 Regression] firefox miscompilation
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.4.6
: P3 normal
Target Milestone: 4.4.7
Assignee: Martin Jambor
URL: http://gcc.gnu.org/ml/gcc-patches/201...
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-02-29 09:44 UTC by Jakub Jelinek
Modified: 2012-03-05 16:05 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-02-29 00:00:00


Attachments
dombindings.ii.bz2 (371.06 KB, application/octet-stream)
2012-02-29 09:47 UTC, Jakub Jelinek
Details
Proposed untested fix (403 bytes, patch)
2012-02-29 16:45 UTC, Martin Jambor
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2012-02-29 09:44:17 UTC
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
Comment 1 Jakub Jelinek 2012-02-29 09:47:17 UTC
Created attachment 26779 [details]
dombindings.ii.bz2
Comment 2 Martin Jambor 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.
Comment 3 Martin Jambor 2012-02-29 16:45:04 UTC
Created attachment 26787 [details]
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.
Comment 4 Martin Jambor 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
Comment 5 Martin Jambor 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
Comment 6 Martin Jambor 2012-03-05 16:05:23 UTC
Fixed.