From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20668 invoked by alias); 20 Sep 2019 05:08:11 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 20650 invoked by uid 89); 20 Sep 2019 05:08:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=controversial, 2225 X-HELO: mail-wm1-f65.google.com Received: from mail-wm1-f65.google.com (HELO mail-wm1-f65.google.com) (209.85.128.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Sep 2019 05:08:09 +0000 Received: by mail-wm1-f65.google.com with SMTP id f16so832390wmb.2; Thu, 19 Sep 2019 22:08:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:references:message-id:date:user-agent:mime-version :in-reply-to:content-language; bh=ptZbvNlPeWNZ/xX3r+xueTNpiicVlxh4TBmD5QRzvis=; b=TdrsHOMKd9FBMJajpWJmDs0mF0ObGT1denhV0ZgH/BVmlLI+9eSHTCFnucI2zFAOFB b9PY0p4tGmYQ5iYliAU42JElJDm+VtLCr+8vX8LbodChNe1Td6e440bcEZBUWN1d9hzW nNE/JZi6IT4H54tD74I69TMw5FQvQ14IRB0ZMiVjWNOVZa2fl/BXLBexBvro+n6P+Np5 BVzgtoSpQeAUnoSVUYfWagFX6XdOYXdzgQkG+5A211T2pHBHCRSmW6I8uPark3w6MdDk 7rUTv1C5YNfZgAjONGz+B4jyhCkRqzluN0NsHZ9SoubYh6xClBQSWArg+cpHf/aGxZBx TMww== Return-Path: Received: from [10.13.4.45] ([109.190.253.11]) by smtp.googlemail.com with ESMTPSA id t13sm1538231wra.70.2019.09.19.22.08.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Sep 2019 22:08:05 -0700 (PDT) Subject: Re: [PATCH] Help compiler detect invalid code From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= To: "libstdc++@gcc.gnu.org" , gcc-patches References: <368143e7-e151-9e37-f055-065044a57e7a@gmail.com> Message-ID: <96b51704-67fe-3ac4-405c-6997151d926c@gmail.com> Date: Fri, 20 Sep 2019 05:08:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <368143e7-e151-9e37-f055-065044a57e7a@gmail.com> Content-Type: multipart/mixed; boundary="------------B1C02B28551691CE54951E0B" X-SW-Source: 2019-09/txt/msg01224.txt.bz2 This is a multi-part message in MIME format. --------------B1C02B28551691CE54951E0B Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 1402 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 > --------------B1C02B28551691CE54951E0B Content-Type: text/x-patch; name="constexpr_algo.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="constexpr_algo.patch" Content-length: 7195 diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h index 4eba053ac75..94a79b85d15 100644 --- a/libstdc++-v3/include/bits/stl_algobase.h +++ b/libstdc++-v3/include/bits/stl_algobase.h @@ -83,27 +83,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION */ template _GLIBCXX14_CONSTEXPR - inline void* - __memmove(_Tp* __dst, const _Tp* __src, size_t __num) + inline void + __memmove(_Tp* __dst, const _Tp* __src, ptrdiff_t __num) { #ifdef __cpp_lib_is_constant_evaluated if (std::is_constant_evaluated()) { - for(; __num > 0; --__num) + __dst += __num; + __src += __num; + for (; __num != 0; --__num) { if constexpr (_IsMove) - *__dst = std::move(*__src); + *--__dst = std::move(*--__src); else - *__dst = *__src; - ++__src; - ++__dst; + *--__dst = *--__src; } - return __dst; } else #endif - return __builtin_memmove(__dst, __src, sizeof(_Tp) * __num); - return __dst; + __builtin_memmove(__dst, __src, sizeof(_Tp) * __num); } /* @@ -730,12 +728,6 @@ _GLIBCXX_END_NAMESPACE_CONTAINER && __is_pointer<_BI2>::__value && __are_same<_ValueType1, _ValueType2>::__value); -#ifdef __cpp_lib_is_constant_evaluated - if (std::is_constant_evaluated()) - return std::__copy_move_backward::__copy_move_b(__first, __last, - __result); -#endif return std::__copy_move_backward<_IsMove, __simple, _Category>::__copy_move_b(__first, __last, diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc index 67910b8773e..fc70db6dae7 100644 --- a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc +++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr.cc @@ -22,14 +22,25 @@ #include constexpr bool -test() +test1() { - constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + constexpr std::array ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}}; std::array 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()); +static_assert(test1()); + +constexpr bool +test2() +{ + std::array 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); + + return out6 == ma0.begin() + 10 && *(ma0.begin() + 9) == 7; +} + +static_assert(test2()); 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..49052467409 --- /dev/null +++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc @@ -0,0 +1,38 @@ +// 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array 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(test()); // { dg-error "outside the bounds" } + +// { 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 constexpr bool -test() +test1() { - constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + constexpr std::array ca0{{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}}; std::array 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 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test() +{ + constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array 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'" } --------------B1C02B28551691CE54951E0B--