[Bug libstdc++/110990] New: `format_to_n` returns wrong value

[Bug libstdc++/110990] New: `format_to_n` returns wrong value

[ensadc at mailnesia dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

            Bug ID: 110990
           Summary: `format_to_n` returns wrong value
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ensadc at mailnesia dot com
  Target Milestone: ---

https://godbolt.org/z/4fv71hhGe
====
#include <format>
#include <iostream>

int main() {
    char buf[10] = "@@@@@@@@@";
    auto result = std::format_to_n(buf, std::size(buf), "a");

    std::cout << result.out - buf << '\n';
}

====
On libstdc++, `result.out - buf` evaluates to 10. I expect it to be 1. Both
libc++ and MSVC STL produce 1.

[Bug libstdc++/110990] `format_to_n` returns wrong value

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
     Ever confirmed|0                           |1
   Target Milestone|---                         |13.3
   Last reconfirmed|                            |2023-08-11
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Hmm, apparently I only tested cases where format_to_n truncates the output, not
where it fits!

[Bug libstdc++/110990] `format_to_n` returns wrong value

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The problem is that the _Iter_sink for contiguous iterators calls _M_overflow()
to update its count of characters written, but _M_overflow() assumes it's only
called when the buffer is full, and so switches to the internal buffer. The
_M_finish()&& function then thinks that if the internal buffer is in use, we
wrote n characters already and so returns out+n which is the end of the buffer.

I'm testing a fix.

[Bug libstdc++/110990] `format_to_n` returns wrong value

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:003016a40844701c48851020df672b70f3446bdb

commit r14-3170-g003016a40844701c48851020df672b70f3446bdb
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Aug 11 23:02:44 2023 +0100

    libstdc++: Fix std::format_to_n return value [PR110990]

    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<C, ContigIter>::_M_overflow): Only switch to the
            internal buffer after running out of space.
            (_Iter_sink<C, ContigIter>::_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.

[Bug libstdc++/110990] `format_to_n` returns wrong value

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk so far.

[Bug libstdc++/110990] `format_to_n` returns wrong value

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:8f82863df33c63693a355d9244c315ef5cb2158e

commit r13-7721-g8f82863df33c63693a355d9244c315ef5cb2158e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Aug 11 23:02:44 2023 +0100

    libstdc++: Fix std::format_to_n return value [PR110990]

    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<C, ContigIter>::_M_overflow): Only switch to the
            internal buffer after running out of space.
            (_Iter_sink<C, ContigIter>::_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.

    (cherry picked from commit 003016a40844701c48851020df672b70f3446bdb)

[Bug libstdc++/110990] `format_to_n` returns wrong value

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110990

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 13.3, thanks for the report.