This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Handle strncpy in tree-ssa-dse.c


On 7/22/19 2:01 AM, Richard Biener wrote:
> On Fri, Jul 19, 2019 at 7:04 PM Jeff Law <law@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 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]