[Bug middle-end/109449] New: false positive stringop-overflow

[Bug middle-end/109449] New: false positive stringop-overflow

[pionere at freemail dot hu]

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

            Bug ID: 109449
           Summary: false positive stringop-overflow
           Product: gcc
           Version: 12.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pionere at freemail dot hu
  Target Milestone: ---

Compiling the following code:

#include <stdint.h>

#define DMAXX 40
#define DMAXY 40
#define DSIZEX 80
#define DSIZEY 80

typedef struct DrlgMem {
    union {
        uint8_t transvalMap[DMAXX][DMAXY];
        uint8_t transDirMap[DSIZEX][DSIZEY];
    };
} DrlgMem;

DrlgMem drlg;

void func()
{
    int i, j;
    uint8_t *tdp = &drlg.transDirMap[0][0];

    for (i = DMAXX - 1; i >= 0; i--) {
        for (j = DMAXY - 1; j >= 0; j--) {
            uint8_t tvm = drlg.transvalMap[i][j];
            uint8_t tpm = 0;
            // 1. subtile
            if (tvm & 1) {
                tpm = 14;
            } else {
                tpm = 0;
            }
            tdp[2 * i * DSIZEY + 2 * j] = tpm;
            // 3. subtile
            if (tvm & 4) {
                tpm = 25;
                if (tvm & (1 << 0)) // 1. subtile
                    tpm |= (1 << 1); // DIR_NW
                if (tvm & (1 << 3)) // 4. subtile
                    tpm |= (1 << 6); // DIR_SW
            } else {
                tpm = 0;
            }
            tdp[2 * i * DSIZEY + 2 * j + 1] = tpm;
            // 2. subtile
            if (tvm & 2) {
                tpm = 98;
            } else {
                tpm = 0;
            }
            tdp[(2 * i + 1) * DSIZEY + 2 * j] = tpm; // 1. warning here
            // 4. subtile
            if (tvm & 8) {
                tpm = 193;
            } else {
                tpm = 0;
            }
            tdp[(2 * i + 1) * DSIZEY + 2 * j + 1] = tpm; // 2. warning here
        }
    }
}

The result is multiple invalid warnings. E.g: '<source>:50:47: error: writing 1
byte into a region of size 0 [-Werror=stringop-overflow=]...'
Tried to reduce the sample code, but the warnings disappear even in case of
unrelated changes.

[Bug middle-end/109449] false positive stringop-overflow

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The trunk provides extra information:
<source>: In function 'void func()':
<source>:51:47: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
   51 |             tdp[(2 * i + 1) * DSIZEY + 2 * j] = tpm; // 1. warning here
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
<source>:16:9: note: at offset 80 into destination object 'drlg' of size 80
   16 | DrlgMem drlg;
      |         ^~~~
<source>:58:51: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
   58 |             tdp[(2 * i + 1) * DSIZEY + 2 * j + 1] = tpm; // 2. warning
here
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
<source>:16:9: note: at offset [81, 6399] into destination object 'drlg' of
size 80
   16 | DrlgMem drlg;
      |         ^~~~

[Bug middle-end/109449] false positive stringop-overflow

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am not 100% sure if this is not a incorrect warning. The question that needs
to be answered is the object located at &drlg.transDirMap[0][0] can only
validly be accessed to offset 80 or not.

[Bug middle-end/109449] false positive stringop-overflow

[pionere at freemail dot hu]

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

