See https://bugzilla.mozilla.org/show_bug.cgi?id=594611 and https://bugzilla.mozilla.org/show_bug.cgi?id=590683 for more details. This breaks users of Firefox Sync on GCC 4.5. The bug isn't present in gcc 4.4 or trunk. What would it take to cherry-pick a fix for 4.5.x?
FWIW our libffi is basically libffi git head: http://github.com/atgreen/libffi Which is regularly synced to gcc libffi.
There have been no ABI changes in 4.5 that I know of for PowerPC64 or even differences between the trunk and 4.5.
Mozilla bugs say "Platform: x86 Linux". But gcc bug says "powerpc64-*-linux". What is going on?
This is on x86_64. (I can't change the field, though. Can someone else?)
I am not ware any x86-64 psABI changes in gcc 4.5. Please provide a testcase.
(In reply to comment #3) > Mozilla bugs say "Platform: x86 Linux". But gcc bug says > "powerpc64-*-linux". What is going on? I must have missed since I saw Linux64 I was thinking powerpc64 :). Really there have been none x86_64 ones either. Though the normal thing here that might happen is strict aliasing issues. Can you try -fno-strict-aliasing. Also maybe look for buffer overflows which might cause issues you think are compiler related.
-fno-strict-aliasing makes no difference.
(In reply to comment #0) > See https://bugzilla.mozilla.org/show_bug.cgi?id=594611 and > https://bugzilla.mozilla.org/show_bug.cgi?id=590683 > for more details. This breaks users of Firefox Sync on GCC 4.5. > The bug isn't present in gcc 4.4 or trunk. What would it take to cherry-pick a > fix for 4.5.x? > You either identify which checkin fixes it or find a testcase so that I can use it to find the fix.
Created attachment 21798 [details] Reduced testcase Both issues Taras mentioned are actually separated. One is an actual bug in ffi (to be filed), the other one is an optimization issue with gcc. I reduced the problematic code to the attached code, which prints "foo" with -O1 (and more), and "bar" with -O0, with gcc 4.5.1. gcc 4.4 compiled code correctly prints "bar" with any optimization level.
Please note this actually only happens on x86. (I would change the summary and target if I could)
Created attachment 21799 [details] Reduced testcase Inlining JSVAL_TO_PRIVATE by hand still makes it break, and reduces the testcase further.
FWIW, it's still broken on a gcc trunk snapshot from the 28th of august.
Confirmed and investigating.
You are accessing a pointer of type char *s1 via an lvalue of type void * (*data). Or speaking in C++, you are accessing an object of dynamic type void * (stored to via *data) by an lvalue of type char * (s1). Thus your testcase invokes undefined behavior. That it is miscompiled at -O1 is a bug. With GCC 4.6 we now assign the same alias-set to all pointers, hiding this issue. data_4 is a non-pointer variable,ignoring constraint:*data_4 = s2.1_5 data_4, points-to vars: { } oops. I will have a look at the points-to bug.
Note that the original code doesn't use char *. I used char * to make it easily visible with a printf. Actually, just writing void foo(jsval_layout l, void *s2) { jsval_layout m; m.asBits = l.asBits; void ** data = (void**)m.ptr; *data = s2; } exhibits the problem, afaics.
The real code where this gets problematic: http://mxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#5615 The function it calls: http://mxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#5542 http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.h#281 http://mxr.mozilla.org/mozilla-central/source/js/src/jsval.h#799 http://mxr.mozilla.org/mozilla-central/source/js/src/jsval.h#506
points-to analysis does not honor GCCs type-punning through union extension (it works on x86_64 because ptr and asBits match in size and thus SRA cleans the code up before pointer-analysis). So PTA sees ss1.0_1 = ss1; l.ptr = ss1.0_1; D.3244_2 = l.asBits; m.asBits = D.3244_2; D.3245_3 = m.ptr; data_4 = (void * *) D.3245_3; and it considers both D.3244_2 = l.asBits and m.asBits = D.3244_2 as irrelevant (as they do not involve pointers). Thus, m.ptr is never assigned to and the points-to set of data_4 ends up as empty which makes us remove the store *data_4 = s2.1_5. Thus, as a workaround you should make sure the asBits field matches pointer-size (so for example use uintptr_t isntead of uint64_t).
(In reply to comment #17) > Thus, as a workaround you should make sure the asBits field matches > pointer-size (so for example use uintptr_t isntead of uint64_t). which is not possible in the original code, as the union is a bit more complicated than in the reduced testcase: http://mxr.mozilla.org/mozilla-central/source/js/src/jsval.h#274
Another workaround is to use -fno-tree-pta.
(In reply to comment #19) > Another workaround is to use -fno-tree-pta. Doesn't work here.
(In reply to comment #20) > (In reply to comment #19) > > Another workaround is to use -fno-tree-pta. > > Doesn't work here. For the original code? Then your reduced testcase is different from the original problem.
(In reply to comment #21) > For the original code? Then your reduced testcase is different from the > original problem. It doesn't work for the reduced testcase here, with gcc 4.5.1
Subject: Re: [4.5/4.6 Regression] GCC 4.5.[01] breaks our ffi on Linux64. ABI break? On Wed, 15 Sep 2010, mh+gcc at glandium dot org wrote: > ------- Comment #22 from mh+gcc at glandium dot org 2010-09-15 13:52 ------- > (In reply to comment #21) > > For the original code? Then your reduced testcase is different from the > > original problem. > > It doesn't work for the reduced testcase here, with gcc 4.5.1 It does fix the PTA problem (thus makes it work at -O1). Still fails at -O2 for some reason (but can't reproduce that on the tip of the branch, only with the 4.5.1 release). Alias-correct testcase: #include <stdint.h> extern void abort (void); char *s1 = "foo"; char *s2 = "bar"; char **ss1 = &s1; typedef union jsval_layout { uint64_t asBits; char **ptr; } jsval_layout; int main() { jsval_layout l, m; l.ptr = ss1; m.asBits = l.asBits; char ** data = m.ptr; *data = s2; if (s1 != s2) abort (); return 0; }
Created attachment 21801 [details] patch I am testing this patch (for 4.5 branch).
Oh, I was trying with -O2, yes, it works with -O1 -fno-tree-pta. Let me try on the original code, too.
Subject: Bug 45623 Author: rguenth Date: Thu Sep 16 11:06:25 2010 New Revision: 164333 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164333 Log: 2010-09-16 Richard Guenther <rguenther@suse.de> PR tree-optimization/45623 * tree-ssa-structalias.c (get_constraint_for_ptr_offset): Adjust. (get_constraint_for_component_ref): If computing a constraint for the rhs handle type punning through unions. (get_constraint_for_address_of): Adjust. (get_constraint_for_1): Likewise. (get_constraint_for): Likewise. (get_constraint_for_rhs): New function. (do_structure_copy): Adjust. (make_constraint_to): Likewise. (handle_const_call): Likewise. (find_func_aliases): Likewise. (process_ipa_clobber): Likewise. (create_variable_info_for): Likewise. * gcc.dg/torture/pr45623.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/torture/pr45623.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-structalias.c
Fixed for trunk sofar.
(In reply to comment #27) > Fixed for trunk sofar. Is there an eta for 4.5 backport? We need this to switch Firefox to 4.5 on Linux.
(In reply to comment #28) > (In reply to comment #27) > > Fixed for trunk sofar. > > Is there an eta for 4.5 backport? We need this to switch Firefox to 4.5 on > Linux. I just want to play safe and see if there is any fallout on trunk. You can use the attachment in comment #24 for the 4.5 branch (and I'd appreciate testing if this fixes your original problem)
Fixed.
Subject: Bug 45623 Author: rguenth Date: Mon Sep 20 08:33:46 2010 New Revision: 164430 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164430 Log: 2010-09-20 Richard Guenther <rguenther@suse.de> PR tree-optimization/45623 * tree-ssa-structalias.c (get_constraint_for_ptr_offset): Adjust. (get_constraint_for_component_ref): If computing a constraint for the rhs handle type punning through unions. (get_constraint_for_address_of): Adjust. (get_constraint_for_1): Likewise. (get_constraint_for): Likewise. (get_constraint_for_rhs): New function. (do_structure_copy): Adjust. (make_constraint_to): Likewise. (handle_const_call): Likewise. (find_func_aliases): Likewise. * gcc.dg/torture/pr45623.c: New testcase. Added: branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/torture/pr45623.c Modified: branches/gcc-4_5-branch/gcc/ChangeLog branches/gcc-4_5-branch/gcc/testsuite/ChangeLog branches/gcc-4_5-branch/gcc/tree-ssa-structalias.c
*** Bug 47712 has been marked as a duplicate of this bug. ***