Help compiler detect invalid code
diff mbox series

Message ID 368143e7-e151-9e37-f055-065044a57e7a@gmail.com
State New
Headers show
Series
  • Help compiler detect invalid code
Related show

Commit Message

François Dumont Sept. 19, 2019, 8:27 p.m. UTC
Hi

     I start working on making recently added constexpr tests to work in 
Debug mode.

     It appears that the compiler is able to detect code mistakes pretty 
well as long we don't try to hide the code intention with a defensive 
approach. This is why I'd like to propose to replace '__n > 0' 
conditions with '__n != 0'.

     The result is demonstrated by the constexpr_neg.cc tests. What do 
you think ?

     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
     (__memmove): Loop as long as __n is not 0.
     (__copy_move<>::__copy_m): Likewise.
     (__copy_move_backward<>::__copy_move_b): Likewise.
     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.

     I'll submit the patch to fix Debug mode depending on the decision 
for this one.

François

Comments

François Dumont Sept. 20, 2019, 5:08 a.m. UTC | #1
I already realized that previous patch will be too controversial to be 
accepted.

In this new version I just implement a real memmove in __memmove so that 
in copy_backward there is no need for a shortcut to a more defensive code.

I'll see if in Debug mode I can do something.

François


On 9/19/19 10:27 PM, François Dumont wrote:
> Hi
>
>     I start working on making recently added constexpr tests to work 
> in Debug mode.
>
>     It appears that the compiler is able to detect code mistakes 
> pretty well as long we don't try to hide the code intention with a 
> defensive approach. This is why I'd like to propose to replace '__n > 
> 0' conditions with '__n != 0'.
>
>     The result is demonstrated by the constexpr_neg.cc tests. What do 
> you think ?
>
>     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
>     (__memmove): Loop as long as __n is not 0.
>     (__copy_move<>::__copy_m): Likewise.
>     (__copy_move_backward<>::__copy_move_b): Likewise.
>     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
> values.
>     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
>     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
>     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
>
>     I'll submit the patch to fix Debug mode depending on the decision 
> for this one.
>
> François
>
François Dumont Sept. 25, 2019, 8:40 p.m. UTC | #2
Some more tests have revealed  a small problem in is_sorted test. It was 
revealed by the Debug mode but not in a clean ways so for the moment I 
prefer to fix it and I'll add a _neg test when Debug is able to report 
it correctly.

I've also added a _neg test for equal which doesn't need Debug mode.

OK to commit ?

François



On 9/20/19 7:08 AM, François Dumont wrote:
> I already realized that previous patch will be too controversial to be 
> accepted.
>
> In this new version I just implement a real memmove in __memmove so 
> that in copy_backward there is no need for a shortcut to a more 
> defensive code.
>
> I'll see if in Debug mode I can do something.
>
> François
>
>
> On 9/19/19 10:27 PM, François Dumont wrote:
>> Hi
>>
>>     I start working on making recently added constexpr tests to work 
>> in Debug mode.
>>
>>     It appears that the compiler is able to detect code mistakes 
>> pretty well as long we don't try to hide the code intention with a 
>> defensive approach. This is why I'd like to propose to replace '__n > 
>> 0' conditions with '__n != 0'.
>>
>>     The result is demonstrated by the constexpr_neg.cc tests. What do 
>> you think ?
>>
>>     * include/bits/stl_algobase.h (__memmove): Return _Tp*.
>>     (__memmove): Loop as long as __n is not 0.
>>     (__copy_move<>::__copy_m): Likewise.
>>     (__copy_move_backward<>::__copy_move_b): Likewise.
>>     * testsuite/25_algorithms/copy/constexpr.cc: Add check on copied 
>> values.
>>     * testsuite/25_algorithms/copy_backward/constexpr.cc: Likewise.
>>     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
>>     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
>>
>>     I'll submit the patch to fix Debug mode depending on the decision 
>> for this one.
>>
>> François
>>
>
Jonathan Wakely Sept. 27, 2019, 11:24 a.m. UTC | #3
On 20/09/19 07:08 +0200, François Dumont wrote:
>I already realized that previous patch will be too controversial to be 
>accepted.
>
>In this new version I just implement a real memmove in __memmove so 

