On 02/20/2018 01:13 PM, Martin Sebor wrote:
> On 02/20/2018 12:03 PM, Martin Sebor wrote:
>> On 02/20/2018 10:17 AM, Jakub Jelinek wrote:
>>> Hi!
>>>
>>> The following testcase distilled from pdftex is miscompiled on i?86,
>>> because gimple_fold_builtin_strlen sets incorrect value range on
>>> strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where
>>> we can't determine anything about string length of something, so the
>>> right value range is [0, maximum_possible_strlen], but we actually used
>>> [2, INF].
>>>
>>> get_range_strlen has many modes, for get_maxval_strlen we check return
>>> value of get_range_strlen, don't set fuzzy and everything seems correct.
>>> Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen
>>> overload, which is used for warnings and recently also for
>>> gimple_fold_builtin_strlen which sets value ranges from it.  Unlike
>>> warnings, which can perhaps afford some heuristics and guessing, for
>>> gimple_fold_builtin_strlen we need to be 100% conservative.
>>>
>>> The thing that isn't handled conservatively is PHIs and COND_EXPR.
>>> The current code, if we can't figure one of the args out, for PHIs in
>>> fuzzy mode increases the *maxval value to +INF, but doesn't touch
>>> *minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and
>>> just
>>> returns false.  Unfortunately, changing that breaks
>>
>> It sounds like not setting *minlen is the problem then.  Attached
>> is a slightly smaller patch that fixes the bug with no testsuite
>> regressions on x86_64-linux.  How does it look to you?
> 
> A safer and even more conservative alternative that should be
> equivalent to your approach while avoiding the sprintf regressions
> is to add another mode to the function and have it clear *minlen
> as an option.  This lets the strlen code obtain the conservative
> lower bound without compromising the sprintf warnings.
Adding another mode to this function seems wrong to me -- it's already a
bit of a tangled mess to figure out.  ISTM that either we allow fuzzy
results or not.  For warnings, fuzzy rules. For code generation fuzzy is
strictly not allowed.

Is there some reason that will not work?

jeff

Reply via email to