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: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)


On 06/13/2018 09:16 AM, Jakub Jelinek wrote:
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.

True.  -Wsizeof-pointer-memaccess is not perfect (irrespective
of this change).  It warns for "suspicious length parameters
to certain string and memory built-in functions if the argument
uses sizeof."  It's unavoidably prone to missing some problems
or flagging non-issues (such as bug 57627).


  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.

I don't think it's necessary to weaken the warning further.

Since nothing else uses OEP_LEXICOGRAPHIC, do you have any
other concerns about backporting the change to GCC 8?

Martin


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