[Bug c++/94985] New: False-positive -Warray-bounds for char[1] on a non-zero offset in a referenced buffer

[Bug c++/94985] New: False-positive -Warray-bounds for char[1] on a non-zero offset in a referenced buffer

[joeyjyliu at gmail dot com]

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

            Bug ID: 94985
           Summary: False-positive -Warray-bounds for char[1] on a
                    non-zero offset in a referenced buffer
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: joeyjyliu at gmail dot com
  Target Milestone: ---

Created attachment 48474
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48474&action=edit
patch for -Warray-bounds with test case

I think the assumption in builtin_memref::set_base_and_offset
(gimple-ssa-warn-restrict.c) is not necessarily correct. It is only applicable
for flexible arrays. Actually even for flexible arrays this check is
controversial as it assumes the pointer to the struct is always a pointer to
the array of structs.  

In the attachment, there is a test file. The incorrect warning is like below.

error: ‘char* strncpy(char*, const char*, size_t)’ offset 1 from the object at
‘<unknown>’ is out of the bounds of referenced subobject ‘cstr<1>::m_data’ with
type ‘char [1]’ at offset 1 [-Werror=array-bounds]
    8 |         ::strncpy( data(), src, n );
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~

Also in the attachment there is a suggested change. With that, the warning goes
away and the regression tests still pass. 

The original patch was sent to gcc-patches but I was advised to open a Bugzilla
ticket first. So here it is and the patch is modified to accommodate the
existing regression tests. 

$ g++ -v
Using built-in specs.
COLLECT_GCC=/local/usr/bin/g++
COLLECT_LTO_WRAPPER=/local/usr/libexec/gcc/x86_64-pc-linux-gnu/11.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --prefix=/local/usr --disable-multilib
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.0 20200505 (experimental) (GCC)

[Bug middle-end/94985] False-positive -Warray-bounds for char[1] on a non-zero offset in a referenced buffer

[msebor at gcc dot gnu.org]

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" }
}

[Bug middle-end/94985] False-positive -Warray-bounds for char[1] on a non-zero offset in a referenced buffer

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |msebor at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug middle-end/94985] False-positive -Warray-bounds for char[1] on a non-zero offset in a referenced buffer

[joeyjyliu at gmail dot com]

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

--- Comment #2 from Joey Liu <joeyjyliu at gmail dot com> ---
Just fyi, the patch attached in this ticket is slightly different than the
patch in the mailing list (I limit it to flexible array only). It can handle
the existing test cases (no regressions). However I do think we can do better
than that since it still can't deduce the correct refsize for my test case. For
now tree::component_ref_size just returns NULL_TREE as it can't really
distinguish the regular a[1] and the flexible array a[1].