A real memmove doesn't just work backwards, it needs to detect any
overlaps and work forwards *or* backwards, as needed.

I think your change will break this case:

#include <algorithm>

constexpr int f(int i, int j, int k)
{
  int arr[5] = { 0, 0, i, j, k };
  std::move(arr+2, arr+5, arr);
  return arr[0] + arr[1] + arr[2];
}

static_assert( f(1, 2, 3) == 6 );

This is valid because std::move only requires that the result iterator
is not in the input range, but it's OK for the two ranges to overlap.

I haven't tested it, but I think with your change the array will end
up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
Jonathan Wakely Sept. 27, 2019, 12:11 p.m. UTC | #4
On 19/09/19 22:27 +0200, François Dumont wrote:
>Hi
>
>    I start working on making recently added constexpr tests to work 
>in Debug mode.

The attached patch seems to be necessary for that, right?
François Dumont Sept. 27, 2019, 4:24 p.m. UTC | #5
On 9/27/19 2:11 PM, Jonathan Wakely wrote:
> On 19/09/19 22:27 +0200, François Dumont wrote:
>> Hi
>>
>>     I start working on making recently added constexpr tests to work 
>> in Debug mode.
>
> The attached patch seems to be necessary for that, right?
>
>
On my side I had done this, almost the same.

For the moment there is a FIXME in macros.h to find out how to generate 
a nice compilation error when the condition is not meant.

static_assert can't be called in this context, too bad.

I also try to define a function with a 
__attribute__((__error__("because"))) attribute. But when I make it 
constexpr gcc complains about missing definition. When I provide a 
definition gcc complains that this attribute must be on a declaration. 
And when I split declaration and definition gcc does not produce the 
expected compilation error.

Unless you have the solution I consider that we need help from the 
front-end.

For the moment if Debug mode finds a problem it will be reported as 
_M_error function not being constexpr !

François
Jonathan Wakely Sept. 27, 2019, 4:45 p.m. UTC | #6
On 27/09/19 18:24 +0200, François Dumont wrote:
>On 9/27/19 2:11 PM, Jonathan Wakely wrote:
>>On 19/09/19 22:27 +0200, François Dumont wrote:
>>>Hi
>>>
>>>    I start working on making recently added constexpr tests to 
>>>work in Debug mode.
>>
>>The attached patch seems to be necessary for that, right?
>>
>>
>On my side I had done this, almost the same.
>
>For the moment there is a FIXME in macros.h to find out how to 
>generate a nice compilation error when the condition is not meant.
>
>static_assert can't be called in this context, too bad.
>
>I also try to define a function with a 
>__attribute__((__error__("because"))) attribute. But when I make it 
>constexpr gcc complains about missing definition. When I provide a 
>definition gcc complains that this attribute must be on a declaration. 
>And when I split declaration and definition gcc does not produce the 
>expected compilation error.

Yes, I've tried similar things without success.

>Unless you have the solution I consider that we need help from the 
>front-end.
>
>For the moment if Debug mode finds a problem it will be reported as 
>_M_error function not being constexpr !

A reasonable workaround is to do:

#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
  if (__builtin_is_constant_evaluated())
    asm("Debug Mode assertion failed");
  else
#endif
  if (!(Cond))
    __gnu_debug::_Error_formatter::...

The builtin is available even for C++98, whereas
std::is_constant_evaluated() is only available for C++20.

This produces errors that include lines like:

asm.cc:12:17:   in ‘constexpr’ expansion of ‘f(-1)’
asm.cc:4:7: error: inline assembly is not a constant expression
    4 |       asm("debug mode assertion failed");
      |       ^~~
asm.cc:8:3: note: in expansion of macro ‘CHECK’
    8 |   _GLIBCXX_ASSERT(i > 0);
      |   ^~~~~
asm.cc:4:7: note: only unevaluated inline assembly is allowed in a ‘constexpr’ function in C++2a
    4 |       asm("debug mode assertion failed");
      |       ^~~
asm.cc:8:3: note: in expansion of macro ‘CHECK’
    8 |   CHECK(i > 0);
      |   ^~~~~

