On Wed, Jun 13, 2018 at 08:53:20AM -0600, Martin Sebor wrote:
> > I'm actually worried about the fold-const.c change and don't understand, why
> > it has been done at all in the context of this PR.
> >
> > case SAVE_EXPR:
> > if (flags & OEP_LEXICOGRAPHIC)
> > return OP_SAME (0);
> > return 0;
> >
> > means it does something different from the state before the patch:
> > default:
> > return 0;
> > only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
> > -Wduplicated-branches warning code, so why it is needed for
> > -Wstringop-truncation warning is unclear to me.
>
> It lets sizeof_pointer_memaccess_warning detect that sizeof b
> refers to the size of the VLA b in cases like the one below
> (the sizeof b expression is represented as SAVE_EXPR (n)) and
> avoid false positives:
I've missed you've added another operand_equal_p call with
OEP_LEXICOGRAPHIC, but you are using it for something it hasn't been
designed for.
Say if the array sizes are not n, but foo (n) for some
unsigned foo (unsigned). That function can return completely different
values between the two calls, while operand_equal_p with OEP_LEXICOGRAPHIC
will still say they are the same. That is fine for -Wduplicated-branches
it has been designed for.
Or consider
char a[n];
n += 3;
char b[n];
f (a, b);
strncpy (a, b, sizeof b);
f (a);
The lengths are even known not to be equal here, but OEP_LEXICOGRAPHIC
happily says they are the same. On the other hand, you could have:
unsigned n2 = n;
char a[n], b[n2];
and OEP_LEXICOGRAPHIC would say they are not equal, even when they are equal
at runtime.
> void f (void*, ...);
>
> void g (unsigned n)
> {
> char a[n], b[n];
> f (a, b);
> strncpy (a, b, sizeof b); // bogus -Wsizeof-pointer-memaccess
> f (a);
> }
>
> Short of moving the SAVE_EXPR logic out of operand_equal_p()
> and into sizeof_pointer_memaccess_warning, is there a better
> way to determine that? (I would expect the SAVE_EXPR logic
> in operand_equal_p() to be useful in other contexts besides
> this warning.)
Only -Wduplicated-branches (and now your code too) are using
OEP_LEXICOGRAPHIC, nothing else, so it doesn't make a difference for other
uses. There we only care about SAVE_EXPR pointer equality, if they aren't
pointer equal, they might, but might not evaluate the same.
If you want to be conservative with the warning, then just don't warn if
sizeof doesn't return a constant.
Jakub
