[patch] handle casesi dispatch insns in create_trace_edges

[Jeff Law]

On 8/9/19 11:07 AM, Olivier Hainque wrote:
> Hello,
> 
> The attached patch is a proposal to plug a hole in create_trace_edges
> (dwarf2cfi.c), which doesn't handle casesi dispatch insns.
> 
> The visible misbehavior we observed is a failure in a cross configuration
> of a recent acats test for Ada, a very simplified sketch of which is provided
> below.
> 
> This was with gcc-7 on a port which has been deprecated since then, but ISTM
> the problem remains latent for other ports.
> 
> Essentially, we had a jump insn like:
> 
>    if X <= 4                      -- for case values
>      set pc *(&label_59 + kind * 4)  -- 0 .. 4
>    else
>      set pc &label_151
> 
> for the case statement, and the tablejump_p processing code in
> create_trace_edges only gets through the first 5 possible targets.
> 
> The missing edge in the re-created control-flow graph eventually materialized
> as missing .cfi_remember_state/.cfi_restore_state pairs in the output, which
> resulted in bogus exception handling behavior.
> 
> The insn pattern corresponds to the one handled in patch_jump_insn
> (cfgrtl.c). The proposed patch extracts the pattern recognition code
> in a separate function and uses it in both patch_jump_insn and
> create_trace_edges.
> 
> This fixed our problem on the cross port (arm-vxworks6) and we
> have been running with it for all our gcc-7 and gcc-8 ports since
> then, about 6 months ago now.
> 
> It also bootstraps and regression tests fine with mainline
> on x86_64-linux.
> 
> Ok, to commit ?
> 
> Thanks in advance!
> 
> With Kind Regards,
> 
> Olivier
> 
> 2019-08-09  Olivier Hainque  <hainque@adacore.com>
> 
>         * rtl.h (tablejump_casesi_pattern): New helper, casesi
>         recognition logic originating from code in cfgrtl.c.
>         * cfgrtl.c (patch_jump_insn): Use it.
>         * dwarf2cfi.c (create_trace_edges): Handle casesi patterns.
Is there a reason to think the routine is performance critical enough to
be inlined?  If not it would make more sense to me to put it into rtl.c
with just a declaration in rtl.h

So if it is performance critical, then the patch is OK as-is.  If not,
moving the implementation into rtl.c with a declaration in rtl.h should
be considered pre-approved -- just post it here for archival purposes.


Thanks,
jeff