* RFA: MIPS: Fix race condition causing PR 69129
@ 2016-01-19 13:52 Nick Clifton
2016-01-20 19:15 ` Matthias Klose
0 siblings, 1 reply; 3+ messages in thread
From: Nick Clifton @ 2016-01-19 13:52 UTC (permalink / raw)
To: clm, echristo, matthew.fortune; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]
Hi Catherine, Hi Eric, Hi Matthew,
GCC PR 69129 reports a problem with the MIPS backend:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69129
I traced the problem down to a race condition in
mips_compute_frame_info. This calls mips_global_pointer, which
through a torturous chain of inferior calls can end up with
mips_get_cprestore_base_and_offset trying to use the information in
the frame structure which has yet to be computed...
The attached patch fixes the problem by moving the initialisation of
the global_pointer field in the frame structure to after the args_size
and hard_frame_pointer_offset fields have been initialised.
Tested with no regressions on a mipsisa32-elf toolchain. (I know that
there are lots of different possible mips configurations. I was not
sure which one(s) I should test, so I chose one at random).
OK to apply ?
Cheers
Nick
gcc/ChangeLog
2016-01-19 Nick Clifton <nickc@redhat.com>
PR target/69129
* config/mips/mips.c (mips_compute_frame_info): Move the
initialisation of the global_pointer field to after the
initialisation of the hard_frame_pointer_offset and args_size
fields.
gcc/testsuite/ChangeLog
2016-01-19 Nick Clifton <nickc@redhat.com>
PR target/69129
* gcc.target/mips/pr69129.c: New testcase.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mips.c.patch --]
[-- Type: text/x-patch, Size: 3021 bytes --]
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c (revision 232560)
+++ gcc/config/mips/mips.c (working copy)
@@ -10347,8 +10347,6 @@
memset (frame, 0, sizeof (*frame));
size = get_frame_size ();
- cfun->machine->global_pointer = mips_global_pointer ();
-
/* The first two blocks contain the outgoing argument area and the $gp save
slot. This area isn't needed in leaf functions. We can also skip it
if we know that none of the called functions will use this space.
@@ -10375,6 +10373,26 @@
frame->args_size = crtl->outgoing_args_size;
frame->cprestore_size = MIPS_GP_SAVE_AREA_SIZE;
}
+
+ /* MIPS16 code offsets the frame pointer by the size of the outgoing
+ arguments. This tends to increase the chances of using unextended
+ instructions for local variables and incoming arguments. */
+ if (TARGET_MIPS16)
+ frame->hard_frame_pointer_offset = frame->args_size;
+
+ /* PR 69129: Beware of a possible race condition. mips_global_pointer
+ might call mips_cfun_has_inflexible_gp_ref_p which in turn can call
+ mips_find_gp_ref which will iterate over the current insn sequence.
+ If any of these insns use the cprestore_save_slot_operand or
+ cprestore_load_slot_operand predicates in order to be recognised then
+ they will call mips_cprestore_address_p which calls
+ mips_get_cprestore_base_and_offset which expects the frame information
+ to be filled in... In fact mips_get_cprestore_base_and_offset only
+ needs the args_size and hard_frame_pointer_offset fields to be filled
+ in, which is why the global_pointer field is initialised here and not
+ earlier. */
+ cfun->machine->global_pointer = mips_global_pointer ();
+
offset = frame->args_size + frame->cprestore_size;
/* Move above the local variables. */
@@ -10520,12 +10538,6 @@
frame->acc_save_offset = frame->acc_sp_offset - offset;
if (frame->num_cop0_regs > 0)
frame->cop0_save_offset = frame->cop0_sp_offset - offset;
-
- /* MIPS16 code offsets the frame pointer by the size of the outgoing
- arguments. This tends to increase the chances of using unextended
- instructions for local variables and incoming arguments. */
- if (TARGET_MIPS16)
- frame->hard_frame_pointer_offset = frame->args_size;
}
/* Return the style of GP load sequence that is being used for the
--- /dev/null 2016-01-19 08:08:42.474962807 +0000
+++ gcc/testsuite/gcc.target/mips/pr69129.c 2016-01-19 13:18:58.554155529 +0000
@@ -0,0 +1,29 @@
+_Noreturn void fn1 (int) __attribute__((__visibility__("hidden")));
+
+void
+fn2 (void *p1)
+{
+ int a[7];
+ float *b;
+ int c, n;
+
+ if (c != p1) /* { dg-warning "comparison between pointer and integer" } */
+ fn1 (1);
+
+ n = 0;
+ for (; c; n++)
+ {
+ int d;
+ if (a[n] != d)
+ fn1(n);
+ }
+
+ b = p1;
+
+ while (1)
+ {
+ *b = 3.40282347e38f;
+ if (a[0])
+ return;
+ }
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: RFA: MIPS: Fix race condition causing PR 69129
2016-01-19 13:52 RFA: MIPS: Fix race condition causing PR 69129 Nick Clifton
@ 2016-01-20 19:15 ` Matthias Klose
2016-01-21 14:11 ` Nick Clifton
0 siblings, 1 reply; 3+ messages in thread
From: Matthias Klose @ 2016-01-20 19:15 UTC (permalink / raw)
To: Nick Clifton, clm, echristo, matthew.fortune; +Cc: gcc-patches
On 19.01.2016 14:52, Nick Clifton wrote:
> Hi Catherine, Hi Eric, Hi Matthew,
>
> GCC PR 69129 reports a problem with the MIPS backend:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69129
>
> I traced the problem down to a race condition in
> mips_compute_frame_info. This calls mips_global_pointer, which
> through a torturous chain of inferior calls can end up with
> mips_get_cprestore_base_and_offset trying to use the information in
> the frame structure which has yet to be computed...
>
> The attached patch fixes the problem by moving the initialisation of
> the global_pointer field in the frame structure to after the args_size
> and hard_frame_pointer_offset fields have been initialised.
>
> Tested with no regressions on a mipsisa32-elf toolchain. (I know that
> there are lots of different possible mips configurations. I was not
> sure which one(s) I should test, so I chose one at random).
this fixes the bootstrap errors for me, seen in both libgnat and libgfortran.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: RFA: MIPS: Fix race condition causing PR 69129
2016-01-20 19:15 ` Matthias Klose
@ 2016-01-21 14:11 ` Nick Clifton
0 siblings, 0 replies; 3+ messages in thread
From: Nick Clifton @ 2016-01-21 14:11 UTC (permalink / raw)
To: Matthias Klose, clm, echristo, matthew.fortune; +Cc: gcc-patches
Hi Matthias,
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69129
> this fixes the bootstrap errors for me, seen in both libgnat and libgfortran.
Great - I have gone ahead and checked the patch in.
Cheers
Nick
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-21 14:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-19 13:52 RFA: MIPS: Fix race condition causing PR 69129 Nick Clifton
2016-01-20 19:15 ` Matthias Klose
2016-01-21 14:11 ` Nick Clifton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).