[Bug c/106297] New: stringop-overflow misbehaviour on atomic

[Bug c/106297] New: stringop-overflow misbehaviour on atomic

[chipitsine at gmail dot com]

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

            Bug ID: 106297
           Summary: stringop-overflow misbehaviour on atomic
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chipitsine at gmail dot com
  Target Milestone: ---

repro steps

git clone https://github.com/haproxy/haproxy
cd haproxy

export CC=/path/to/gcc
make CC=$CC ERR=1 TARGET=linux-glibc 

error reported:

src/haproxy.c: In function ‘run_poll_loop’:
include/haproxy/atomic.h:428:39: error: ‘__atomic_load_8’ writing 8 bytes into
a region of size 0 overflows the destination [-Werror=stringop-overflow=]
  428 | #define _HA_ATOMIC_LOAD(val)          __atomic_load_n(val,
__ATOMIC_RELAXED)
      |                                      
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/haproxy.c:2843:46: note: in expansion of macro ‘_HA_ATOMIC_LOAD’
 2843 |                                         if
((_HA_ATOMIC_LOAD(&ha_tgroup_ctx[i].stopping_threads) &
ha_tgroup_info[i].threads_enabled) !=
      |                                              ^~~~~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.




error was reviewed by Willy Tarreau in
https://github.com/haproxy/haproxy/issues/1767 and it is considered as false
positive.

I bisected gcc, breaking change is:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=88b504b7a8c5affb0ffa97990d22af2b199e36ed

[Bug tree-optimization/106297] stringop-overflow misbehaviour on atomic

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-07-14
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Can you attach the preprocessed source as requested on
https://gcc.gnu.org/bugs/ ?

[Bug tree-optimization/106297] stringop-overflow misbehaviour on atomic

[chipitsine at gmail dot com]

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

--- Comment #2 from Илья Шипицин <chipitsine at gmail dot com> ---
I have provided repro steps, hope they can be used to find any answer.

I tried to add "-save-temps", but it gave another error (not seen without that
option):

  CC      src/slz.o
In file included from src/slz.c:29:
include/import/slz-tables.h: In function ‘dist_to_code’:
include/import/slz-tables.h:182:35: error: this statement may fall through
[-Werror=implicit-fallthrough=]
  182 |         case 24577 ... 32768: code++; /* fall through */
      |                               ~~~~^~
compilation terminated due to -Wfatal-errors.
cc1: all warnings being treated as errors
make: *** [Makefile:1004: src/slz.o] Error 1

[Bug tree-optimization/106297] [12/13 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
                 CC|                            |marxin at gcc dot gnu.org
            Summary|stringop-overflow           |[12/13 Regression]
                   |misbehaviour on atomic      |stringop-overflow
                   |                            |misbehaviour on atomic
                   |                            |since
                   |                            |r12-4725-g88b504b7a8c5affb

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-4725-g88b504b7a8c5affb.

$ gcc haproxy.i -c -Wno-address-of-packed-member -O2 -Werror
haproxy.i: In function ‘run_poll_loop’:
haproxy.i:32760:11: error: ‘__atomic_load_8’ writing 8 bytes into a region of
size 0 overflows the destination [-Werror=stringop-overflow=]
32760 |      if ((__atomic_load_n(&ha_tgroup_ctx[i].stopping_threads, 0) &
ha_tgroup_info[i].threads_enabled) !=
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
haproxy.i:11985:26: note: at offset 80 into destination object ‘ha_tgroup_ctx’
of size 64
11985 | extern struct tgroup_ctx ha_tgroup_ctx[1];
      |                          ^~~~~~~~~~~~~
cc1: all warnings being treated as errors

[Bug tree-optimization/106297] [12/13 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 53301
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53301&action=edit
Unreduced test-case

[Bug tree-optimization/106297] [12/13 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.2

[Bug tree-optimization/106297] [12/13 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
           Keywords|                            |needs-reduction

[Bug tree-optimization/106297] [12/13 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-reduction             |
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This boils down to:
struct S { long a, b, c; char d[0], e[0]; };
extern struct S s[1];

int
foo (int n)
{
  int i;
  for (i = 0; i < n; i++)
    if ((__atomic_load_n (&s[i].c, 0) & s[i].b) == s[i].b)
      break;
  return i;
}

As the array has just a single element, this is UB if n > 1, but for some
reason we decide in the cunroll pass to completely unroll the loop.  The first
iteration is full, the second one has the __atomic_load_8 call plus
__builtin_unreachable right after it.
Bet we only think that s[i_14].b for i_14 1 will be UB, while __atomic_load_8
(&s[i].c, 0) is fine.  Even that is UB, even just forming the address, and even
if not - say the atomic would be on &s[i].a - it is accessing that member.
Though, for the atomic load we have
  _8 = (sizetype) i_14;
  _7 = _8 * 24;
  _15 = _7 + 16;
  _1 = &s + _15;
  _2 = __atomic_load_8 (_1, 0);
in the IL while for the load everything in one stmt:
  _4 = s[i_14].b;

The warning is on dead code here, the second __atomic_load_8 which would happen
only if  is > 1.

[Bug tree-optimization/106297] [12/13 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |12.1.0, 13.0
             Blocks|                            |88443
           Keywords|                            |missed-optimization

--- Comment #6 from Martin Sebor <msebor at gcc dot gnu.org> ---
There have been other reports of false positives due to the same issue (e.g.,
some or all of pr65461, pr92539, pr92955, pr95140, and pr96447).  Since the
unrolling pass uses the invalid access to decide to unroll the loop maybe it
could insert the __builtin_unreachable() call before it (or instead of it)
rather than after it.  That way the bad access would get eliminated and the
warning avoided.  Or, it could, in addition to inserting the
__builtin_unreachable() call after it, also suppress the access warning for the
bad statement.  Alternatively, these problems could be worked around in the
warning code by suppressing it in basic blocks that terminate by a call to
unreachable.  But this would cause false negatives where the unreachable call
is added after real problems in the user's source).

Until this is solved in GCC it can be dealt with in user code by asserting the
loop doesn't iterate more times than there are elements in the array.  In the
test case in comment #5 that might look like so:

  if (n >= sizeof s / sizeof *s)
    __builtin_unreachable ();


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443
[Bug 88443] [meta-bug] bogus/missing -Wstringop-overflow warnings

[Bug tree-optimization/106297] [12/13/14 Regression] stringop-overflow misbehaviour on atomic since r12-4725-g88b504b7a8c5affb

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.