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
