* [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix")
@ 2019-03-21 10:13 Richard Biener
2019-03-21 14:04 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2019-03-21 10:13 UTC (permalink / raw)
To: gcc-patches
This also avoids the ICE in PR89779 but IMHO is not a real fix.
Still it restores a previously active check against released SSA names
which now have error_mark_node type rather than NULL. The new way
opens up consolidation so I've adjusted tree_nop_conversion plus
operand_equal_p which got a defensive check at the same time
(and checks for error_mark_node right before that check).
Bootstrap & regtest running on x86_64-unknown-linux-gnu.
The testcase comes with the other patch (but still included below).
Richard.
2019-03-21 Richard Biener <rguenther@suse.de>
PR tree-optimization/89779
* tree.c (tree_nop_conversion): Consolidate and fix defensive
checks with respect to released SSA names now having error_mark_node
type.
* fold-const.c (operand_equal_p): Likewise.
Index: gcc/tree.c
===================================================================
--- gcc/tree.c (revision 269832)
+++ gcc/tree.c (working copy)
@@ -12812,13 +12812,10 @@ tree_nop_conversion (const_tree exp)
if (!CONVERT_EXPR_P (exp)
&& TREE_CODE (exp) != NON_LVALUE_EXPR)
return false;
- if (TREE_OPERAND (exp, 0) == error_mark_node)
- return false;
outer_type = TREE_TYPE (exp);
inner_type = TREE_TYPE (TREE_OPERAND (exp, 0));
-
- if (!inner_type)
+ if (inner_type == error_mark_node)
return false;
return tree_nop_conversion_p (outer_type, inner_type);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 269832)
+++ gcc/fold-const.c (working copy)
@@ -2973,11 +2973,6 @@ operand_equal_p (const_tree arg0, const_
|| TREE_TYPE (arg1) == error_mark_node)
return 0;
- /* Similar, if either does not have a type (like a released SSA name),
- they aren't equal. */
- if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
- return 0;
-
/* We cannot consider pointers to different address space equal. */
if (POINTER_TYPE_P (TREE_TYPE (arg0))
&& POINTER_TYPE_P (TREE_TYPE (arg1))
Index: gcc/testsuite/gcc.dg/torture/pr89779.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr89779.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr89779.c (working copy)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+
+typedef int a;
+void h(a);
+void c(a *d, int b)
+{
+ int e, f, g;
+ for (; e; e++)
+ for (f = 0; f < 4; f++)
+ if (d)
+ for (g = e + 1; g; g++)
+ h(d[g]);
+}
+void i()
+{
+ a *j;
+ int k, l;
+ for (; k; k++)
+ c(j, l);
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix")
2019-03-21 10:13 [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix") Richard Biener
@ 2019-03-21 14:04 ` Richard Biener
2019-03-21 19:03 ` Jason Merrill
2019-03-22 9:22 ` Christophe Lyon
0 siblings, 2 replies; 6+ messages in thread
From: Richard Biener @ 2019-03-21 14:04 UTC (permalink / raw)
To: gcc-patches; +Cc: jason
On Thu, 21 Mar 2019, Richard Biener wrote:
>
> This also avoids the ICE in PR89779 but IMHO is not a real fix.
>
> Still it restores a previously active check against released SSA names
> which now have error_mark_node type rather than NULL. The new way
> opens up consolidation so I've adjusted tree_nop_conversion plus
> operand_equal_p which got a defensive check at the same time
> (and checks for error_mark_node right before that check).
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>
> The testcase comes with the other patch (but still included below).
Ugh, looks like my defense installed for released SSA names is now
relied upon by the C++ FE via
#2 0x0000000001864a96 in tree_strip_nop_conversions (
exp=<convert_expr 0x7ffff69bc920>)
at /space/rguenther/src/svn/trunk2/gcc/tree.c:12848
#3 0x0000000000ab2d2f in iterative_hash_template_arg (
arg=<convert_expr 0x7ffff69bc920>, val=3363282904)
at /space/rguenther/src/svn/trunk2/gcc/cp/pt.c:1751
for
<convert_expr 0x7ffff69bc920
type <integer_type 0x7ffff68985e8 int public type_6 SI
size <integer_cst 0x7ffff689c078 constant 32>
unit-size <integer_cst 0x7ffff689c090 constant 4>
align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff68985e8 precision:32 min <integer_cst 0x7ffff689c030 -2147483648>
max <integer_cst 0x7ffff689c048 2147483647>
pointer_to_this <pointer_type 0x7ffff68a19d8>>
arg:0 <scope_ref 0x7ffff69d0b40 tree_0
arg:0 <template_type_parm 0x7ffff69d17e0 i tree_0 type_0 type_6
VOID
align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x7ffff69d1150
index 0 level 1 orig_level 1
chain <type_decl 0x7ffff69d2098 i>>
arg:1 <identifier_node 0x7ffff69c39c0 c
normal local bindings <(nil)>>>>
where both the SCOPE_REF and TEMPLATE_TYPE_PARM have NULL
TREE_TYPE.
So I'll restore the NULL check in tree_nop_conversion.
But since the C++ FE overloads almost every tree field possible
I wonder whether that STRIP_NOPS in iterative_hash_template_arg
is a good idea?
Also I hope the FE doesn't call into operand_equal_p with such
typed trees.
Updated patch below, retesting now.
Thanks,
Richard.
2019-03-21 Richard Biener <rguenther@suse.de>
PR tree-optimization/89779
* tree.c (tree_nop_conversion): Consolidate and fix defensive
checks with respect to released SSA names now having error_mark_node
type.
* fold-const.c (operand_equal_p): Likewise.
Index: gcc/tree.c
===================================================================
--- gcc/tree.c (revision 269832)
+++ gcc/tree.c (working copy)
@@ -12812,13 +12812,10 @@ tree_nop_conversion (const_tree exp)
if (!CONVERT_EXPR_P (exp)
&& TREE_CODE (exp) != NON_LVALUE_EXPR)
return false;
- if (TREE_OPERAND (exp, 0) == error_mark_node)
- return false;
outer_type = TREE_TYPE (exp);
inner_type = TREE_TYPE (TREE_OPERAND (exp, 0));
-
- if (!inner_type)
+ if (!inner_type || inner_type == error_mark_node)
return false;
return tree_nop_conversion_p (outer_type, inner_type);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 269832)
+++ gcc/fold-const.c (working copy)
@@ -2973,11 +2973,6 @@ operand_equal_p (const_tree arg0, const_
|| TREE_TYPE (arg1) == error_mark_node)
return 0;
- /* Similar, if either does not have a type (like a released SSA name),
- they aren't equal. */
- if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
- return 0;
-
/* We cannot consider pointers to different address space equal. */
if (POINTER_TYPE_P (TREE_TYPE (arg0))
&& POINTER_TYPE_P (TREE_TYPE (arg1))
Index: gcc/testsuite/gcc.dg/torture/pr89779.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr89779.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr89779.c (working copy)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+
+typedef int a;
+void h(a);
+void c(a *d, int b)
+{
+ int e, f, g;
+ for (; e; e++)
+ for (f = 0; f < 4; f++)
+ if (d)
+ for (g = e + 1; g; g++)
+ h(d[g]);
+}
+void i()
+{
+ a *j;
+ int k, l;
+ for (; k; k++)
+ c(j, l);
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix")
2019-03-21 14:04 ` Richard Biener
@ 2019-03-21 19:03 ` Jason Merrill
2019-03-22 8:41 ` Richard Biener
2019-03-22 9:22 ` Christophe Lyon
1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2019-03-21 19:03 UTC (permalink / raw)
To: Richard Biener, gcc-patches
On 3/21/19 9:43 AM, Richard Biener wrote:
> On Thu, 21 Mar 2019, Richard Biener wrote:
>
>>
>> This also avoids the ICE in PR89779 but IMHO is not a real fix.
>>
>> Still it restores a previously active check against released SSA names
>> which now have error_mark_node type rather than NULL. The new way
>> opens up consolidation so I've adjusted tree_nop_conversion plus
>> operand_equal_p which got a defensive check at the same time
>> (and checks for error_mark_node right before that check).
>>
>> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>>
>> The testcase comes with the other patch (but still included below).
>
> Ugh, looks like my defense installed for released SSA names is now
> relied upon by the C++ FE via
>
> #2 0x0000000001864a96 in tree_strip_nop_conversions (
> exp=<convert_expr 0x7ffff69bc920>)
> at /space/rguenther/src/svn/trunk2/gcc/tree.c:12848
> #3 0x0000000000ab2d2f in iterative_hash_template_arg (
> arg=<convert_expr 0x7ffff69bc920>, val=3363282904)
> at /space/rguenther/src/svn/trunk2/gcc/cp/pt.c:1751
>
> for
>
> <convert_expr 0x7ffff69bc920
> type <integer_type 0x7ffff68985e8 int public type_6 SI
> size <integer_cst 0x7ffff689c078 constant 32>
> unit-size <integer_cst 0x7ffff689c090 constant 4>
> align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> 0x7ffff68985e8 precision:32 min <integer_cst 0x7ffff689c030 -2147483648>
> max <integer_cst 0x7ffff689c048 2147483647>
> pointer_to_this <pointer_type 0x7ffff68a19d8>>
>
> arg:0 <scope_ref 0x7ffff69d0b40 tree_0
> arg:0 <template_type_parm 0x7ffff69d17e0 i tree_0 type_0 type_6
> VOID
> align:8 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x7ffff69d1150
> index 0 level 1 orig_level 1
> chain <type_decl 0x7ffff69d2098 i>>
> arg:1 <identifier_node 0x7ffff69c39c0 c
> normal local bindings <(nil)>>>>
>
> where both the SCOPE_REF and TEMPLATE_TYPE_PARM have NULL
> TREE_TYPE.
>
> So I'll restore the NULL check in tree_nop_conversion.
>
> But since the C++ FE overloads almost every tree field possible
> I wonder whether that STRIP_NOPS in iterative_hash_template_arg
> is a good idea?
I think it still makes sense; TREE_TYPE of an expression is either a
type, error_mark_node, or NULL_TREE (for type-dependent expressions in a
template).
> Also I hope the FE doesn't call into operand_equal_p with such
> typed trees.
It shouldn't, we have cp_tree_equal to use instead.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix")
2019-03-21 19:03 ` Jason Merrill
@ 2019-03-22 8:41 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2019-03-22 8:41 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Thu, 21 Mar 2019, Jason Merrill wrote:
> On 3/21/19 9:43 AM, Richard Biener wrote:
> > On Thu, 21 Mar 2019, Richard Biener wrote:
> >
> > >
> > > This also avoids the ICE in PR89779 but IMHO is not a real fix.
> > >
> > > Still it restores a previously active check against released SSA names
> > > which now have error_mark_node type rather than NULL. The new way
> > > opens up consolidation so I've adjusted tree_nop_conversion plus
> > > operand_equal_p which got a defensive check at the same time
> > > (and checks for error_mark_node right before that check).
> > >
> > > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> > >
> > > The testcase comes with the other patch (but still included below).
> >
> > Ugh, looks like my defense installed for released SSA names is now
> > relied upon by the C++ FE via
> >
> > #2 0x0000000001864a96 in tree_strip_nop_conversions (
> > exp=<convert_expr 0x7ffff69bc920>)
> > at /space/rguenther/src/svn/trunk2/gcc/tree.c:12848
> > #3 0x0000000000ab2d2f in iterative_hash_template_arg (
> > arg=<convert_expr 0x7ffff69bc920>, val=3363282904)
> > at /space/rguenther/src/svn/trunk2/gcc/cp/pt.c:1751
> >
> > for
> >
> > <convert_expr 0x7ffff69bc920
> > type <integer_type 0x7ffff68985e8 int public type_6 SI
> > size <integer_cst 0x7ffff689c078 constant 32>
> > unit-size <integer_cst 0x7ffff689c090 constant 4>
> > align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> > 0x7ffff68985e8 precision:32 min <integer_cst 0x7ffff689c030 -2147483648>
> > max <integer_cst 0x7ffff689c048 2147483647>
> > pointer_to_this <pointer_type 0x7ffff68a19d8>>
> > arg:0 <scope_ref 0x7ffff69d0b40 tree_0
> > arg:0 <template_type_parm 0x7ffff69d17e0 i tree_0 type_0 type_6
> > VOID
> > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x7ffff69d1150
> > index 0 level 1 orig_level 1
> > chain <type_decl 0x7ffff69d2098 i>>
> > arg:1 <identifier_node 0x7ffff69c39c0 c
> > normal local bindings <(nil)>>>>
> >
> > where both the SCOPE_REF and TEMPLATE_TYPE_PARM have NULL
> > TREE_TYPE.
> >
> > So I'll restore the NULL check in tree_nop_conversion.
> >
> > But since the C++ FE overloads almost every tree field possible
> > I wonder whether that STRIP_NOPS in iterative_hash_template_arg
> > is a good idea?
>
> I think it still makes sense; TREE_TYPE of an expression is either a type,
> error_mark_node, or NULL_TREE (for type-dependent expressions in a template).
>
> > Also I hope the FE doesn't call into operand_equal_p with such
> > typed trees.
>
> It shouldn't, we have cp_tree_equal to use instead.
OK, let's see how to use that for PR89790 then...
Richard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix")
2019-03-21 14:04 ` Richard Biener
2019-03-21 19:03 ` Jason Merrill
@ 2019-03-22 9:22 ` Christophe Lyon
2019-03-25 10:08 ` Richard Biener
1 sibling, 1 reply; 6+ messages in thread
From: Christophe Lyon @ 2019-03-22 9:22 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc Patches, Jason Merrill
On Thu, 21 Mar 2019 at 14:44, Richard Biener <rguenther@suse.de> wrote:
>
> On Thu, 21 Mar 2019, Richard Biener wrote:
>
> >
> > This also avoids the ICE in PR89779 but IMHO is not a real fix.
> >
> > Still it restores a previously active check against released SSA names
> > which now have error_mark_node type rather than NULL. The new way
> > opens up consolidation so I've adjusted tree_nop_conversion plus
> > operand_equal_p which got a defensive check at the same time
> > (and checks for error_mark_node right before that check).
> >
> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >
> > The testcase comes with the other patch (but still included below).
>
> Ugh, looks like my defense installed for released SSA names is now
> relied upon by the C++ FE via
>
> #2 0x0000000001864a96 in tree_strip_nop_conversions (
> exp=<convert_expr 0x7ffff69bc920>)
> at /space/rguenther/src/svn/trunk2/gcc/tree.c:12848
> #3 0x0000000000ab2d2f in iterative_hash_template_arg (
> arg=<convert_expr 0x7ffff69bc920>, val=3363282904)
> at /space/rguenther/src/svn/trunk2/gcc/cp/pt.c:1751
>
> for
>
> <convert_expr 0x7ffff69bc920
> type <integer_type 0x7ffff68985e8 int public type_6 SI
> size <integer_cst 0x7ffff689c078 constant 32>
> unit-size <integer_cst 0x7ffff689c090 constant 4>
> align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> 0x7ffff68985e8 precision:32 min <integer_cst 0x7ffff689c030 -2147483648>
> max <integer_cst 0x7ffff689c048 2147483647>
> pointer_to_this <pointer_type 0x7ffff68a19d8>>
>
> arg:0 <scope_ref 0x7ffff69d0b40 tree_0
> arg:0 <template_type_parm 0x7ffff69d17e0 i tree_0 type_0 type_6
> VOID
> align:8 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x7ffff69d1150
> index 0 level 1 orig_level 1
> chain <type_decl 0x7ffff69d2098 i>>
> arg:1 <identifier_node 0x7ffff69c39c0 c
> normal local bindings <(nil)>>>>
>
> where both the SCOPE_REF and TEMPLATE_TYPE_PARM have NULL
> TREE_TYPE.
>
> So I'll restore the NULL check in tree_nop_conversion.
>
> But since the C++ FE overloads almost every tree field possible
> I wonder whether that STRIP_NOPS in iterative_hash_template_arg
> is a good idea?
>
> Also I hope the FE doesn't call into operand_equal_p with such
> typed trees.
>
> Updated patch below, retesting now.
>
> Thanks,
> Richard.
>
> 2019-03-21 Richard Biener <rguenther@suse.de>
>
> PR tree-optimization/89779
> * tree.c (tree_nop_conversion): Consolidate and fix defensive
> checks with respect to released SSA names now having error_mark_node
> type.
> * fold-const.c (operand_equal_p): Likewise.
>
Hi,
I think this patch is causing an ICE when building gdb-8.2. I'm seeing:
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/_build/builds/destdir/x86_64-unknown-linux-gnu/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:
In function 'constexpr void std::__advance(_RandomAccessIter
ator&, _Distance, std::random_access_iterator_tag)':
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/_build/builds/destdir/x86_64-unknown-linux-gnu/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:182:54:
internal compiler error: Segmentation fault
182 | else if (__builtin_constant_p(__n) && __n == -1)
| ^
0xf9964f crash_signal
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/toplev.c:326
0xf9964f crash_signal
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/toplev.c:326
0xf9964f crash_signal
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/toplev.c:326
0xc980dc operand_equal_p(tree_node const*, tree_node const*, unsigned int)
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/fold-const.c:2977
0xc9829a operand_equal_p(tree_node const*, tree_node const*, unsigned int)
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/fold-const.c:2950
0x9c1cf5 cp_parser_selection_statement
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11784
0x9c1cf5 cp_parser_statement
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11147
0x9db9a7 cp_parser_implicitly_scoped_statement
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:13004
0x9c1c37 cp_parser_selection_statement
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11865
0x9c1c37 cp_parser_statement
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11147
0x9c2218 cp_parser_statement_seq_opt
/home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11628
0x9c22f8 cp_parser_compound_statement
[...]
while compiling linux-procfs.o, gdb.o, linux-waitpid.o, arch/amd64.o,
linux-ptrace.o and others
Seeing that on x86_64, arm, aarch64.
Christophe
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c (revision 269832)
> +++ gcc/tree.c (working copy)
> @@ -12812,13 +12812,10 @@ tree_nop_conversion (const_tree exp)
> if (!CONVERT_EXPR_P (exp)
> && TREE_CODE (exp) != NON_LVALUE_EXPR)
> return false;
> - if (TREE_OPERAND (exp, 0) == error_mark_node)
> - return false;
>
> outer_type = TREE_TYPE (exp);
> inner_type = TREE_TYPE (TREE_OPERAND (exp, 0));
> -
> - if (!inner_type)
> + if (!inner_type || inner_type == error_mark_node)
> return false;
>
> return tree_nop_conversion_p (outer_type, inner_type);
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 269832)
> +++ gcc/fold-const.c (working copy)
> @@ -2973,11 +2973,6 @@ operand_equal_p (const_tree arg0, const_
> || TREE_TYPE (arg1) == error_mark_node)
> return 0;
>
> - /* Similar, if either does not have a type (like a released SSA name),
> - they aren't equal. */
> - if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
> - return 0;
> -
> /* We cannot consider pointers to different address space equal. */
> if (POINTER_TYPE_P (TREE_TYPE (arg0))
> && POINTER_TYPE_P (TREE_TYPE (arg1))
> Index: gcc/testsuite/gcc.dg/torture/pr89779.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr89779.c (nonexistent)
> +++ gcc/testsuite/gcc.dg/torture/pr89779.c (working copy)
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +
> +typedef int a;
> +void h(a);
> +void c(a *d, int b)
> +{
> + int e, f, g;
> + for (; e; e++)
> + for (f = 0; f < 4; f++)
> + if (d)
> + for (g = e + 1; g; g++)
> + h(d[g]);
> +}
> +void i()
> +{
> + a *j;
> + int k, l;
> + for (; k; k++)
> + c(j, l);
> +}
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix")
2019-03-22 9:22 ` Christophe Lyon
@ 2019-03-25 10:08 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2019-03-25 10:08 UTC (permalink / raw)
To: Christophe Lyon; +Cc: gcc Patches, Jason Merrill
On Fri, 22 Mar 2019, Christophe Lyon wrote:
> On Thu, 21 Mar 2019 at 14:44, Richard Biener <rguenther@suse.de> wrote:
> >
> > On Thu, 21 Mar 2019, Richard Biener wrote:
> >
> > >
> > > This also avoids the ICE in PR89779 but IMHO is not a real fix.
> > >
> > > Still it restores a previously active check against released SSA names
> > > which now have error_mark_node type rather than NULL. The new way
> > > opens up consolidation so I've adjusted tree_nop_conversion plus
> > > operand_equal_p which got a defensive check at the same time
> > > (and checks for error_mark_node right before that check).
> > >
> > > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> > >
> > > The testcase comes with the other patch (but still included below).
> >
> > Ugh, looks like my defense installed for released SSA names is now
> > relied upon by the C++ FE via
> >
> > #2 0x0000000001864a96 in tree_strip_nop_conversions (
> > exp=<convert_expr 0x7ffff69bc920>)
> > at /space/rguenther/src/svn/trunk2/gcc/tree.c:12848
> > #3 0x0000000000ab2d2f in iterative_hash_template_arg (
> > arg=<convert_expr 0x7ffff69bc920>, val=3363282904)
> > at /space/rguenther/src/svn/trunk2/gcc/cp/pt.c:1751
> >
> > for
> >
> > <convert_expr 0x7ffff69bc920
> > type <integer_type 0x7ffff68985e8 int public type_6 SI
> > size <integer_cst 0x7ffff689c078 constant 32>
> > unit-size <integer_cst 0x7ffff689c090 constant 4>
> > align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> > 0x7ffff68985e8 precision:32 min <integer_cst 0x7ffff689c030 -2147483648>
> > max <integer_cst 0x7ffff689c048 2147483647>
> > pointer_to_this <pointer_type 0x7ffff68a19d8>>
> >
> > arg:0 <scope_ref 0x7ffff69d0b40 tree_0
> > arg:0 <template_type_parm 0x7ffff69d17e0 i tree_0 type_0 type_6
> > VOID
> > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x7ffff69d1150
> > index 0 level 1 orig_level 1
> > chain <type_decl 0x7ffff69d2098 i>>
> > arg:1 <identifier_node 0x7ffff69c39c0 c
> > normal local bindings <(nil)>>>>
> >
> > where both the SCOPE_REF and TEMPLATE_TYPE_PARM have NULL
> > TREE_TYPE.
> >
> > So I'll restore the NULL check in tree_nop_conversion.
> >
> > But since the C++ FE overloads almost every tree field possible
> > I wonder whether that STRIP_NOPS in iterative_hash_template_arg
> > is a good idea?
> >
> > Also I hope the FE doesn't call into operand_equal_p with such
> > typed trees.
> >
> > Updated patch below, retesting now.
> >
> > Thanks,
> > Richard.
> >
> > 2019-03-21 Richard Biener <rguenther@suse.de>
> >
> > PR tree-optimization/89779
> > * tree.c (tree_nop_conversion): Consolidate and fix defensive
> > checks with respect to released SSA names now having error_mark_node
> > type.
> > * fold-const.c (operand_equal_p): Likewise.
> >
> Hi,
>
> I think this patch is causing an ICE when building gdb-8.2. I'm seeing:
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/_build/builds/destdir/x86_64-unknown-linux-gnu/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:
> In function 'constexpr void std::__advance(_RandomAccessIter
> ator&, _Distance, std::random_access_iterator_tag)':
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/_build/builds/destdir/x86_64-unknown-linux-gnu/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:182:54:
> internal compiler error: Segmentation fault
> 182 | else if (__builtin_constant_p(__n) && __n == -1)
> | ^
> 0xf9964f crash_signal
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/toplev.c:326
> 0xf9964f crash_signal
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/toplev.c:326
> 0xf9964f crash_signal
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/toplev.c:326
> 0xc980dc operand_equal_p(tree_node const*, tree_node const*, unsigned int)
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/fold-const.c:2977
> 0xc9829a operand_equal_p(tree_node const*, tree_node const*, unsigned int)
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/fold-const.c:2950
> 0x9c1cf5 cp_parser_selection_statement
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11784
> 0x9c1cf5 cp_parser_statement
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11147
> 0x9db9a7 cp_parser_implicitly_scoped_statement
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:13004
> 0x9c1c37 cp_parser_selection_statement
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11865
> 0x9c1c37 cp_parser_statement
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11147
> 0x9c2218 cp_parser_statement_seq_opt
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_ae5efb2cfe93a4277b5fa185c2c8582592b47f7a/gcc/cp/parser.c:11628
> 0x9c22f8 cp_parser_compound_statement
> [...]
>
> while compiling linux-procfs.o, gdb.o, linux-waitpid.o, arch/amd64.o,
> linux-ptrace.o and others
>
> Seeing that on x86_64, arm, aarch64.
Fixed like following.
Bootstrapped / tested on x86_64-unknown-linux-gnu, applied.
Richard.
2019-03-25 Richard Biener <rguenther@suse.de>
PR middle-end/89790
* fold-const.c (operand_equal_p): Revert last change with
updated comment.
* g++.dg/pr89790.C: New testcase.
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 269905)
+++ gcc/fold-const.c (working copy)
@@ -2973,6 +2973,11 @@ operand_equal_p (const_tree arg0, const_
|| TREE_TYPE (arg1) == error_mark_node)
return 0;
+ /* Similar, if either does not have a type (like a template id),
+ they aren't equal. */
+ if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
+ return 0;
+
/* We cannot consider pointers to different address space equal. */
if (POINTER_TYPE_P (TREE_TYPE (arg0))
&& POINTER_TYPE_P (TREE_TYPE (arg1))
Index: gcc/testsuite/g++.dg/pr89790.C
===================================================================
--- gcc/testsuite/g++.dg/pr89790.C (revision 0)
+++ gcc/testsuite/g++.dg/pr89790.C (working copy)
@@ -0,0 +1,20 @@
+// { dg-do compile }
+// { dg-options "-Wduplicated-cond" }
+
+template <typename>
+class a
+{
+ typedef a b;
+ template <typename> void c();
+};
+template <typename d> template <typename>
+void a<d>::c()
+{
+ int f;
+ b g;
+ if (g == 0)
+ ;
+ else if (f)
+ {
+ }
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-25 10:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-21 10:13 [PATCH] Restore defensive check in tree_nop_conversion (also PR89779 "fix") Richard Biener
2019-03-21 14:04 ` Richard Biener
2019-03-21 19:03 ` Jason Merrill
2019-03-22 8:41 ` Richard Biener
2019-03-22 9:22 ` Christophe Lyon
2019-03-25 10:08 ` Richard Biener
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).