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

[jakub at gcc dot gnu.org via Gcc-bugs]

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

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek  ---
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 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

[redi at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #5 from Jonathan Wakely  ---
(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

[rguenth at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #4 from Richard Biener  ---
(In reply to Jonathan Wakely from comment #3)
> (In reply to Romain Geissler from comment #0)
> > std::weak_ptr 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 via Gcc-bugs]

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

--- Comment #3 from Jonathan Wakely  ---
(In reply to Romain Geissler from comment #0)
> std::weak_ptr 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 via Gcc-bugs]

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

Richard Biener  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  ---
We now say

cc1plus: note: destination object is likely at address zero

 [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 ; [53.47%]
else
  goto ; [46.53%]

 [local count: 499612072]:
__atomic_load_8 (16B, 5); [tail call]
D.37576 ={v} {CLOBBER};
D.37576 ={v} {CLOBBER(eol)};
goto ; [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

[romain.geissler at amadeus dot com via Gcc-bugs]

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

--- Comment #1 from Romain Geissler  ---
I forgot to mention: this happens on x86-64 with -O1.