[Bug tree-optimization/108374] New: [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[Bug tree-optimization/108374] New: [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[romain.geissler at amadeus dot com]

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

            Bug ID: 108374
           Summary: [12/13 Regression] unexpected -Wstringop-overflow when
                    using std::atomic and std::shared_ptr
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: romain.geissler at amadeus dot com
  Target Milestone: ---

Hi,

The following snippet produces an unexpected -Wstringop-overflow with gcc 12
and current trunk:

#include <atomic>
#include <memory>

struct A: public std::enable_shared_from_this<A>
{
    std::atomic<size_t> _attr;
};

void f(std::shared_ptr<A> pointer)
{
    std::weak_ptr<A> weakPointer(pointer);

    [[maybe_unused]] const unsigned int aAttr = weakPointer.lock()->_attr;
}


Compiler Explorer output:
In file included from
/opt/compiler-explorer/gcc-trunk-20230111/include/c++/13.0.0/atomic:41,
                 from <source>:1:
In member function 'std::__atomic_base<_IntTp>::__int_type
std::__atomic_base<_IntTp>::load(std::memory_order) const [with _ITp = long
unsigned int]',
    inlined from 'std::__atomic_base<_IntTp>::operator __int_type() const [with
_ITp = long unsigned int]' at
/opt/compiler-explorer/gcc-trunk-20230111/include/c++/13.0.0/bits/atomic_base.h:365:20,
    inlined from 'void f(std::shared_ptr<A>)' at <source>:13:69:
/opt/compiler-explorer/gcc-trunk-20230111/include/c++/13.0.0/bits/atomic_base.h:505:31:
error: 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8
bytes into a region of size 0 overflows the destination
[-Werror=stringop-overflow=]
  505 |         return __atomic_load_n(&_M_i, int(__m));
      |                ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
In function 'void f(std::shared_ptr<A>)':
cc1plus: note: destination object is likely at address zero
cc1plus: some warnings being treated as errors
Compiler returned: 1


I have found bug #104475 which seems to also deal with atomics and
-Wstringop-overflow however I can't judge if it looks like a duplicate or a
different issue.

Cheers,
Romain

[Bug tree-optimization/108374] [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[romain.geissler at amadeus dot com]

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

--- Comment #1 from Romain Geissler <romain.geissler at amadeus dot com> ---
I forgot to mention: this happens on x86-64 with -O1.

[Bug tree-optimization/108374] [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P2
   Target Milestone|---                         |12.3
   Last reconfirmed|                            |2023-01-12
     Ever confirmed|0                           |1
            Version|unknown                     |12.2.0

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
We now say

cc1plus: note: destination object is likely at address zero

<bb 2> [local count: 1073741824]:
_9 = MEM[(const struct __shared_ptr &)pointer_2(D)]._M_ptr;
_10 = MEM[(const struct __shared_count &)pointer_2(D) + 8]._M_pi;
if (_10 != 0B)
  goto <bb 4>; [53.47%]
else
  goto <bb 3>; [46.53%]

<bb 3> [local count: 499612072]:
__atomic_load_8 (16B, 5); [tail call]
D.37576 ={v} {CLOBBER};
D.37576 ={v} {CLOBBER(eol)};
goto <bb 19>; [100.00%] (leads straight to return)

so we have __shared_count of pointer _M_pi == 0 here, whatever that means
and not sure why we atomically load here.

Confirmed.

[Bug tree-optimization/108374] [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Romain Geissler from comment #0)
>     std::weak_ptr<A> weakPointer(pointer);
> 
>     [[maybe_unused]] const unsigned int aAttr = weakPointer.lock()->_attr;

If pointer == nullptr then weakPointer.lock() is also null, and so
dereferencing it to access the attr member is undefined, and does indeed
perform an atomic load at address 0.

Instead of complaining about it, I would expect GCC to treat that undefined
condition as unreachable and optimize it away.

[Bug tree-optimization/108374] [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #3)
> (In reply to Romain Geissler from comment #0)
> >     std::weak_ptr<A> weakPointer(pointer);
> > 
> >     [[maybe_unused]] const unsigned int aAttr = weakPointer.lock()->_attr;
> 
> If pointer == nullptr then weakPointer.lock() is also null, and so
> dereferencing it to access the attr member is undefined, and does indeed
> perform an atomic load at address 0.
> 
> Instead of complaining about it, I would expect GCC to treat that undefined
> condition as unreachable and optimize it away.

Hmm, but then the program is bogus, no?  And a diagnostic warranted.

At least if it is well-defined to have a nullptr == pointer.

So I'd be inclined to close as INVALID?

[Bug tree-optimization/108374] [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> Hmm, but then the program is bogus, no?  And a diagnostic warranted.

No.

> At least if it is well-defined to have a nullptr == pointer.

It's well defined, but that doesn't mean the program is bogus. It just means
you can't call that function with such a value.

> So I'd be inclined to close as INVALID?

No, I don't think so. It would only be invalid if you called f(nullptr) or
similar.

The code is basically doing something like:

int f(const A* p, bool is_valid)
{
  const A* q = is_valid ? p : nullptr;
  return *q;
}

Instead of complaining that q might be null, we can optimize that to return *p.

It might be nicer to optimize it to:

  if (!p)
    __builtin_trap();
  return *p;

but either way, we can't just declare the whole program to be invalid because
it's possible to call the function incorrectly.

[Bug tree-optimization/108374] [12/13 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The warning is simply badly designed like most of the middle-end warnings.

As has been discussed, we should have some switch to decide what to do when we
have such provable UB, and the possibilities should be turn it into
__builtin_unreachable (), turn it into __builtin_trap () or for users who don't
mind seeing lots of false positives keep the warning.  Though perhaps now that
we have -funreachable-traps the option could be just 2 possibilities,
unreachable vs. (useless) warnings, with the default of the former.

[Bug tree-optimization/108374] [12/13/14 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[rguenth at gcc dot gnu.org]

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

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

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

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