Re: Make tree-ssa-strlen.c handle partial unterminated strings

[Jeff Law <law at redhat dot com>]

On 05/05/2017 10:28 AM, Martin Sebor wrote:
> On 05/05/2017 09:55 AM, Jakub Jelinek wrote:
> > On Fri, May 05, 2017 at 08:50:04AM -0700, Andi Kleen wrote:
> > > Richard Sandiford <richard.sandiford@linaro.org> writes:
> > >
> > > > tree-ssa-strlen.c looks for cases in which a string is built up using
> > > > operations like:
> > > >
> > > >     memcpy (a, "foo", 4);
> > > >     memcpy (a + 3, "bar", 4);
> > > >     int x = strlen (a);
> > > >
> > > > As a side-effect, it optimises the non-final memcpys so that they don't
> > > > include the nul terminator.
> > > >
> > > > However, after removing some "& ~0x1"s from tree-ssa-dse.c, the DSE 
> > > > pass
> > > > does this optimisation itself (because it can tell that later memcpys
> > > > overwrite the terminators).  The strlen pass wasn't able to handle 
> > > > these
> > > > pre-optimised calls in the same way as the unoptimised ones.
> > > >
> > > > This patch adds support for tracking unterminated strings.
> > >
> > > Would that be useful as a warning too? If the pass can figure out
> > > the final string can be not null terminated when passed somewhere else,
> > > warn, because it's likely a bug in the program.
> >
> > Why would it be a bug?  Not all sequences of chars are zero terminated
> > strings, it can be arbitrary memory and have size somewhere on the side.
> > Also, the fact that strlen pass sees a memcpy (a, "foo", 3); and a passed
> > somewhere else doesn't mean a isn't zero terminated, the pass records 
> > only
> > what it can prove, so even when you have:
> > memcpy (a, "abcdefgh", 9);
> > *p = 0; // unrelated pointer, but compiler can't prove that
> > memcpy (a, "foo", 3);
> > call (a);
>
> There have been requests for a warning to diagnose invalid uses
> of character arrays that are not nul-terminated, such as arguments
> to functions that expect a (nul-terminated) string.  For example:
>
>      char *p = (char*)malloc (20);
>      memcpy (p, "/tmp/", 5);
>      strcat (p, "file.text");   // << warn here
>
> It would be helpful to diagnose such cases (while avoiding false
> positives on the indeterminate cases you mention, of course).
Can't we just start at point where the string must be terminated (strcat 
call in this case) and walk the VUSE/VDEF chain back to determine if 
object's NUL termination status is:

1. Terminated
2. Not terminated
3. Indeterminate

You'd probably want to have some kind of propagation step so that you 
don't just give up when you see a PHI node.  Instead you recurse on the 
PHI args before determining the state at the PHI itself.

Jeff