From patchwork Fri Aug 11 22:37:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 134752 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b824:0:b0:3f2:4152:657d with SMTP id z4csp1395165vqi; Fri, 11 Aug 2023 15:38:27 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEdxZ+jcAytClukI3WYg7gze+4mDB3DJ3Ru6+2OglrT1toeBP0Fe7ob9lfHar/WU6V46rxT X-Received: by 2002:aa7:cc0e:0:b0:523:48d7:70b1 with SMTP id q14-20020aa7cc0e000000b0052348d770b1mr2700815edt.19.1691793507558; Fri, 11 Aug 2023 15:38:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691793507; cv=none; d=google.com; s=arc-20160816; b=t+d/ALp012NxkWdRifGONhKtycqoRhfhIKq6fl4T5wI33A+Il0Ztw1ApurcUZYfSeq f+PpvcrKtcgT64zDbuAn7Jae3hLAwUc0ZKfOyzfffuhgzADWbooiuY2N+4hAvyHe2LkP LlUTHRVdN1ed1SVkZphRUwTQDCE34UAb66r7IPWpWNZQ/yn6M+pwIkHjJCF8HMGjmbKc gaz6S73uhO7/HQQiF1psyFiE/zmxcft0M3kgICZfjaEmCDYC3AhXGDpwogfOK5xFJlMs TDSHmboMk4NYy3vcnHaOkLBClpFBwIdtKVpu0dSCBxA3h0juBjmtx6nQ7nhkDtvJQQ6o zQrQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=Ju8hPgnmJ4pu7eBWk3mJq2zUAS7JKwbDcBM3Q8jUkhk=; fh=GtCD7JBPkLOIY7Tn8ekFN6kLiAYVGjJ4uy4ogv1SEY8=; b=YDLhevGRRqGOkIUeMRJcZH/PHQPItIHG35zUhXEwwO16z3mIER9Ot5d3GGgtjnvBU/ ccILgbG36lBAtpMrDvZ4VraPkvU4/fSXxbdUdgOwgN/DuwIueJiRB1HY/+L3dmSEcCgH zadp455LFlmZJBzJ+2NC5SHJ23ZYrnfP/U2WfwhQ2ZDqprW4iPB6sdVtJtbCZ3MG+otU P3vAPBmInmAYJYxQOgBuSvijxz3KoM0ssAbpwe74wDG5yGUMx130grNJvBWxLEazQDqc kBhYZHCIVnvR//kmXzhsJkBqjJAeg5clSErrwM3ywjWW4ZQjX8qQ+b+UcpA0mL986Zfn gISg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=KOjl+Juf; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id w4-20020aa7da44000000b005233636f1ccsi3863700eds.600.2023.08.11.15.38.27 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Aug 2023 15:38:27 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=KOjl+Juf; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id ED58C3855587 for ; Fri, 11 Aug 2023 22:38:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ED58C3855587 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1691793489; bh=Ju8hPgnmJ4pu7eBWk3mJq2zUAS7JKwbDcBM3Q8jUkhk=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=KOjl+JufXnvRDDq5Q9kRMQ6yK2BTnExjVlwzI9AUJQgpIglzra8lqtRoKMxBtv7J2 baCR2y9KdtyXvZvZRDcDQHT/19Su+ZFomMxLvMHDoaWBM0re7yYGnXz2ULEJkyOMB6 vpNrVsy2zF1FzO5gcRimXZ9jt4/lKh2SMzF4sh8g= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2A4943858D32 for ; Fri, 11 Aug 2023 22:37:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2A4943858D32 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-97-qlYJiTCKOVCtA0T8hqLGyA-1; Fri, 11 Aug 2023 18:37:23 -0400 X-MC-Unique: qlYJiTCKOVCtA0T8hqLGyA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B3D6C85CBE6; Fri, 11 Aug 2023 22:37:22 +0000 (UTC) Received: from localhost (unknown [10.42.28.9]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7E76A40C2063; Fri, 11 Aug 2023 22:37:22 +0000 (UTC) To: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org Subject: [committed] libstdc++: Fix std::format_to_n return value [PR110990] Date: Fri, 11 Aug 2023 23:37:05 +0100 Message-ID: <20230811223721.34729-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773974069059885199 X-GMAIL-MSGID: 1773974069059885199 Tested x86_64-linux. Pushed to trunk. Backport to gcc-13 also needed. -- >8 -- When writing to a contiguous iterator, std::format_to_n(out, n, ...) always returns out + n, even if it wrote fewer than n characters to the iterator. The problem is in the _M_finish() member function of the _Iter_sink specialization for contiguous iterators. _M_finish() calls _M_overflow() to update its count of characters written, so it can return the count of characters that would be written if there was room. But _M_overflow() assumes it's only called when the buffer is full, and so switches to the internal buffer. _M_finish() then thinks that if the internal buffer is in use, we already wrote at least n characters and so returns out+n as the output position. We can fix the problem by adding a check in _M_overflow() so that we don't update the count and switch to the internal buffer unless we've run out of room, i.e. _M_unused().size() is zero. The caller then needs to be prepared for _M_count not being the final total, and so add _M_used.size() to it. However, there's not actually any need for _M_finish() to call _M_overflow() to get the count. We now need to use _M_count and _M_used.size() to get the total anyway so _M_overflow() doesn't help with that. And we don't need to use _M_overflow() to flush unwritten characters to the output, because the specialization for contiguous iterators always writes directly to the output without buffering (except when we've exceeded the maximum number of characters, in which case we want to discard the buffered characters anyway). So _M_finish() can be simplified and can avoid calling _M_overflow(). This change also fixes some member functions of other sink classes to only call _M_overflow() when there are characters in the buffer, which is needed to meet _M_overflow's precondition that _M_used().size()!=0. libstdc++-v3/ChangeLog: PR libstdc++/110990 * include/std/format (_Seq_sink::get): Only call _M_overflow if its precondition is met. (_Iter_sink::_M_finish): Likewise. (_Iter_sink::_M_overflow): Only switch to the internal buffer after running out of space. (_Iter_sink::_M_finish): Do not use _M_overflow. (_Counting_sink::count): Likewise. * testsuite/std/format/functions/format_to_n.cc: Check cases where the output fits into the buffer. --- libstdc++-v3/include/std/format | 37 +++++++++++-------- .../std/format/functions/format_to_n.cc | 17 +++++++++ 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format index 23da6b008c5..f4520ff3f74 100644 --- a/libstdc++-v3/include/std/format +++ b/libstdc++-v3/include/std/format @@ -2484,7 +2484,8 @@ namespace __format _Seq get() && { - _Seq_sink::_M_overflow(); + if (this->_M_used().size() != 0) + _Seq_sink::_M_overflow(); return std::move(_M_seq); } }; @@ -2540,7 +2541,8 @@ namespace __format format_to_n_result<_OutIter> _M_finish() && { - _Iter_sink::_M_overflow(); + if (this->_M_used().size() != 0) + _Iter_sink::_M_overflow(); iter_difference_t<_OutIter> __count(_M_count); return { std::move(_M_out), __count }; } @@ -2567,8 +2569,11 @@ namespace __format protected: void - _M_overflow() + _M_overflow() override { + if (this->_M_unused().size() != 0) + return; // No need to switch to internal buffer yet. + auto __s = this->_M_used(); _M_count += __s.size(); @@ -2632,15 +2637,18 @@ namespace __format format_to_n_result<_OutIter> _M_finish() && { - _Iter_sink::_M_overflow(); - iter_difference_t<_OutIter> __count(_M_count); auto __s = this->_M_used(); - auto __last = _M_first; - if (__s.data() == _M_buf) // Wrote at least _M_max characters. - __last += _M_max; - else - __last += iter_difference_t<_OutIter>(__s.size()); - return { __last, __count }; + if (__s.data() == _M_buf) + { + // Switched to internal buffer, so must have written _M_max. + iter_difference_t<_OutIter> __count(_M_count + __s.size()); + return { _M_first + _M_max, __count }; + } + else // Not using internal buffer yet + { + iter_difference_t<_OutIter> __count(__s.size()); + return { _M_first + __count, __count }; + } } }; @@ -3882,11 +3890,8 @@ namespace __format [[__gnu__::__always_inline__]] size_t - count() - { - _Counting_sink::_M_overflow(); - return this->_M_count; - } + count() const + { return this->_M_count + this->_M_used().size(); } }; #else template diff --git a/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc b/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc index 846bda30fdf..f7df3ed36dc 100644 --- a/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc +++ b/libstdc++-v3/testsuite/std/format/functions/format_to_n.cc @@ -88,9 +88,26 @@ test_move_only() VERIFY( wlen == 11 ); } +void +test_pr110990() +{ + // PR libstdc++/110990 - format_to_n returns wrong value + + char buf[2]; + auto [ptr, len] = std::format_to_n(buf, 2, "x"); + VERIFY( len == 1 ); + VERIFY( ptr == buf + len ); + + wchar_t wbuf[3]; + auto [wptr, wlen] = std::format_to_n(wbuf, 3, L"yz"); + VERIFY( wlen == 2 ); + VERIFY( wptr == wbuf + wlen ); +} + int main() { test(); test_wchar(); test_move_only(); + test_pr110990(); }