--- Comment #3 from pionere at freemail dot hu ---
(In reply to Andrew Pinski from comment #2)
> I am not 100% sure if this is not a incorrect warning. The question that
> needs to be answered is the object located at &drlg.transDirMap[0][0] can
> only validly be accessed to offset 80 or not.

Have you checked the definition of drlg? Its size is definitely larger than 80.
It is 80 * 80 bytes.

[Bug middle-end/109449] [11/12/13 Regression] false positive stringop-overflow

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
      Known to fail|                            |11.3.0
           Priority|P3                          |P2
     Ever confirmed|0                           |1
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
            Summary|false positive              |[11/12/13 Regression] false
                   |stringop-overflow           |positive stringop-overflow
      Known to work|                            |10.4.0
   Last reconfirmed|                            |2023-04-12
   Target Milestone|---                         |11.4

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think the code is correct.  What we see is for example

_11 = (int) ivtmp.16_45;
_12 = (sizetype) _11;
_13 = &drlg.D.2834.transDirMap[0][0] + _12;
*_13 = tpm_21;

and it seems to be confused about the computed object size from
pointer-query.cc
via handle_mem_ref of *_13 which eventually computes the size of
&drlg.D.2834.transDirMap[0][0] and then eventually does

static bool
handle_array_ref (tree aref, gimple *stmt, bool addr, int ostype,
                  access_ref *pref, ssa_name_limit_t &snlim,
                  pointer_query *qry)
{
...
  if (ostype && TREE_CODE (eltype) == ARRAY_TYPE)
    {
      /* Except for the permissive raw memory functions which use
         the size of the whole object determined above, use the size
         of the referenced array.  Because the overall offset is from
         the beginning of the complete array object add this overall
         offset to the size of array.  */
      offset_int sizrng[2] =
        {
         pref->offrng[0] + orng[0] + sz,
         pref->offrng[1] + orng[1] + sz
        };
      if (sizrng[1] < sizrng[0])
        std::swap (sizrng[0], sizrng[1]);
      if (sizrng[0] >= 0 && sizrng[0] <= pref->sizrng[0])
        pref->sizrng[0] = sizrng[0];
      if (sizrng[1] >= 0 && sizrng[1] <= pref->sizrng[1])
        pref->sizrng[1] = sizrng[1];

thus it thinks we offset transDirMap[0] and thus run out of that bound.

That's wishful thinking outside of any coding reality.  The following
fixes all of the bogus diagnostics.  But will likely regress some of the
testsuite, will check what.  There are likely duplicates of this bug.
I think it's definitely valid C and only in violation of some stricter
security-style coding rules and thus definitely shouldn't behave like this
by default.

diff --git a/gcc/pointer-query.cc b/gcc/pointer-query.cc
index 5b05e9bedf8..067e264fddb 100644
--- a/gcc/pointer-query.cc
+++ b/gcc/pointer-query.cc
@@ -1834,7 +1834,7 @@ handle_array_ref (tree aref, gimple *stmt, bool addr, int
ostype,
   orng[0] *= sz;
   orng[1] *= sz;

-  if (ostype && TREE_CODE (eltype) == ARRAY_TYPE)
+  if (!addr && ostype && TREE_CODE (eltype) == ARRAY_TYPE)
     {
       /* Except for the permissive raw memory functions which use
         the size of the whole object determined above, use the size

[Bug middle-end/109449] [11/12/13 Regression] false positive stringop-overflow

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
As expected that causes

FAIL: c-c++-common/Wstringop-truncation.c  -std=gnu++98 bug 77293 (test for
warn
ings, line 271)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
lin
e 49)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
lin
e 50)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
lin
e 51)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
lin
e 55)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
line 56)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
line 57)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
line 61)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
line 62)
FAIL: g++.dg/warn/Wplacement-new-size-7.C  -std=gnu++98  (test for warnings,
line 63)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 28)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 29)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 30)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 50)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 51)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 52)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 53)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 127)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 128)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 129)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 131)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 132)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 135)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 136)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 137)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 139)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 140)
FAIL: gcc.dg/Wstringop-overflow-37.c note (test for warnings, line 194)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 195)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 213)
FAIL: gcc.dg/Wstringop-overflow-37.c note (test for warnings, line 230)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 231)
FAIL: gcc.dg/Wstringop-overflow-37.c note (test for warnings, line 236)
FAIL: gcc.dg/Wstringop-overflow-37.c  (test for warnings, line 237)
FAIL: gcc.dg/Wstringop-overflow-37.c (test for excess errors)
FAIL: gcc.dg/Wstringop-overflow-40.c  (test for warnings, line 103)
FAIL: gcc.dg/Wstringop-overflow-40.c (test for excess errors)
FAIL: gcc.dg/pr56837.c scan-tree-dump-times optimized "memset ..c, 68, 16384.;"
1
FAIL: gcc.dg/warn-strnlen-no-nul.c  (test for warnings, line 150)
FAIL: gcc.dg/warn-strnlen-no-nul.c  (test for warnings, line 154)
FAIL: c-c++-common/Wstringop-truncation.c  -Wc++-compat  bug 77293 (test for
warnings, line 271)

as those all explicitely test for this "misbehavior" (aka _FORTIFY_SOURCE=2).
Note -Wstringop-overflow=1 diagnoses this the same.

The workaround in the source would be to use for example

    uint8_t *tdp = (uint8_t *)drlg.transDirMap;

or simply use two-dimensional array accesses directly to transDirMap.


So as a summary, the diagnostic works this way by design (a design not
everyone agrees to, me included, esp. for the case of multidimensional
arrays).

[Bug middle-end/109449] [11/12/13/14 Regression] false positive stringop-overflow

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |11.5

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.4 is being released, retargeting bugs to GCC 11.5.