Re: [PR middle-end/82123] 00/06 Use EVRP range data in sprintf warnings

[Jeff Law <law@redhat.com>]

On 02/20/2018 05:00 PM, Joseph Myers wrote:
> Does this help with any of the cases in bug 80776 that weren't already 
> fixed, or are those distinct despite looking similar?
> 
I don't think so.

THe __builtin_unreachable markers are removed by vrp1 -- well before the
sprintf warning code gets run.

So the sprintf warning code never gets to exploit the properties implied
by the __builtin_unreachable calls.


It doesn't look like VRP records the narrowed ranges implied by the
__builtin_unreachable calls.

After ASSERT_EXPR insertion we have:

;;   basic block 6, loop depth 0, count 1072883002 (estimated locally),
maybe hot
;;    prev block 5, next block 1, flags: (NEW, REACHABLE, VISITED)
;;    pred:       4 [100.0% (guessed)]  count:1072883003 (estimated
locally) (FALSE_VALUE,EXECUTABLE)
  i_7 = ASSERT_EXPR <i_6, (unsigned int) i_6 <= 999999>;
  __builtin___sprintf_chk (&number, 1, 7, "%d", i_7);
  return;

ANd the ranges computed by VRP:


i.0_1: [0, 999999]
i_4: [0, +INF]
i_6: [0, +INF]  EQUIVALENCES: { i_4 } (1 elements)
i_7: [0, 999999]  EQUIVALENCES: { i_4 i_6 } (2 elements)

So VRP does identify the narrow range for i_7.

But then we remove the ASSERT_EXPRs and we're left with:


  i_4 = somerandom ();
  i.0_1 = (unsigned int) i_4;
  __builtin___sprintf_chk (&number, 1, 7, "%d", i_4);
  return;

Subsequent EVRP analysis will start with the range of i_4 as a seed.
BUt there's nothing to further narrow that range.

If ASSERT_EXPR removal could be taught to use i_7 I suspect the right
things would "just happen".  I haven't thought at all about what might
be required to have VRP do-the-right-thing.  Given the overall desire to
drop ASSERT_EXPRs and the range propagation step in VRP in favor of EVRP
style analysis I doubt anyone is likely to spend much time on fixing
this in the old style VRP analysis.

jeff