https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94985

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-05-07
     Ever confirmed|0                           |1
           Keywords|                            |diagnostic
                 CC|                            |msebor at gcc dot gnu.org
          Component|c++                         |middle-end

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Thanks for opening the bug.  For reference, the gcc-patches thread is here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545280.html

As I said in response, I agree the warning in this instance (writing a single
byte into a one-element array) is a false positive, so confirmed.  But the fix
in the submitted patch is not correct because it both introduces testsuite
regressions and other false negatives, and doesn't prevent other similar false
positives.  A more comprehensive test with the expected diagnostics is below.

The root cause of the problem is that the builtin_memref::set_base_and_offset
(tree) function doesn't handle the sorts of cases where the access is to a
member of an object pointed to indirectly by some pointer.  The IL for the
simple test case below is:

  _1 = &q0_7(D)->a;
  q0_7(D)->p = _1;
  _2 = &MEM[(struct A1 *)q0_7(D) + 8B].a;

and set_base_and_offset() only considers A1::a when (for flexible array members
and arrays of size 0; I'm not sure we should cater to other array sizes) it
should look through the pointer and consider what it points to (i.e., look
through q0_7(D) and consider it points to &q0_7(D)->a).

The code in set_base_and_offset() is quite convoluted and should probably be
rewritten more cleanly.

That being said, the original test case, although valid, suggests it may be
derived from code that uses one-element trailing arrays as a substitute for
flexible array members (or zero-length arrays).  If that's so, GCC has been
increasingly diagnosing such misuses and so while I will work on fixing this
bug it could well be that other similar uses of the class will continue to be
diagnosed.

typedef struct A1
{
  char c;
  char a[1];
} A1;

typedef struct BA1
{
  struct A1 *p;
  char a[4];
} BA1;

void test_a1 (BA1 *q0, BA1 *q1, BA1 *q2, char *s)
{
  q0->p = (A1*)q0->a;
  __builtin_strncpy (q0->p->a, s, 1);   // { dg-bogus
"\\\[-Warray-bounds|-Wstringop-overflow" }

  q0->p = (A1*)q0->a;
  __builtin_strncpy (q0->p->a, s, 2);   // { dg-bogus
"\\\[-Warray-bounds|-Wstringop-overflow" }

  q0->p = (A1*)q0->a;
  __builtin_strncpy (q0->p->a, s, 3);   // { dg-bogus
"\\\[-Warray-bounds|-Wstringop-overflow" }

  q1->p = (A1*)q1->a;
  __builtin_strncpy (q1->p->a, s, 4);   // { dg-bogus
"\\\[-Warray-bounds|-Wstringop-overflow" }

  q2->p = (A1*)q2->a;
  __builtin_strncpy (q2->p->a, s, 5);   // { dg-warning
"\\\[-Warray-bounds|-Wstringop-overflow" }
}

struct A2
{
  char c;
  char a[2];
};

struct BA2
{
  struct A2 *p;
  char a[4];
};

void test_a2 (BA2 *q0, BA2 *q1, BA2 *q2, char *s)
{
  q0->p = (A2*)q0->a;
  __builtin_strncpy (q0->p->a, s, 1);   // { dg-bogus
"\\\[-Warray-bounds|-Wstringop-overflow" }

  q0->p = (A2*)q0->a;
  __builtin_strncpy (q0->p->a, s, 2);   // { dg-bogus
"\\\[-Warray-bounds|-Wstringop-overflow" }

  q0->p = (A2*)q0->a;
  __builtin_strncpy (q0->p->a, s, 3);   // { dg-warning
"\\\[-Warray-bounds|-Wstringop-overflow" }
}

Reply via email to