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 <hain...@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
