On 11/16/2018 01:46 AM, Richard Biener wrote:
On Fri, Nov 16, 2018 at 4:09 AM Martin Sebor <mse...@gmail.com> wrote:
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01934.html
Do I need to change something in this patch to make it acceptable
or should I assume we will leave this bug in GCC unfixed?
So the issue is the next_stmt handling because for the _next_ stmt
we did not yet replace uses with lattice values. This information
was missing all the time along (and absent from patch context).
I notice the next_stmt handling is incredibly fragile and it doesn't
even check the store you identify as thouching the same object
writes a '\0', nor does it verify the store writes to a position after
the strncpy accessed area (but eventually anywhere is OK...).
Yes, this is being tracked in bug 84396. I have been planing
to tighten it up to check that it is, in fact, the NUL character
being inserted but other things have been getting in the way (like
trying to fix this bug).
So I really wonder why there's the restriction on 1:1 equality of the
base. That relies on proper CSE (as you saw and tried to work-around
in your patch) and more.
So what I'd do is the attached. Apply more leeway (and less at the
same time) and restrict it to stores from zero but allow any aliasing
one. One could build up more precision by, instead of using
ptr_may_alias_ref_p use refs_may_alias_p and build up a ao_ref
for the strncpy destination using ao_ref_from_ptr_and_size, but
I didn't bother to figure out what constraint on len the function
computed up to this point.
The patch fixes the testcase.
It does, but it also introduces two regressions into the test
suite (false negatives). The code your patch removes is there
to handle these cases. I'll look into your suggestion to use
refs_may_alias_p to avoid these regressions.
Martin
PS With the suggested patch GCC fails to detect the following:
struct A { char str[3]; };
struct B { struct A a[3]; int i; };
struct C { struct B b[3]; int i; };
void f (struct B *p, const char *s)
{
// write into p->a[0]:
__builtin_strncpy (p->a[0].str, s, sizeof p->a[0].str);
// write into p->a[1]:
p->a[1].str[sizeof p->a[1].str - 1] = 0;
}
