* [PATCH] combine: Fix PR80233
@ 2017-03-29 18:47 Segher Boessenkool
2017-03-29 20:44 ` Jeff Law
0 siblings, 1 reply; 5+ messages in thread
From: Segher Boessenkool @ 2017-03-29 18:47 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool, jakub
If combine has added an unconditional trap there will be a new basic
block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK
as the last_combined_insn, but then it tries to take the DF_INSN_LUID
of that and that dereferences a NULL pointer (since such a note is not
an INSN_P).
This fixes it by not taking non-insns as last_combined_insn.
Bootstrapped and tested on powerpc64-linux {-m32,-m64}.
Segher
2017-03-29 Segher Boessenkool <segher@kernel.crashing.org>
PR rtl-optimization/80233
* combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns
as last_combined_insn. Do not test for BARRIER_P separately.
gcc/testsuite/
PR rtl-optimization/80233
* gcc.c-torture/compile/pr80233.c: New testcase.
---
gcc/combine.c | 4 ++--
gcc/testsuite/gcc.c-torture/compile/pr80233.c | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr80233.c
diff --git a/gcc/combine.c b/gcc/combine.c
index 87e9670..88ac475 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1250,10 +1250,10 @@ combine_instructions (rtx_insn *f, unsigned int nregs)
continue;
while (last_combined_insn
- && last_combined_insn->deleted ())
+ && (!NONDEBUG_INSN_P (last_combined_insn)
+ || last_combined_insn->deleted ()))
last_combined_insn = PREV_INSN (last_combined_insn);
if (last_combined_insn == NULL_RTX
- || BARRIER_P (last_combined_insn)
|| BLOCK_FOR_INSN (last_combined_insn) != this_basic_block
|| DF_INSN_LUID (last_combined_insn) <= DF_INSN_LUID (insn))
last_combined_insn = insn;
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr80233.c b/gcc/testsuite/gcc.c-torture/compile/pr80233.c
new file mode 100644
index 0000000..eb9b4a4
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr80233.c
@@ -0,0 +1,22 @@
+/* PR rtl-optimization/80233 */
+
+int xg;
+
+void
+t4 (int o9)
+{
+ int it;
+
+ if (o9 == 0)
+ {
+ int fx;
+
+ xg *= it;
+ if (xg == 0)
+ it /= 0;
+
+ fx = (it != 0) ? (xg < 0) : (xg / o9);
+ if (fx != 0)
+ xg = 0;
+ }
+}
--
1.9.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combine: Fix PR80233
2017-03-29 18:47 [PATCH] combine: Fix PR80233 Segher Boessenkool
@ 2017-03-29 20:44 ` Jeff Law
2017-03-29 20:48 ` Jakub Jelinek
2017-03-29 20:56 ` Segher Boessenkool
0 siblings, 2 replies; 5+ messages in thread
From: Jeff Law @ 2017-03-29 20:44 UTC (permalink / raw)
To: Segher Boessenkool, gcc-patches; +Cc: jakub
On 03/29/2017 12:23 PM, Segher Boessenkool wrote:
> If combine has added an unconditional trap there will be a new basic
> block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK
> as the last_combined_insn, but then it tries to take the DF_INSN_LUID
> of that and that dereferences a NULL pointer (since such a note is not
> an INSN_P).
>
> This fixes it by not taking non-insns as last_combined_insn.
>
> Bootstrapped and tested on powerpc64-linux {-m32,-m64}.
>
>
> Segher
>
>
> 2017-03-29 Segher Boessenkool <segher@kernel.crashing.org>
>
> PR rtl-optimization/80233
> * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns
> as last_combined_insn. Do not test for BARRIER_P separately.
>
> gcc/testsuite/
> PR rtl-optimization/80233
> * gcc.c-torture/compile/pr80233.c: New testcase.
No strong opinions on this vs Jakub's patch. I guess yours may walk
more objects on the chain, but in doing so is more likely to find a
useful LAST_COMBINED_INSN. Jakub's stops earlier, but is less likely to
have stopped on something useful.
Your call Segher.
jeff
ps. Never in a million years would I have expected isolation of
division by zero to have exposed as many latent issues as it has. Sigh.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combine: Fix PR80233
2017-03-29 20:44 ` Jeff Law
@ 2017-03-29 20:48 ` Jakub Jelinek
2017-03-29 20:49 ` Jeff Law
2017-03-29 20:56 ` Segher Boessenkool
1 sibling, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2017-03-29 20:48 UTC (permalink / raw)
To: Jeff Law; +Cc: Segher Boessenkool, gcc-patches
On Wed, Mar 29, 2017 at 02:35:32PM -0600, Jeff Law wrote:
> On 03/29/2017 12:23 PM, Segher Boessenkool wrote:
> > If combine has added an unconditional trap there will be a new basic
> > block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK
> > as the last_combined_insn, but then it tries to take the DF_INSN_LUID
> > of that and that dereferences a NULL pointer (since such a note is not
> > an INSN_P).
> >
> > This fixes it by not taking non-insns as last_combined_insn.
> >
> > Bootstrapped and tested on powerpc64-linux {-m32,-m64}.
> >
> >
> > Segher
> >
> >
> > 2017-03-29 Segher Boessenkool <segher@kernel.crashing.org>
> >
> > PR rtl-optimization/80233
> > * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns
> > as last_combined_insn. Do not test for BARRIER_P separately.
> >
> > gcc/testsuite/
> > PR rtl-optimization/80233
> > * gcc.c-torture/compile/pr80233.c: New testcase.
> No strong opinions on this vs Jakub's patch. I guess yours may walk more
> objects on the chain, but in doing so is more likely to find a useful
> LAST_COMBINED_INSN. Jakub's stops earlier, but is less likely to have
> stopped on something useful.
>
> Your call Segher.
I like Segher's latest patch. But it is his call anyway ;)
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combine: Fix PR80233
2017-03-29 20:48 ` Jakub Jelinek
@ 2017-03-29 20:49 ` Jeff Law
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2017-03-29 20:49 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Segher Boessenkool, gcc-patches
On 03/29/2017 02:44 PM, Jakub Jelinek wrote:
> On Wed, Mar 29, 2017 at 02:35:32PM -0600, Jeff Law wrote:
>> On 03/29/2017 12:23 PM, Segher Boessenkool wrote:
>>> If combine has added an unconditional trap there will be a new basic
>>> block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK
>>> as the last_combined_insn, but then it tries to take the DF_INSN_LUID
>>> of that and that dereferences a NULL pointer (since such a note is not
>>> an INSN_P).
>>>
>>> This fixes it by not taking non-insns as last_combined_insn.
>>>
>>> Bootstrapped and tested on powerpc64-linux {-m32,-m64}.
>>>
>>>
>>> Segher
>>>
>>>
>>> 2017-03-29 Segher Boessenkool <segher@kernel.crashing.org>
>>>
>>> PR rtl-optimization/80233
>>> * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns
>>> as last_combined_insn. Do not test for BARRIER_P separately.
>>>
>>> gcc/testsuite/
>>> PR rtl-optimization/80233
>>> * gcc.c-torture/compile/pr80233.c: New testcase.
>> No strong opinions on this vs Jakub's patch. I guess yours may walk more
>> objects on the chain, but in doing so is more likely to find a useful
>> LAST_COMBINED_INSN. Jakub's stops earlier, but is less likely to have
>> stopped on something useful.
>>
>> Your call Segher.
>
> I like Segher's latest patch. But it is his call anyway ;)
In that case let's just go with Segher's patch :-)
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combine: Fix PR80233
2017-03-29 20:44 ` Jeff Law
2017-03-29 20:48 ` Jakub Jelinek
@ 2017-03-29 20:56 ` Segher Boessenkool
1 sibling, 0 replies; 5+ messages in thread
From: Segher Boessenkool @ 2017-03-29 20:56 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches, jakub
On Wed, Mar 29, 2017 at 02:35:32PM -0600, Jeff Law wrote:
> ps. Never in a million years would I have expected isolation of
> division by zero to have exposed as many latent issues as it has. Sigh.
The trap insns weren't handled very well before, but we didn't generate
many either. They still aren't handled very well, but hopefully we don't
ICE so much on them anymore ;-)
Segher
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-29 20:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-29 18:47 [PATCH] combine: Fix PR80233 Segher Boessenkool
2017-03-29 20:44 ` Jeff Law
2017-03-29 20:48 ` Jakub Jelinek
2017-03-29 20:49 ` Jeff Law
2017-03-29 20:56 ` Segher Boessenkool
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).