Re: [PATCH] consider MIN_EXPR in get_size_range() (PR 85888)

[Martin Sebor <msebor at gmail dot com>]

On 05/24/2018 03:39 AM, Richard Biener wrote:
> On Thu, May 24, 2018 at 12:50 AM Martin Sebor <msebor@gmail.com> wrote:
>
> > The attached patch enhances the get_size_range() function to
> > extract more accurate ranges out of MIN_EXPR and MAX_EXPR nodes
> > with SSA_NAME operand(s).  This helps -Wstringop-overflow avoid
> > false positives on some targets in cases like some of those
> > reported in bug 85623 - strncmp() warns about attribute nonstring
> > incorrectly in -Wstringop-overflow
>
> > When first trying to expand calls to __builtin_strncmp, most back
> > ends succeed even when the expansion results in a call to the library
> > function.  The powerpc64 back-end, however, fails and and allows
> > the generic expansion to take place.  There is a subtle difference
> > between the two cases that shows up when examining the bound of
> > the strncmp call expression (the third argument): in the original
> > call expression (in expand_builtin_strncmp) the bound is or can be
> > an SSA_NAME.  But when the early expansion fails,
> > expand_builtin_strncmp transforms the original call expression into
> > a new one, substituting MIN_EXPR (bound, CST) for the bound where
> > CST is the constant result of strlen (STR), with STR being either
> > the first or second strncmp argument.  When the bound is an SSA_NAME
> > the subsequent attempt to determine its range from the MIN_EXPR fails.
>
> Hmm, wouldn't sth extracted from the attached random patch I have lying
> around be more useful in general?  That is, refactor the GENERIC
> expression walk in determine_value_range_1 to sth that can be used
> in the current get_range_info () interface?  Or if we don't want to change
> that to accept non-SSA names add a corresponding get_expr_range_info ()
> interface.

Thanks.  It is certainly more general.  I'm just not sure how much
use the generality can be put to because...

> If you do that please cut out the dominator/loop condition handling and
> refrain from the idea of looking at SSA def stmts.

...I've done that but I'm having trouble exercising the code except
for this bug.  (I've run the C testsuite but the only tests that end
up in it call it with uninteresting arguments like VAR_DECL).  Do
you have any suggestions?

Martin

PS I was able to create a test that at -O0 could call it with
a more interesting argument such as:

  const __SIZE_TYPE__ i = 3, j = 5;

  int f (void)
  {
    return __builtin_strncmp ("1234", "123", i < j ? i : j);
  }

Here, the first call to gimple_fold_builtin_string_compare() that
I picked as a test case sees:

  j.0_1 = 5;
  i.1_2 = 3;
  _3 = MIN_EXPR <j.0_1, i.1_2>;
  D.1963 = __builtin_strncmp ("1234", "123", _3);

Above, _3's range info is not available yet at this point so
the expression could be evaluated but only by looking at def stmts.
Why is it not a good idea?

Martin