public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/101542] New: __gnu_cxx::sequence_buffer const copy constructor is badly broken
@ 2021-07-21  2:51 brooks at gcc dot gnu.org
  2021-07-21 10:00 ` [Bug libstdc++/101542] " redi at gcc dot gnu.org
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: brooks at gcc dot gnu.org @ 2021-07-21  2:51 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 101542
           Summary: __gnu_cxx::sequence_buffer const copy constructor is
                    badly broken
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: brooks at gcc dot gnu.org
  Target Milestone: ---

This problem showed up when we were running the GCC 4.9.4 testsuite with the
latest Clang trunk as the compiler, and found that the ext/rope/4.cc test
started failing -- producing "wibblewibble" when it should produce "wibble".

However, the problem is broader than that, and affects much more current GCC
versions as well -- as can be seen at https://godbolt.org/z/74W78rdWP. 
Compiling the following program with Clang trunk produces "hellohello" as
output:
----
#include <ext/rope>
#include <iostream>

template<typename T> T make_copy(const T &x) { return x; }

int main() {
    std::string s;
    __gnu_cxx::sequence_buffer<std::string> a(s);
    {
        __gnu_cxx::sequence_buffer<std::string> b = a;
        b.push_back('h');
        b.push_back('e');
        b.push_back('l');
        b.push_back('l');
        b.push_back('o');

        // Making a copy causes sequence_buffer to break.
        make_copy(b);
    }
    std::cout << s;
}
----

This started failing with a recent Clang change
(https://github.com/llvm/llvm-project/commit/7d2d5a3a6d7aaa40468c30250bf6b0938ef02c08),
described as "Apply P1825 as Defect Report from C++11 up to C++20".  See
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1825r0.html for the
defect report details.  I would guess that GCC will be applying a similar
change.

My colleagues Jorge Gorbe and Richard Smith did a lot of digging on this, and
Richard explains the problem as follows:

----
The problem with sequence_buffer is that the sequence_buffer(const
sequence_buffer&) copy constructor copies the pending characters in the buffer
to the new copy, and when both are destroyed, they get flushed to the
underlying sequence twice.

But sequence_buffer is more broken than that: it also has another constructor,
sequence_buffer(sequence_buffer&) that has different semantics: it flushes the
source first. So if you only ever use non-const sequence_buffer objects, never
modify a copied-from object, and never do anything that would call the
sequence_buffer(const sequence_buffer&) constructor, it will appear to work.
And that's what this test was relying on.

Part of the consequence of the Clang change is that in code like this:

sequence_buffer f(sequence_buffer x) {
  return x;
}

... the return statement unconditionally behaves like return
(sequence_buffer&&)x;. And that means that we now choose the
sequence_buffer(const sequence_buffer&) constructor, not the
sequence_buffer(sequence_buffer&) constructor, because a non-const lvalue
reference can't bind to an rvalue.  Ultimately, I think that happens at line
342 of stl_algobase.h ("return __result"), here:
https://gcc.gnu.org/git?p=gcc.git;a=blame;f=libstdc%2B%2B-v3/include/bits/stl_algobase.h;hb=9b61c47826156fe17fd5f4306470ade01e2fc4dc#l238.

So: this libstdc++ code is broken, and the test really should never have
passed.
----

In our case, we're simply xfailing the test since we don't have any code that
uses __gnu_cxx::sequence_buffer or __gnu_cxx::rope, but I'm passing this bug
report along in hopes that it's useful to the libstdc++ maintainers.  :)

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-21 16:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21  2:51 [Bug libstdc++/101542] New: __gnu_cxx::sequence_buffer const copy constructor is badly broken brooks at gcc dot gnu.org
2021-07-21 10:00 ` [Bug libstdc++/101542] " redi at gcc dot gnu.org
2021-07-21 12:46 ` redi at gcc dot gnu.org
2021-07-21 14:24 ` redi at gcc dot gnu.org
2021-07-21 16:20 ` cvs-commit at gcc dot gnu.org
2021-07-21 16:28 ` redi at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).