Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Jun 13, 2018 at 09:58:49AM +0200, Jakub Jelinek wrote:
> I'm actually worried about the fold-const.c change and don't understand, why
> it has been done at all in the context of this PR.
> 
>         case SAVE_EXPR:
>           if (flags & OEP_LEXICOGRAPHIC)
>             return OP_SAME (0);
>           return 0;
> 
> means it does something different from the state before the patch:
> 	default:
> 	  return 0;
> only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
> -Wduplicated-branches warning code, so why it is needed for
> -Wstringop-truncation warning is unclear to me.  More importantly,
> especially -fsanitize=undefined has a tendency of creating trees
> which contain two or more uses of the same SAVE_EXPR at each level, and very
> deep nesting levels of such SAVE_EXPRs, so if we handle it without checking
> for duplicates, it results in exponential compile time complexity.
> So, if we really want to handle SAVE_EXPR here, we'd need to create some
> cache where we'd remember if in the same toplevel operand_equal_p call we've
> already compared two particular SAVE_EXPRs and what was the result.

Random testcase for -Wduplicated-branches -fsanitize=shift:
int
foo (int x, int y)
{
  if (x)
    y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
          << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
          << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
          << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1;
  else
    y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
          << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
          << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
          << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1;
  return y;
}

Though it seems we have that problem already in inchash::add_expr.  In that
case perhaps we could have a pointer to a hashmap in inchash::hash objects,
clear it in the ctors and destroy/clear in inchash::hash::end (), though we
have the add_commutative that has two separate hash objects.

	Jakub