Re: [PATCH] fix ICE in generic_overlap (PR 84526)

[Martin Sebor]

On 02/23/2018 02:54 PM, Jakub Jelinek wrote:

On Fri, Feb 23, 2018 at 02:46:28PM -0700, Martin Sebor wrote:

This doesn't address any of my concerns that it is completely random
what {dst,src}ref->base is, apples and oranges; sometimes it is a pointer
(e.g. the argument of the function), sometimes the ADDR_EXPR operand,
sometimes the base of the reference, sometimes again address (if the
base of the reference is MEM_REF).  By the lack of consistency in what
it is, just deciding on its type whether you take TREE_TYPE or
TREE_TYPE (TREE_TYPE ()) of it also gives useless result.  You could e.g
call the memcpy etc. function with ADDR_EXPR of a VAR_DECL that has pointer
type, then if dstref->base is that VAR_DECL, POINTER_TYPE_P (basetype)
would be true.

I think I understand what you're saying but this block is only
used for string functions (not for memcpy), and only as a stopgap
to avoid false positives.  Being limited to (a subset of) string
functions the case I think you're concerned about, namely calling
strcpy with a pointer to a pointer, shouldn't come up in valid
code.  It's not bullet-proof but I don't think there is

Can you explain what is invalid on:
char *p;

void
foo (void)
{
  if (sizeof (p) < 8)
    return;
  memcpy (&p, "abcdefg");
  strcpy ((char *) &p, (char *) &p + 5);
}

and similar code?  Both memcpy and strcpy are defined as char accesses
that can alias anything.  If needed tweak it so that you run into this code.

This is arguably valid but (I hope you will agree) questionable
at best.  I haven't seen any warnings for code like this (I've
looked at all those in Marek's Fedora rebuild), nor have I seen
such code in practice.  But if I did come across it I wouldn't
be too concerned about warnings (in fact, I'd view warning for
it as helpful since it's most likely a bug).

That said, the test case as is doesn't trigger -Wrestrict because
there is no overlap.  Even with overlap I could find no way to
exercise the code because the strlen pass doesn't handle the
MEM_REF that the call to memcpy is folded into early on and so
-Wrestrict doesn't know how long the string is.

Without a way to test it I don't feel comfortable making changes
here at this stage.

This isn't meant to say there aren't bugs here.  Just that I don't
know about any (none has been reported) and that I have known bugs
I do need to get to.  If/when we find more here as I'm sure we
will, I'll deal with them as I always do.

Martin