On 7/22/19 8:55 AM, Jeff Law wrote:
On 7/22/19 2:01 AM, Richard Biener wrote:
On Fri, Jul 19, 2019 at 7:04 PM Jeff Law <l...@redhat.com> wrote:
While looking at BZ 80576 I realized a few things.
First for STRNCPY we know the exact count of bytes written and we can
treat it just like MEMCPY and others, both in terms of removing/trimming
them and in terms of using them to allow removal of other stores.
This patch adds support for those routines in DSE. We test that
subsequent statements can make those calls dead and vice versa and that
we can trim from the head or tail appropriately.
While writing that code I also stumbled over a blob of code that I think
I copied from tree-ssa-alias.c that isn't necessary. In the relevant
code the byte count is always found in the same place. There's no need
to check the number of operands to the call to figure out where the
count would be. So that little blob of code is simplified ever so slightly.
Finally, while writing the tests for strncpy I stumbled over a case that
we're still not handling well.
In particular something like this:
void h (char *s)
{
extern char a[8];
__builtin_memset (a, 0, sizeof a);
__builtin_strncpy (a, s, sizeof a);
frob (a);
}
In this case ref_maybe_used_by_stmt_p returns true for the "a" array at
the strncpy call. AFAICT that appears to happen because "a" and "s"
could alias each other.
strncpy is documented as not allowing overlap between the source and
destination objects. So in theory we could consider them not aliasing
for this call. I haven't implemented this, but I've got some ideas
here.
But it does allow things like strncpy (&a[0], &a[n+1], n) for example?
Given that they're not allowed to overlap, I would think not. If that
were allowed then the code which aggressively transforms strncpy to
memcpy would need to be disabled (or at least throttled back) as well.
I think there's some (maybe too much) room for interpretation here
as to exactly what the sentence
If copying takes place between objects that overlap, the behavior
is undefined.
means. Taken to an extreme, one might say that the following
violates the requirement:
char a[4] = "123\0";
strcpy (a, a + 2);
because the call copies bytes within the same object (the array a)
which inevitably overlaps itself. But I'm pretty sure that's not
the intended interpretation -- the object itself does overlap but
not the bytes that are copied. (This is also the view -Wrestrict
takes.) If it were undefined, then so would be the following:
memcpy (a, a + 2, 2);
With that in mind, I would be inclined to expect the following not
to violate the requirement either:
strncpy (a, a + 2, 4);
because the bytes that are copied do not overlap. With a set to
"123\0" as done above it's equivalent to:
memcpy (a, a + 2, 2);
memset (a + 2, 0, 2);
Admittedly, the examples aren't exactly the same which makes this
question interesting. Is it worth raising an interpretation request
with WG14?
Martin
I think this kind of specialities should be handled in
ref_maybe_used_by_call_p_1
directly, but I'm not exactly sure how - it's probably another case of
a missing must-alias query, with that we could do
/* If REF overlaps with the strncpy destination then the source argument
may not alias it. */
if (refs_must_alias_p (ref_for_strncpy_dest, ref))
return false;
hmm, OTOH for REF covering &a[n/2] to &a[3*n/2] (half overlapping
source and destination) and the above strncpy we have a must-alias
(not kill!) of REF but also are using it.
So it's not so easy and would cover only very specific cases.
I'd been working under the assumption there would be nothing we could do
from a global standpoint in the alias oracle. Except for trivial
straightline functions, the call is always going to be in some kind of
control context. Thus the ability to exploit would be context dependent
on the control path leading to the call and not globally true.
With that in mind I was thinking that we could tackle in DSE.
Essentially asking if there's an overlap between the object we're
tracking for DSE and the destination of the str[n]cpy just before the
call to ref_maybe_used_by_stmt_p.
Obviously any such check has to avoid doing the wrong thing for memmove
and any other call where the source/destination are allowed to overlap.
That work would be a few patches deep in the queue of things I'm looking
at. First up is handling strcpy in tree-ssa-dse.c as well as
non-constant write sizes, neither of which are particularly complex.
Jeff
