[Bug middle-end/65233] [5 Regression] ICE (segfault) on arm-linux-gnueabihf and aarch64-linux-gnu

["law at redhat dot com" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65233

--- Comment #21 from Jeffrey A. Law <law at redhat dot com> ---
I was a bit concerned that something here might be more deeply amiss, so I took
a little time to walk through things under the debugger.


If I put a breakpoint in cleanup_tree_cfg and ignore it until the call that
faults...  Then a bit of debugging yields:

(gdb) p debug_bb_n (30)
<bb 30>:
# SR.33_23 = PHI <>
# SR.32_80 = PHI <>
goto <bb 12>;

(gdb) p debug_bb_n (13)
<bb 13>:
# iftmp.4_25 = PHI <iftmp.4_20(42), iftmp.4_22(45), iftmp.4_98(46)>
# SR.33_83 = PHI <SR.33_23(42), SR.33_52(45), SR.33_52(46)>
# SR.32_16 = PHI <SR.32_80(42), SR.32_81(45), SR.32_81(46)>
_18 = SR.33_83->_vptr.C;
_19 = *_18;
OBJ_TYPE_REF(_19;(struct C)SR.33_83->0) (SR.33_83);
goto <bb 15>;

So, that's consistent with what I expected.  The key being BB30 defines _23,
but is unreachable...

After removing the unreachable blocks and computing dominators we have:

<bb 13>:
# iftmp.4_25 = PHI <iftmp.4_20(42), iftmp.4_22(45), iftmp.4_98(46)>
# SR.33_83 = PHI <SR.33_23(42), SR.33_52(45), SR.33_52(46)>
# SR.32_16 = PHI <SR.32_80(42), SR.32_81(45), SR.32_81(46)>
_18 = SR.33_83->_vptr.C;
_19 = *_18;
OBJ_TYPE_REF(_19;(struct C)SR.33_83->0) (SR.33_83);
goto <bb 15>;

So there's still a reference to SR.33_23, but BB13 (the defining block for
SR.33_23) has been removed. 

When we removed BB13, we tried to release SR.33_23, but it's queued for SSA
updating:

(gdb) p ((*cfun)->gimple_df->ssa_names).m_vecdata[23]
$46 = (tree_node *) 0x7ffff65e1ea0
(gdb) p name_registered_for_update_p ($46)
$47 = true

And that's the key here.  We are at the point where the SSA graph is
potentially in consistent (SSA update occurs after CFG cleanup).  This is the
situation I was hinting at in IRC today Aldy.  The fact that SSA updates occur
after cfgcleanup implies that the SSA graph can have some inconsistencies 
(such as an SSA_NAME with no defining statement).  Anything called during
cfgcleanup has to be prepared to handle these inconsistencies (and yes, it's
incredibly lame to have these kind of inconsistencies).

So diving deeper, we're trying to merge bb47 and bb13:

(gdb) p debug_bb (a)
<bb 47>:
# iftmp.4_55 = PHI <-1(48)>
goto <bb 13>;

$70 = void
(gdb) p debug_bb (b)
<bb 13>:
# SR.33_83 = PHI <SR.33_23(47)>
# SR.32_16 = PHI <SR.32_80(47)>
_18 = SR.33_83->_vptr.C;
_19 = *_18;
OBJ_TYPE_REF(_19;(struct C)SR.33_23->0) (SR.33_23);
goto <bb 15>;


Which is very reasonable.  In the case where the successor has PHIs we try to
propagate the PHI RHS.  Ie, we'll try to replace _83 with _23 in the call. 
That in turn triggers the polymorphic call code that walks the up the use-def
chain via walk_ssa_copies.  walk_ssa_copies has to be prepared to find an
SSA_NAME (_23 in this case) in an inconsistent state (such as being defined in
an unreachable block by a PHI with no RHS arguments).

I think Aldy's patch is fine for this issue.  However, I certainly worry that
with the design issue that has the SSA graph in an inconsistent state that
we'll likely run into other problems with code that's not prepared to handle
the inconsistencies.