[PATCH/RFC] Fix ICE in find_taken_edge_computed_goto (PR 84136)
David Malcolm
dmalcolm@redhat.com
Wed Jan 31 16:03:00 GMT 2018
PR 84136 reports an ICE within sccvn_dom_walker when handling a
C/C++ source file that overuses the labels-as-values extension.
The code in question stores a jump label into a global, and then
jumps to it from another function, which ICEs after inlining:
void* a;
void foo() {
if ((a = &&l))
return;
l:;
}
int main() {
foo();
goto *a;
return 0;
}
This appears to be far beyond what we claim to support in this
extension - but we shouldn't ICE.
What's happening is that, after inlining, we have usage of a *copy*
of the label, which optimizes away the if-return logic, turning it
into an infinite loop.
On entry to the sccvn_dom_walker we have this gimple:
main ()
{
void * a.0_1;
<bb 2> [count: 0]:
a = &l;
<bb 3> [count: 0]:
l:
a.0_1 = a;
goto a.0_1;
}
and:
edge taken = find_taken_edge (bb, vn_valueize (val));
reasonably valueizes the:
goto a.0_1;
after the:
a = &l;
a.0_1 = a;
as if it were:
goto *&l;
find_taken_edge_computed_goto then has:
2380 dest = label_to_block (val);
2381 if (dest)
2382 {
2383 e = find_edge (bb, dest);
2384 gcc_assert (e != NULL);
2385 }
which locates dest as a self-jump from block 3 back to itself.
However, the find_edge call returns NULL - it has a predecessor edge
from block 2, but no successor edges.
Hence the assertion fails and we ICE.
A successor edge from the computed goto could have been created by
make_edges if the label stmt had been in the function, but make_edges
only looks in the current function when handling computed gotos, and
the label only appeared after inlining.
The following patch removes the assertion, fixing the ICE.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
If that's option (a), there could be some other approaches:
(b) convert the assertion into a warning/error/sorry, on the
assumption that if we don't detect such an edge then the code is
presumably abusing the labels-as-values feature
(c) have make_edges detect such a problematic computed goto (maybe
converting make_edges_bb's return value to an enum and adding a 4th
value - though it's not clear what to do then with it)
(d) detect this case on inlining and handle it somehow (e.g. adding
edges for labels that have appeared since make_edges originally
ran, for computed gotos that have no out-edges)
(e) do nothing, keeping the assertion, and accept that this is going
to fail on a non-release build
(f) something else?
Of the above, (d) seems to me to be the most robust solution, but I
don't know how far we want to go "down the rabbit hole" of handling
such uses of labels-as-values (beyond not ICE-ing on them).
Thoughts?
gcc/testsuite/ChangeLog:
PR tree-optimization/84136
* gcc.c-torture/compile/pr84136.c: New test.
gcc/ChangeLog:
PR tree-optimization/84136
* tree-cfg.c (find_taken_edge_computed_goto): Remove assertion
that the result of find_edge is non-NULL.
---
gcc/testsuite/gcc.c-torture/compile/pr84136.c | 15 +++++++++++++++
gcc/tree-cfg.c | 5 +----
2 files changed, 16 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr84136.c
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr84136.c b/gcc/testsuite/gcc.c-torture/compile/pr84136.c
new file mode 100644
index 0000000..0a70e4e
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr84136.c
@@ -0,0 +1,15 @@
+void* a;
+
+void foo() {
+ if ((a = &&l))
+ return;
+
+ l:;
+}
+
+int main() {
+ foo();
+ goto *a;
+
+ return 0;
+}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index c5318b9..6b89307 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -2379,10 +2379,7 @@ find_taken_edge_computed_goto (basic_block bb, tree val)
dest = label_to_block (val);
if (dest)
- {
- e = find_edge (bb, dest);
- gcc_assert (e != NULL);
- }
+ e = find_edge (bb, dest);
return e;
}
--
1.8.5.3
More information about the Gcc-patches
mailing list