It's not ideal, but it does show the failed condition and the text
"debug mode assertion failed" (or whatever message you choose to use
there).
François Dumont Oct. 1, 2019, 8:05 p.m. UTC | #7
On 9/27/19 1:24 PM, Jonathan Wakely wrote:
> On 20/09/19 07:08 +0200, François Dumont wrote:
>> I already realized that previous patch will be too controversial to 
>> be accepted.
>>
>> In this new version I just implement a real memmove in __memmove so 
>
> A real memmove doesn't just work backwards, it needs to detect any
> overlaps and work forwards *or* backwards, as needed.
ok, good to know, I understand now why using __builtin_memcopy didn't 
show any performance enhancement when I tested it !
>
> I think your change will break this case:
>
> #include <algorithm>
>
> constexpr int f(int i, int j, int k)
> {
>  int arr[5] = { 0, 0, i, j, k };
>  std::move(arr+2, arr+5, arr);
>  return arr[0] + arr[1] + arr[2];
> }
>
> static_assert( f(1, 2, 3) == 6 );
>
> This is valid because std::move only requires that the result iterator
> is not in the input range, but it's OK for the two ranges to overlap.
>
> I haven't tested it, but I think with your change the array will end
> up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
>
Indeed, I've added a std::move constexpr test in this new proposal which 
demonstrate that.

C++ Standard clearly states that [copy|move]_backward is done backward. 
So in this new proposal I propose to add a __memcopy used in copy/move 
and keep __memmove for *_backward algos. Both are using 
__builtin_memmove as before.


     * include/bits/stl_algobase.h (__memmove): Return void, loop as long as
     __n != 0.
     (__memcopy): New.
     (__copy_move<_IsMove, true, 
std::random_access_iterator_tag>::__copy_m):
     Adapt to use latter.
     (__copy_move_backward_a): Remove std::is_constant_evaluated block.
     * testsuite/25_algorithms/copy/constexpr.cc (test): Add check on copied
     values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc (test): Likewise
     and rename in test1.
     (test2): New.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
     * testsuite/25_algorithms/equal/constexpr_neg.cc: New.
     * testsuite/25_algorithms/move/constexpr.cc: New.
     * testsuite/25_algorithms/move/constexpr_neg.cc: New.

Tested under Linux x86_64.

Ok to commit ?

François
Jonathan Wakely Oct. 10, 2019, 8:03 p.m. UTC | #8
On 01/10/19 22:05 +0200, François Dumont wrote:
>On 9/27/19 1:24 PM, Jonathan Wakely wrote:
>>On 20/09/19 07:08 +0200, François Dumont wrote:
>>>I already realized that previous patch will be too controversial 
>>>to be accepted.
>>>
>>>In this new version I just implement a real memmove in __memmove 
>>>so
>>
>>A real memmove doesn't just work backwards, it needs to detect any
>>overlaps and work forwards *or* backwards, as needed.
>ok, good to know, I understand now why using __builtin_memcopy didn't 
>show any performance enhancement when I tested it !
>>
>>I think your change will break this case:
>>
>>#include <algorithm>
>>
>>constexpr int f(int i, int j, int k)
>>{
>> int arr[5] = { 0, 0, i, j, k };
>> std::move(arr+2, arr+5, arr);
>> return arr[0] + arr[1] + arr[2];
>>}
>>
>>static_assert( f(1, 2, 3) == 6 );
>>
>>This is valid because std::move only requires that the result iterator
>>is not in the input range, but it's OK for the two ranges to overlap.
>>
>>I haven't tested it, but I think with your change the array will end
>>up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
>>
>Indeed, I've added a std::move constexpr test in this new proposal 
>which demonstrate that.
>
>C++ Standard clearly states that [copy|move]_backward is done 
>backward. So in this new proposal I propose to add a __memcopy used in 
>copy/move and keep __memmove for *_backward algos. Both are using 
>__builtin_memmove as before.

