On Mon, Feb 14, 2022 at 4:54 AM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
>
> This simple fix to the middle-end, resolves PR c/104506, by adding an
>
> explicit check for error_mark_node to useless_type_conversion_p. I first
>
> trying fixing this in the C front-end, but the type is valid at the point
>
> that the NOP_EXPR is created, so the poisoned type leaks to the middle-end.
>
> Returning either true or false from useless_type_conversion_p avoids the
>
> ICE-after-error. Apologies to Andrew Pinski, I hadn't noticed that he'd
>
> assigned this PR to himself until after my regression testing had finished.
>
>
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
>
> make -k check with no new failures. Ok for mainline?
>
>
>
>
>
> 2022-02-14 Roger Sayle <ro...@nextmovesoftware.com>
>
>
>
> gcc/ChangeLog
>
> PR c/104506
>
> * gimple-expr.cc (useless_type_conversion_p): Add a check for
>
> error_mark_node.
I came up with a different patch (attached) which just changes
tree_ssa_useless_type_conversion rather than useless_type_conversion_p
which I was going to submit but had an issue with my build machine.
I did it this way as it was similar to how
STRIP_NOPS/tree_nop_conversion was done already.
Also from my description of the patch
STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier
and the places where it is used outside of the gimplifier would not
be adding too much overhead.
Though I think Richard Biener's patch is better really. It would be
interesting to see how the C++ front-end handles this case, I remember
it using integer_type_node in some locations after an error for a
type.
Thanks,
Andrew Pinski
>
>
>
> gcc/testsuite/ChangeLog
>
> PR c/104506
>
> * gcc.dg/pr104506.c: New test case.
>
>
>
>
>
> Roger
>
> --
>
>
From 43d418042efacbb7efe8664fbb1176608470474a Mon Sep 17 00:00:00 2001
From: Andrew Pinski <apin...@marvell.com>
Date: Sun, 13 Feb 2022 00:09:39 +0000
Subject: [PATCH] c: [PR104506] Fix ICE after error due to change of type to
error_mark_node
The problem here is we end up with an error_mark_node when calling
useless_type_conversion_p and that ICEs. STRIP_NOPS/tree_nop_conversion
has had a check for the inner type being an error_mark_node since g9a6bb3f78c96
(2000). This just adds the check also to tree_ssa_useless_type_conversion.
STRIP_USELESS_TYPE_CONVERSION is mostly used inside the gimplifier
and the places where it is used outside of the gimplifier would not
be adding too much overhead.
OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
Thanks,
Andrew Pinski
PR c/104506
gcc/ChangeLog:
* tree-ssa.cc (tree_ssa_useless_type_conversion):
Check the inner type before calling useless_type_conversion_p.
gcc/testsuite/ChangeLog:
* gcc.dg/pr104506-1.c: New test.
* gcc.dg/pr104506-2.c: New test.
* gcc.dg/pr104506-3.c: New test.
---
gcc/testsuite/gcc.dg/pr104506-1.c | 12 ++++++++++++
gcc/testsuite/gcc.dg/pr104506-2.c | 11 +++++++++++
gcc/testsuite/gcc.dg/pr104506-3.c | 11 +++++++++++
gcc/tree-ssa.cc | 20 +++++++++++++-------
4 files changed, 47 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr104506-1.c
create mode 100644 gcc/testsuite/gcc.dg/pr104506-2.c
create mode 100644 gcc/testsuite/gcc.dg/pr104506-3.c
diff --git a/gcc/testsuite/gcc.dg/pr104506-1.c
b/gcc/testsuite/gcc.dg/pr104506-1.c
new file mode 100644
index 00000000000..5eb71911b71
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr104506-1.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu11" } */
+/* PR c/104506: we used to ICE after the error of
+ changing the type. */
+
+void
+foo (double x)
+/* { dg-message "note: previous definition" "previous definition" { target
*-*-* } .-1 } */
+{
+ (void)x;
+ int x; /* { dg-error "redeclared as different kind of symbol" } */
+}
diff --git a/gcc/testsuite/gcc.dg/pr104506-2.c
b/gcc/testsuite/gcc.dg/pr104506-2.c
new file mode 100644
index 00000000000..3c3aaaac4f8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr104506-2.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu11" } */
+/* PR c/104506: we used to ICE after the error of
+ changing the type. */
+void
+foo (double x)
+/* { dg-message "note: previous definition" "previous definition" { target
*-*-* } .-1 } */
+{
+ x;
+ int x; /* { dg-error "redeclared as different kind of symbol" } */
+}
diff --git a/gcc/testsuite/gcc.dg/pr104506-3.c
b/gcc/testsuite/gcc.dg/pr104506-3.c
new file mode 100644
index 00000000000..b14deb5cf25
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr104506-3.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* PR c/104506: we used to ICE after the error of
+ changing the type. */
+double x;
+/* { dg-message "note: previous declaration" "previous declaration" { target
*-*-* } .-1 } */
+void
+foo (void)
+{
+ x;
+}
+int x; /* { dg-error "conflicting types" } */
diff --git a/gcc/tree-ssa.cc b/gcc/tree-ssa.cc
index 8fe0682981d..98dada09647 100644
--- a/gcc/tree-ssa.cc
+++ b/gcc/tree-ssa.cc
@@ -1256,18 +1256,24 @@ delete_tree_ssa (struct function *fn)
bool
tree_ssa_useless_type_conversion (tree expr)
{
+ tree outer_type, inner_type;
+
/* If we have an assignment that merely uses a NOP_EXPR to change
the top of the RHS to the type of the LHS and the type conversion
is "safe", then strip away the type conversion so that we can
enter LHS = RHS into the const_and_copies table. */
- if (CONVERT_EXPR_P (expr)
- || TREE_CODE (expr) == VIEW_CONVERT_EXPR
- || TREE_CODE (expr) == NON_LVALUE_EXPR)
- return useless_type_conversion_p
- (TREE_TYPE (expr),
- TREE_TYPE (TREE_OPERAND (expr, 0)));
+ if (!CONVERT_EXPR_P (expr)
+ && TREE_CODE (expr) != VIEW_CONVERT_EXPR
+ && TREE_CODE (expr) != NON_LVALUE_EXPR)
+ return false;
- return false;
+ outer_type = TREE_TYPE (expr);
+ inner_type = TREE_TYPE (TREE_OPERAND (expr, 0));
+
+ if (!inner_type || inner_type == error_mark_node)
+ return false;
+
+ return useless_type_conversion_p (outer_type, inner_type);
}
/* Strip conversions from EXP according to
--
2.17.1
