From patchwork Thu Sep 19 20:27:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 1164865 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-509328-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="H4ncj5AP"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="NsHIodLG"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46Z7hZ3G8Nz9sNk for ; Fri, 20 Sep 2019 06:28:12 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=DOMJEnoss8gZg3vH8RgRBs3ljQIQD0HIV3kGznv57wRVc9OMU2 ydCJX1/2nNpYDukvxWxYemdJtO/RISA0ob+hAVQ+wZbGJlI0ojPaO/HNpARNZANY GzMblajTlDO7r/QzSw3qaQ38KABe6YzmYme0pmWuTIu0hqSyYOSrsylRA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=iBaW3PM8KtvV6z5tAdfL17Kc/ls=; b=H4ncj5APK9qSDdR2wctg C9wVziptC3Uo4zm7P9B8fJDnvDNnbwp0OuaYD7KkDbjdepHNDwz892ZHOff6s0vR F0R7szpMP9NA3LRVfuO8H5wycNhFMT1tm1Yh8fugBK6GzuQVMndp4DO4SHANymLJ DJvEPWrOtCj0xqAnZZoOknk= Received: (qmail 12082 invoked by alias); 19 Sep 2019 20:28:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 12068 invoked by uid 89); 19 Sep 2019 20:28:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=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= X-HELO: mail-wr1-f53.google.com Received: from mail-wr1-f53.google.com (HELO mail-wr1-f53.google.com) (209.85.221.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Sep 2019 20:28:01 +0000 Received: by mail-wr1-f53.google.com with SMTP id q17so4462426wrx.10; Thu, 19 Sep 2019 13:28:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=/rRgAPLq+YzbYragqYOQuKep4LAVpjSkvlEHQDikesQ=; b=NsHIodLGMH0W4KHoF2MG6Ev83Bjz37cbPh1WLJQSgyDFIktrMz+S7fvLA8l1iDMbzB LEScB4LRMZA9Gb0cFLXgUQ44E8/pYbScyhp+bCztIXM6SHJPPgqBbVpzlzV3AFb7x/r8 vj2X9fpTRMpp+hp+a2PO+7ZHh+yBcIcfZCVYsGZ5MVTSc8yOPh+0DsV6q+JkA+jEQDeZ +0A1sEv0f0dpj90mPPbN7Qs66k+iynibU9/LoQSdbJdtABs8PlKFQKUIusMateUfFcij Qz6ZlAzrbdWnhFZVmQNIAupv5i/r6nZ1w57ACeRpuww3wyX09XIwM7H1BS6X0/xKYeO8 6elg== Received: from ?IPv6:2a01:e0a:1dc:b1c0:5cff:194e:8ce7:fb2f? ([2a01:e0a:1dc:b1c0:5cff:194e:8ce7:fb2f]) by smtp.googlemail.com with ESMTPSA id t6sm11075334wmf.8.2019.09.19.13.27.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Sep 2019 13:27:58 -0700 (PDT) To: "libstdc++@gcc.gnu.org" , gcc-patches From: =?utf-8?q?Fran=C3=A7ois_Dumont?= Subject: [PATCH] Help compiler detect invalid code Message-ID: <368143e7-e151-9e37-f055-065044a57e7a@gmail.com> Date: Thu, 19 Sep 2019 22:27:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 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 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 _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 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()); 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 +// . + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include +#include + +constexpr bool +test1() +{ + 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(test1()); // { dg-error "outside the bounds" } + +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); + + // 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 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'" }