string: Add a workaround for -Wstringop-overread false positives [PR98465]

Hi!

In the PR there are several possibilities how to improve _M_disjunct at
least in certain cases so that the compiler can figure out at least in some
cases where __s is provably disjunct from _M_data() ... _M_data() + this->size()
but it is probably GCC 12 material.

The false positive warning is on this particular copy, which is done for
non-disjunct pointers when __len2 > __len1 and the __s >= __p + __len1,
i.e. __s used to point to the characters moved through _S_move a few lines earlier
by __len2 - __len1 characters up to make space.  That is why the
_S_copy source is __s + __len2 - __len1.  Unfortunately, when the compiler
can't prove objects are disjunct, that copying from __s + __len2 - __len1
of __len2 characters can very well mean accessing characters the source
object (if it is not disjunct) provably can't have.

The following patch works around that by making the _S_copy be a __p based
pointer instead of __s based pointer.
__s + __len2 - __len1
and
__p + (__s - __p) + (__len2 - __len1)
have the same value and the latter may seem to be uselessly longer,
but it seems at least currently in GIMPLE we keep it that way and so that is
what the warning code during expansion will see, and only actually
optimize it to __s + __len2 - __len1 during RTL when we lose information
on what is a pointer and what is a mere offset with the same mode.

So, in the end we emit exactly the same assembly, just without the false
positive warning.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-02-09  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/98465
	* include/bits/basic_string.tcc (basic_string::_M_replace): When __s
	points to the characters moved by earlier _S_move, compute the source
	address using expression based on the __p pointer rather than __s
	pointer.

	* g++.dg/warn/Wstringop-overread-1.C: New test.


	Jakub

On 09/02/21 09:57 +0100, Jakub Jelinek wrote:
>Hi!
>
>In the PR there are several possibilities how to improve _M_disjunct at
>least in certain cases so that the compiler can figure out at least in some
>cases where __s is provably disjunct from _M_data() ... _M_data() + this->size()
>but it is probably GCC 12 material.
>
>The false positive warning is on this particular copy, which is done for
>non-disjunct pointers when __len2 > __len1 and the __s >= __p + __len1,
>i.e. __s used to point to the characters moved through _S_move a few lines earlier
>by __len2 - __len1 characters up to make space.  That is why the
>_S_copy source is __s + __len2 - __len1.  Unfortunately, when the compiler
>can't prove objects are disjunct, that copying from __s + __len2 - __len1
>of __len2 characters can very well mean accessing characters the source
>object (if it is not disjunct) provably can't have.
>
>The following patch works around that by making the _S_copy be a __p based
>pointer instead of __s based pointer.
>__s + __len2 - __len1
>and
>__p + (__s - __p) + (__len2 - __len1)
>have the same value and the latter may seem to be uselessly longer,
>but it seems at least currently in GIMPLE we keep it that way and so that is
>what the warning code during expansion will see, and only actually
>optimize it to __s + __len2 - __len1 during RTL when we lose information
>on what is a pointer and what is a mere offset with the same mode.
>
>So, in the end we emit exactly the same assembly, just without the false
>positive warning.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
>2021-02-09  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/98465
>	* include/bits/basic_string.tcc (basic_string::_M_replace): When __s
>	points to the characters moved by earlier _S_move, compute the source
>	address using expression based on the __p pointer rather than __s
>	pointer.
>
>	* g++.dg/warn/Wstringop-overread-1.C: New test.
>
>--- libstdc++-v3/include/bits/basic_string.tcc.jj	2021-02-04 18:15:05.317113098 +0100
>+++ libstdc++-v3/include/bits/basic_string.tcc	2021-02-08 19:39:16.864936344 +0100
>@@ -478,7 +478,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 		  if (__s + __len2 <= __p + __len1)
> 		    this->_S_move(__p, __s, __len2);
> 		  else if (__s >= __p + __len1)
>-		    this->_S_copy(__p, __s + __len2 - __len1, __len2);
>+		    {

Please add:

     // Hint to middle end that __p and __s overlap (PR 98465)


OK with that comment added, thanks.

>+		      const size_type __poff = (__s - __p) + (__len2 - __len1);
>+		      this->_S_copy(__p, __p + __poff, __len2);
>+		    }
> 		  else
> 		    {
> 		      const size_type __nleft = (__p + __len1) - __s;
>--- gcc/testsuite/g++.dg/warn/Wstringop-overread-1.C.jj	2021-02-08 19:47:15.486646129 +0100
>+++ gcc/testsuite/g++.dg/warn/Wstringop-overread-1.C	2021-02-08 19:46:16.118302317 +0100
>@@ -0,0 +1,12 @@
>+// PR middle-end/98994
>+// { dg-do compile }
>+// { dg-additional-options "-Wstringop-overread -O2" }
>+
>+#include <string>
>+
>+const char constantString[] = {42, 53};
>+
>+void f(std::string& s)
>+{
>+  s.insert(0, static_cast<const char*>(constantString), 2);
>+}
>
>	Jakub