Bug 92285 - Layout of istreambuf_iterator subobject depends on -std mode
Summary: Layout of istreambuf_iterator subobject depends on -std mode
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 10.0
Assignee: Jonathan Wakely
URL:
Keywords: ABI
Depends on:
Blocks:
 
Reported: 2019-10-30 12:16 UTC by Jonathan Wakely
Modified: 2020-11-04 12:55 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.6.4
Known to fail: 4.7.0
Last reconfirmed: 2019-10-30 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2019-10-30 12:16:31 UTC
The fix for PR 50336 (r178713) makes this program depend on the -std mode:

#include <iterator>
#include <iostream>

struct I : std::iterator<std::input_iterator_tag, char>
{ };

struct J : I, std::istreambuf_iterator<char>
{ };

int main()
{
  std::cout << sizeof(J) << '\n';
}

For C++98 it prints 24 but for other modes it prints 16. The reason is
that std::istreambuf_iterator has a different base class:

  template<typename _CharT, typename _Traits>
    class istreambuf_iterator
    : public iterator<input_iterator_tag, _CharT, typename _Traits::off_type,
                      _CharT*,
#if __cplusplus >= 201103L
    // LWG 445.
                      _CharT>
#else
                      _CharT&>
#endif

This affects layout because std::iterator is an empty class, so 
whether the two base classes can share the same address depends on
what istreambuf_iterator's base class is.

This isn't a disaster, because in practice it is probably very rare 
for a type to have two std::iterator subobjects that could have the
same address. But technically it's still an ABI incompatibility 
between C++98 and C++11/14/17 modes.

The solution is to make istreambuf_iterator always have the same base
class, but then conditionally override the reference member:

  template<typename _CharT, typename _Traits>
    class istreambuf_iterator
    : public iterator<input_iterator_tag, _CharT, typename _Traits::off_type,
                      _CharT*, ???>
    {                 
    public:
      using reference = ???;
      
Now the base class will always be the same, and so won't change layout
when __cplusplus changes.
Comment 1 Richard Biener 2019-10-30 13:16:49 UTC
Ugh.  I hope we can keep the "new" ABI for the default std though?  That means
breaking it also for -std=c++98?

Or simply document this defect :/

"Works" in 4.6.4 as far as I can see, broken starting with 4.7.
Comment 2 Jonathan Wakely 2019-10-30 13:43:50 UTC
(In reply to Richard Biener from comment #1)
> Ugh.  I hope we can keep the "new" ABI for the default std though?  That
> means
> breaking it also for -std=c++98?

Yes, see https://gcc.gnu.org/ml/libstdc++/2019-10/msg00129.html for additional discussion of the options and what breaks with each one.

As I said there, I would prefer to keep the default std unchanged, even though that breaks c++98.

> Or simply document this defect :/

Yes, and I'll be adding it to https://gcc.gnu.org/wiki/Cxx11AbiCompatibility too.

> "Works" in 4.6.4 as far as I can see, broken starting with 4.7.

Yeah.
Comment 3 Jonathan Wakely 2020-01-10 15:28:16 UTC
Author: redi
Date: Fri Jan 10 15:27:39 2020
New Revision: 280116

URL: https://gcc.gnu.org/viewcvs?rev=280116&root=gcc&view=rev
Log:
libstdc++: Make istreambuf_iterator base class consistent (PR92285)

Since LWG 445 was implemented for GCC 4.7, the std::iterator base class
of std::istreambuf_iterator changes type depending on the -std mode
used. This creates an ABI incompatibility between different -std modes.

This change ensures the base class always has the same type. This makes
layout for C++98 compatible with the current -std=gnu++14 default, but
no longer compatible with C++98 code from previous releases. In practice
this is unlikely to cause real problems, because it only affects the
layout of types with two std::iterator base classes, one of which comes
from std::istreambuf_iterator. Such types are expected to be vanishingly
rare.

	PR libstdc++/92285
	* include/bits/streambuf_iterator.h (istreambuf_iterator): Make type
	of base class independent of __cplusplus value.
	[__cplusplus < 201103L] (istreambuf_iterator::reference): Override the
	type defined in the base class
	* testsuite/24_iterators/istreambuf_iterator/92285.cc: New test.
	* testsuite/24_iterators/istreambuf_iterator/requirements/
	base_classes.cc: Adjust expected base class for C++98.

Added:
    trunk/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/92285.cc
      - copied, changed from r280109, trunk/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/requirements/base_classes.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/streambuf_iterator.h
    trunk/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/requirements/base_classes.cc
Comment 4 Jonathan Wakely 2020-01-10 15:49:20 UTC
Fixed, but documentation needed.
Comment 5 Jakub Jelinek 2020-05-07 11:56:11 UTC
GCC 10.1 has been released.
Comment 6 Richard Biener 2020-07-23 06:51:53 UTC
GCC 10.2 is released, adjusting target milestone.
Comment 7 Jonathan Wakely 2020-07-23 11:16:03 UTC
This was already fixed for 10.1, I just need to do some docs.
Comment 8 CVS Commits 2020-11-04 12:47:14 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:3ef33e756a65484a17abb95ef0d4133f80c014b1

commit r11-4720-g3ef33e756a65484a17abb95ef0d4133f80c014b1
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 4 12:45:32 2020 +0000

    libstdc++: Document istreambuf_iterator base class change [PR 92285]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/92285
            * doc/xml/manual/evolution.xml: Document change to base class.
            * doc/html/manual/api.html: Regenerate.
Comment 9 Jonathan Wakely 2020-11-04 12:55:40 UTC
Now documented too.