Then they *really* need better names now (__memmove was already a bad
name, but now it's terrible). If the difference is that one goes
forwards and one goes backwards, the names should reflect that.

I'll review it properly tomorrow.
François Dumont Oct. 16, 2019, 8:22 p.m. UTC | #9
Here is a version with __detail::__copy and __detail::__copy_backward.

I prefered to introduce the __detail namespace cause __copy is quite a 
common name so putting it in __detail namespace will I hope clarify that 
it is for internal usage only.

I even hesitated to put more details into this namespace, maybe for 
another patch later.

     * include/bits/stl_algobase.h (__memmove): Replace by...
     (__detail::__copy) ...that. Return void, loop as long as __n != 0.
     (__copy_move<_IsMove, true, 
std::random_access_iterator_tag>::__copy_m):
     Adapt to use latter.
     (__detail::__copy_backward): New.
     (__copy_move_backward<_IsMove, true,
     std::random_access_iterator_tag>::__copy_m): Adapt to use latter.
     (__copy_move_backward_a): Remove std::is_constant_evaluated block.
     * testsuite/25_algorithms/copy/constexpr.cc (test): Add check on copied
     values.
     * testsuite/25_algorithms/copy_backward/constexpr.cc (test): Likewise
     and rename in test1.
     (test2): New.
     * testsuite/25_algorithms/copy/constexpr_neg.cc: New.
     * testsuite/25_algorithms/copy_backward/constexpr.cc: New.
     * testsuite/25_algorithms/equal/constexpr_neg.cc: New.
     * testsuite/25_algorithms/move/constexpr.cc: New.
     * testsuite/25_algorithms/move/constexpr_neg.cc: New.

François

On 10/10/19 10:03 PM, Jonathan Wakely wrote:
> On 01/10/19 22:05 +0200, François Dumont wrote:
>> On 9/27/19 1:24 PM, Jonathan Wakely wrote:
>>> On 20/09/19 07:08 +0200, François Dumont wrote:
>>>> I already realized that previous patch will be too controversial to 
>>>> be accepted.
>>>>
>>>> In this new version I just implement a real memmove in __memmove so
>>>
>>> A real memmove doesn't just work backwards, it needs to detect any
>>> overlaps and work forwards *or* backwards, as needed.
>> ok, good to know, I understand now why using __builtin_memcopy didn't 
>> show any performance enhancement when I tested it !
>>>
>>> I think your change will break this case:
>>>
>>> #include <algorithm>
>>>
>>> constexpr int f(int i, int j, int k)
>>> {
>>>  int arr[5] = { 0, 0, i, j, k };
>>>  std::move(arr+2, arr+5, arr);
>>>  return arr[0] + arr[1] + arr[2];
>>> }
>>>
>>> static_assert( f(1, 2, 3) == 6 );
>>>
>>> This is valid because std::move only requires that the result iterator
>>> is not in the input range, but it's OK for the two ranges to overlap.
>>>
>>> I haven't tested it, but I think with your change the array will end
>>> up containing {3, 2, 3, 2, 3} instead of {1, 2, 3, 2, 3}.
>>>
>> Indeed, I've added a std::move constexpr test in this new proposal 
>> which demonstrate that.
>>
>> C++ Standard clearly states that [copy|move]_backward is done 
>> backward. So in this new proposal I propose to add a __memcopy used 
>> in copy/move and keep __memmove for *_backward algos. Both are using 
>> __builtin_memmove as before.
>
> Then they *really* need better names now (__memmove was already a bad
> name, but now it's terrible). If the difference is that one goes
> forwards and one goes backwards, the names should reflect that.
>
> I'll review it properly tomorrow.
>
>

Patch
diff mbox series

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 4eba053ac75..33593197b4f 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -83,13 +83,13 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
    */
   template<bool _IsMove, typename _Tp>
     _GLIBCXX14_CONSTEXPR
-    inline void*
-    __memmove(_Tp* __dst, const _Tp* __src, size_t __num)
+    inline _Tp*
+    __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num)
     {
 #ifdef __cpp_lib_is_constant_evaluated
       if (std::is_constant_evaluated())
 	{
-	  for(; __num > 0; --__num)
+	  for (; __num != 0; --__num)
 	    {
 	      if constexpr (_IsMove)
 		*__dst = std::move(*__src);
@@ -100,10 +100,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    }
 	  return __dst;
 	}
-      else
 #endif
-	return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num);
-      return __dst;
+      return static_cast<_Tp*>(
+	__builtin_memmove(__dst, __src, sizeof(_Tp) * __num));
     }
 
   /*
@@ -398,7 +397,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__copy_m(_II __first, _II __last, _OI __result)
 	{
 	  typedef typename iterator_traits<_II>::difference_type _Distance;
-	  for(_Distance __n = __last - __first; __n > 0; --__n)
+	  for (_Distance __n = __last - __first; __n != 0; --__n)
 	    {
 	      *__result = *__first;
 	      ++__first;
@@ -418,7 +417,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__copy_m(_II __first, _II __last, _OI __result)
 	{
 	  typedef typename iterator_traits<_II>::difference_type _Distance;
-	  for(_Distance __n = __last - __first; __n > 0; --__n)
+	  for (_Distance __n = __last - __first; __n != 0; --__n)
 	    {
 	      *__result = std::move(*__first);
 	      ++__first;
@@ -446,8 +445,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 	  const ptrdiff_t _Num = __last - __first;
 	  if (_Num)
-	    std::__memmove<_IsMove>(__result, __first, _Num);
-	  return __result + _Num;
+	    return std::__memmove<_IsMove>(__result, __first, _Num);
+	  return __result;
 	}
     };
 
@@ -671,7 +670,7 @@  _GLIBCXX_END_NAMESPACE_CONTAINER
 	{
 	  typename iterator_traits<_BI1>::difference_type
 	    __n = __last - __first;
-	  for (; __n > 0; --__n)
+	  for (; __n != 0; --__n)
 	    *--__result = *--__last;
 	  return __result;
 	}
@@ -688,7 +687,7 @@  _GLIBCXX_END_NAMESPACE_CONTAINER
 	{
 	  typename iterator_traits<_BI1>::difference_type
 	    __n = __last - __first;
-	  for (; __n > 0; --__n)
+	  for (; __n != 0; --__n)
 	    *--__result = std::move(*--__last);
 	  return __result;
 	}
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
index 67910b8773e..a0460603496 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc
@@ -24,12 +24,12 @@ 
 constexpr bool
 test()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
   const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 2);
 
-  return out6 == ma0.begin() + 10;
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 2) == 1 && *out6 == 0;
 }
 
 static_assert(test());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
new file mode 100644
index 00000000000..34e20be97eb
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc
@@ -0,0 +1,50 @@ 
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test1()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2);
+
+  return out6 == ma0.begin() + 10;
+}
+
+static_assert(test1()); // { dg-error "outside the bounds" }
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out6 = std::copy(ma0.begin(), ma0.begin() + 8, ma0.begin() + 2);
+
+  // Check what should be the result if the range didn't overlap.
+  return out6 == ma0.begin() + 10 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2()); // { dg-error "static assertion failed" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
index ed7487905a8..c0e1a747832 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr.cc
@@ -22,15 +22,27 @@ 
 #include <array>
 
 constexpr bool
-test()
+test1()
 {
-  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  constexpr std::array<int, 12> ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}};
 
   std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
   const auto out7 = std::copy_backward(ca0.begin(), ca0.begin() + 8,
 				       ma0.begin() + 10);
 
-  return true;
+  return out7 == ma0.begin() + 2 && *out7 == 1 && *(ma0.begin() + 10) == 0;
 }
 
-static_assert(test());
+static_assert(test1());
+
+constexpr bool
+test2()
+{
+  std::array<int, 12> ma0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  const auto out7 = std::copy_backward(ma0.begin(), ma0.begin() + 8,
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2 && *out7 == 0 && *(ma0.begin() + 9) == 7;
+}
+
+static_assert(test2());
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
new file mode 100644
index 00000000000..3dc595d239f
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/constexpr_neg.cc
@@ -0,0 +1,39 @@ 
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
+
+#include <algorithm>
+#include <array>
+
+constexpr bool
+test()
+{
+  constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}};
+  std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+  const auto out7 = std::copy_backward(ca0.begin() + 8, ca0.begin(),
+				       ma0.begin() + 10);
+
+  return out7 == ma0.begin() + 2;
+}
+
+static_assert(test()); // { dg-error "outside the bounds" }
+
+// { dg-prune-output "non-constant condition" }
+// { dg-prune-output "in 'constexpr'" }