On 01/31/2018 08:39 AM, David Malcolm wrote:
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?
If the source code is invalid or unsupported as you say and
should stay that way then one of the alternatives mentioned
in (b) sounds preferable to me, at least in the near term.
It will make it clear to users that it is, in fact, not
supported.
Loner term, I agree that (d) seems ideal if it's feasible
(I have very little experience with this extension to judge
that, or even how useful it might be).
Martin
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;
}
