[libcxx] basic_ios<wchar_t> cannot store fill character WCHAR_MAX

[xingxue]

Summary

On AIX and zOS in 64-bit mode, wchar_t is 4 bytes unsigned and wint_t is also 4 bytes. libcxx std::basic_ios uses WEOF to indicate that the fill value is uninitialized, which cannot be distinguished from WCHAR_MAX by std::char_traits<wchar_t>::eq_int_type.

The constraints of the C++ standard impose:

• Cannot map any wchar_t value to WEOF.
• All wchar_t values must map uniquely.

Compatibility with the platform C ABI imposes:

• wint_t and wchar_t have the same width on AIX in 64-bit mode.

The library is implemented correctly with respect to the C++ standard, however, an implementation cannot be compliant where wchar_t is unsigned and the same size as wint_t, consequently I've added a __fill_set_ member to indicate whether or not the fill character has been initialized.

I plan to have IBM follow up with the C++ standard to have the issue dealt with. In particular, C++ imposes requirements on C types not also imposed by C (thereby breaking compatibility with C on platforms with preexisting C implementations).

The change is enabled in both 32 and 64 bit on AIX and z/OS for compatibility with the current IBM system provided libc++ on the targets. Unfortunately this member could have been added more selectively, however, in interest of compatibility I need to propose the patch as-is.

[cebowleratibm]

libcxx/test/std/input.output/iostream.format/std.manip/setfill.pass.cpp

47 ↗

(On Diff #425599)

It appears that this test is failing on other targets as well. I think I'll need to split this test out and control the target. Because the change is ABI breaking I only intend the work for AIX and zOS. Other targets can opt-in in the config when/if they want to.

[SeanP]

libcxx/include/ios

787

Shouldn't __fill_set become true at this point?

[SeanP]

libcxx/include/ios

796

I also noticed these first few lines can be replaced with:

char_type __r = fill();

804

If you call fill() you don't need to set __fill_set here since fill() will already set it.

[cebowleratibm]

libcxx/include/ios

787

Fair point. I'll need to make __fill_set_ mutable as well.

I don't think there's a functional problem because a non default fill will cause __fill_set to be set. Here the problem is we'll redundantly __fill_ = widen(' '), nonetheless the code is more clear if I implement it as you've suggested.

[cebowleratibm]

• Only enabled on zOS in 64-bit.
• Added XFAILs for other targets from the CI that also fail the new test.
• Only check the new __fill_set_ member when necessary to mitigate compat problems if a target already shipped the version without the member.

[cebowleratibm]

Addressed early comments. Some offline discussion occurred to decide to only enable the new member on zOS in 64-bit. I've XFAILed other affected targets. They can opt into the new member in the __config at their discretion as the change affects compatibility, though I've mitigated the compatibility break as much as possible.

[Jake-Egan]

libcxx/test/std/input.output/iostream.format/std.manip/setfill_wcharmax.pass.cpp

13 ↗

(On Diff #427014)

Instead of XFAILing each target, it might make more sense to add a Lit feature for the new macro, then you can simply do:

// REQUIRES: libcpp_basic_ios_has_fill_set

[philnik]

libcxx/test/std/input.output/iostream.format/std.manip/setfill_wcharmax.pass.cpp

13 ↗

(On Diff #427014)

Or just // REQUIRES: target=zOS-target-triple. Seems quite wasteful to add a lit feature for a single test.

[cebowleratibm]

libcxx/test/std/input.output/iostream.format/std.manip/setfill_wcharmax.pass.cpp

13 ↗

(On Diff #427014)

I thought the test should be opt out rather than opt in because it should legitimately pass. A user is permitted to set a fill char to WCHARMAX and it should work. The test will also work on 32 bit targets or targets where wchar_t is signed.

I explicitly called out the failing targets so that they can be on notice that this doesn't work and they can opt into the fix if they like.

[xingxue]

LGTM; however, will need the review from the libc++ group.

[ldionne]

In its current state, this patch would cause any new target where wint_t can't represent all values of CharType to have the same bug. IIUC, the same is true on existing targets like x86_64 Darwin with a custom CharType that has the same size as int_type -- is that correct? If so, I think we should instead fix the problem by default, but provide an escape hatch for existing targets where this would break the ABI to opt out. For example:

template <class _Traits>
struct _OptionalFill {
  _OptionalFill() : __set_(false) { }
  _OptionalFill& operator=(typename _Traits::int_type __x) { __set_ = true; __fill_ = __x; return *this; }
  bool __is_set() const { return __set_; }
  typename _Traits::int_type __fill() const { return __fill_; }

private:
  typename _Traits::int_type __fill_;
  bool __set_;
};

template <class _Traits>
struct _SentinelValueFill {
  _SentinelValueFill() : __fill_(_Traits::eof()) { }
  _SentinelValueFill& operator=(typename _Traits::int_type __x) { __fill_ = __x; return *this; }
  bool __is_set() const { return __fill_ == _Traits::eof(); }
  typename _Traits::int_type __fill() const { return __fill_; }

private:
  typename _Traits::int_type __fill_;
};

template <class _CharT, class _Traits>
class basic_ios {
  // ...

private:
  basic_ostream<char_type, traits_type>* __tie_;

#if defined(_WIN32) || defined(whatever)
  static constexpr bool _OptOutForABICompat = true;
#else
  static constexpr bool _OptOutForABICompat = false;
#endif

  // Here, basically always use an appropriate representation for the fill, except on
  // platforms where we'd rather keep the bug in place than break the ABI.
  using _FillType = _If<sizeof(char_type) >= sizeof(typename traits_type::int_type) && !_OptOutForABICompat, _OptionalFill<_Traits>, _SentinelValueFill<_Traits> >;
  _FillType __fill_;
};

Thoughts?

Also, please make sure to follow https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line when uploading patches, it ensures that there's context in Phabricator, which is useful for reviewing :-)

libcxx/include/ios

707

Do we need to set the __fill_ when we use __fill_set_?

782

When we use __fill_set_, isn't this condition redundant?

libcxx/test/std/input.output/iostream.format/std.manip/setfill_wcharmax.pass.cpp

9–12 ↗

(On Diff #427014)

Do I understand correctly that all of those targets currently have a bug that one can't set a fill char to WCHAR_MAX, however fixing that requires breaking the ABI?

17 ↗

(On Diff #427014)

35 ↗

(On Diff #427014)

Let's use // UNSUPPORTED: no-wide-characters instead.

[cebowleratibm]

using _FillType = _If<sizeof(char_type) >= sizeof(typename traits_type::int_type) && !_OptOutForABICompat, _OptionalFill<_Traits>, _SentinelValueFill<_Traits> >;

The problem isn't necessarily exposed based on the type size. For example: if wchar is 4 bytes signed then WCHAR_MAX is 0x7fffffff and WEOF is 0xffffffff so the sentinel value can be represented.

I suggest:

using _FillType = _If<traits_type::eq_int_type(std::numeric_limits<char_type>::max(), traits_type::eof) && !_OptOutForABICompat, _OptionalFill<_Traits>, _SentinelValueFill<_Traits> >;

libcxx/include/ios

707

I agree, we don't have to initialize __fill_ if we have __fill_set_. I was trying to mitigate the need to check __fill_set_ elsewhere but I think we will always need to on the first call.

782

I wrote it this way to mitigate the need to check the __fill_set_ member to minimize the impact of the ABI breakage. The only case you really need to check __fill_set is if the fill charafter was is eof.

Reflecting now I think the effort was misguided because you're always going to get in here on the first call to fill. Probably better to keep it simple than get cute.

libcxx/test/std/input.output/iostream.format/std.manip/setfill_wcharmax.pass.cpp

9–12 ↗

(On Diff #427014)

Those were all the targets the failed the new test via the CI when I submitted the first version of the patch. I believe these targets will need to break the ABI if they want the fix.

[hubert.reinterpretcast]

In D124555#3566746, @ldionne wrote:

template <class _Traits>
struct _OptionalFill {
  _OptionalFill() : __set_(false) { }
  _OptionalFill& operator=(typename _Traits::int_type __x) { __set_ = true; __fill_ = __x; return *this; }
  bool __is_set() const { return __set_; }
  typename _Traits::int_type __fill() const { return __fill_; }

private:
  typename _Traits::int_type __fill_;
  bool __set_;
};

Thoughts?

From the AIX and z/OS perspective, the replacement of the two members with a member having two fields is not 100% binary compatible for all usage. At least if the basic_ios specialization is used as a non-virtual base class, any derived class members that could be allocated in the padding following the bool would now move. This could be preemptively avoided if the new class type is given a user-provided copy constructor or destructor and the new member made [[no_unique_address]]. I don't know if [[no_unique_address]] is usable is some form under C++98/03 though.

[cebowleratibm]

In D124555#3571553, @hubert.reinterpretcast wrote:

In D124555#3566746, @ldionne wrote:

template <class _Traits>
struct _OptionalFill {
  _OptionalFill() : __set_(false) { }
  _OptionalFill& operator=(typename _Traits::int_type __x) { __set_ = true; __fill_ = __x; return *this; }
  bool __is_set() const { return __set_; }
  typename _Traits::int_type __fill() const { return __fill_; }

private:
  typename _Traits::int_type __fill_;
  bool __set_;
};

Thoughts?

From the AIX and z/OS perspective, the replacement of the two members with a member having two fields is not 100% binary compatible for all usage. At least if the basic_ios specialization is used as a non-virtual base class, any derived class members that could be allocated in the padding following the bool would now move. This could be preemptively avoided if the new class type is given a user-provided copy constructor or destructor and the new member made [[no_unique_address]]. I don't know if [[no_unique_address]] is usable is some form under C++98/03 though.

For background, the (customized) libc++ that is provided on AIX used the form where the bool member is injected directly in basic_ios.

I drafted an update with the _OptionalFill / _SentinelFill suggestion and have confirmed that there are layout differences on AIX in derived classes when the first derived member is 1 or 2 byte aligned. Alternatively, even if all inheritance of basic_ios is virtual, there's still potential layout differences where a user derived class has multiple virtual bases and the second virtual base wants to start inside the padding. I did try [[no_unique_address]], however, the syntax is not accepted with -std=c++03. Unless we want to version the 03 code from the C++11 forward code, I don't think there's a way to get a layout compatible type using this approach.

One idea Hubert and I discussed offline is to use a member declaration bool __is_set[Boolean expression] and have it deliberately zero-size when the __is_set member isn't needed. We can still use _OptionalFill / _SentinelFill to control access to the member.

Another obstacle is that the proper check to enable to use of _OptionalFill looks like:

using _FillType = _If<traits_type::eq_int_type(
                          std::numeric_limits<char_type>::max(),
                          traits_type::eof()),
                      _OptionalFill, _SentinelValueFill>;

but note that eq_int_type and eof can only participate in constant expressions where they are constexpr in C++11, so I again have a challenge in delivering a solution that works in C++03. I'll need to use a wchar specialization and use something like WEOF < (wint_t)WCHAR_MIN || (win_t)WCHAR_MAX < WEOF.

Given my limited experience with LLVM libc++ some guidance along which fundamental approach we should take would be appreciated.

[cebowleratibm]

The new approach injects the new __fill_set_ only for wchar_t and only if WEOF is in the range [WCHAR_MIN, WCHAR_MAX]. I didn't use numeric_limits because the solution needs to work in C++03.

On AIX I need to force the injection of the member to retain compatibility, even when it's not actually needed.

I disabled the ABI break on _WIN32 targets, and I suspect I need to further customize the compatibility opt out. The default behaviour for new targets is to adopt the fix for wchar_t if wchar_t is impacted. I can't fix this for other char types without constexpr numeric limits and char_traits and we need to keep the layout compatible between C++03 and C++11.

[cebowleratibm]

libcxx/include/ios

701

I think I should respect _LIBCPP_HAS_NO_WIDE_CHARACTERS

[cebowleratibm]

libcxx/include/ios

699

I made a mistake, for AIX libc++ compatibility I need to inject the bool member even for non wchar_t types.

[cebowleratibm]

1. The selection of the fill set member is moved to the __config.
2. This patch retains ABI compatibility for any of the libc++ CI that reported failures on this issue.
3. Always inject the fill set member for AIX to retain compatibility for AIX.
4. This only fixes basic_ios<wchar_t>. It is not compliant to instantiate basic_ios with a char type where char_traits::eof is not a sentinel value. #wchar_t is an exception because C++ shouldn't be adding constraints beyond what C guarantees.

I haven't received significant feedback since I overhauled the patch in response to @ldionne 's initial feedback. The new design implements the fix as needed for new targets, which I think was the primary reason for the revision.

[hubert.reinterpretcast]

libcxx/include/__config

324

It is my understanding that z/OS is also in the same situation.

327

It seems to me that _M_ARM and _M_ARM64 is associated with Windows and not Linux.

libcxx/include/ios

705

If wchar_t is 16-bit unsigned, wint_t is 32-bit signed and WEOF is -1, then WEOF is less than (wint_t)WCHAR_MAX and value becomes true even though the new member is unneeded.

[hubert.reinterpretcast]

libcxx/include/ios

705

This should detect when wint_t is not wider than wchar_t:

WCHAR_MAX >= WINT_MAX || 1 + (wint_t)WCHAR_MAX == (wint_t)WCHAR_MIN

but using sizeof might be reasonable.

[cebowleratibm]

Writing revision to use SentinelFill/OptionalFill classes.

libcxx/include/__config

324

zOS hasn't provided a 32-bit libc++ solution yet. They're 64-bit injected a bool a __fill_set_ member. The compile time selection of the fill_set implementation will do the right thing for zOS 32/64 bit without need for explicit overrides.

libcxx/include/ios

705

I agree. Turns out signed wchar also has a problem with -1 and @ldionne 's suggestion was the right way to go. I'm going to use sizeof and post a revision using OptionalFill/SentinelFill suggestion.

[cebowleratibm]

libcxx/include/ios

705

I forgot the reason we chose not to use the OptionalFill/SentinelFill suggestion is that it may affect the layout of derived classes, compared to appending the __is_set_ member directly in basic_ios

I'm now planning a tweaked version of the current revision that uses sizeof but continues to use the zero-length/size 1 array member.

[cebowleratibm]

Any basic_ios where sizeof(char_type) >= sizeof(int_type) has at least some char value that overlaps with the eof character. I've modified the proposed test to demonstrate signed char types are affected at -1.

I've proposed a __has_fill_ member directly in basic_ios to retain layout compatibility of derived classes on what IBM has done (downstream) with prior libc++ on AIX.

The patch as proposed also works for zOS. There's no need for explicit change in the config as the sizeof check get the desired layout in both 32 and 64 bit.

[cebowleratibm]

Quick testcase fix.

[daltenty]

libcxx/include/__config

324

What about adding an ABI guard around this whole block? Something like:

#if _LIBCPP_ABI_BASIC_IOS_HAS_FILL_SET_IF_NEEDED

And defining that for the unstable ABI (_LIBCPP_ABI_VERSION >= 2) in the relevant block here? That would let folks not requiring the stable ABI on the below platforms pickup the fix automatically if needed.

[daltenty]

libcxx/include/__config

324

edit: or rather the reverse check:

#ifndef _LIBCPP_ABI_BASIC_IOS_HAS_FILL_SET_IF_NEEDED

[cebowleratibm]

Plan to revise the config to pick up the "fill set member as needed" mode in the config for all targets when _LIBCPP_ABI_VERSION >= 2.

libcxx/include/__config

324

There are 3 states:

1. The fill set member always exists (AIX compat)
2. The fill set member never exists (non AIX target compat)
3. Add the fill set member only if needed. (the desirable path)

I chose a single macro _LIBCPP_BASIC_IOS_HAS_FILL_SET to represent these states. The third state is achieved by leaving the macro undefined, which is to say: there's no force override so just do the right thing.

If I introduce another macro _LIBCPP_ABI_BASIC_IOS_HAS_FILL_SET_IF_NEEDED then the config is conflicted if both macros are defined. I thought it better to avoid the potential for things to go wrong.

You have an excellent point that we can enable the "as needed" mode on all targets when _LIBCPP_ABI_VERSION >= 2. If will revise the patch for this.

[cebowleratibm]

• Only add fill set as needed for _LIBCPP_ABI > 1 for all targets
• zOS 64-bit always needs fill set for compatibility
• tweaked the test to use the char value for eof as it exposes the problem for both signed and unsigned char types

[cebowleratibm]

Fix formatting and a typo in the __config

[xingxue]

libcxx/include/__config

327

Delete the extra ).

libcxx/include/ios

815–816

821

Formatting.

[cebowleratibm]

Thanks for catching the formatting problems Xing. I'll need to look at my workflow to see why I didn't catch those.

[cebowleratibm]

Formatting and typo.

[cebowleratibm]

New test fails on macos targets (as it should) but I needed to correct a typo on the XFAIL line.

[cebowleratibm]

The new test passes/fails based on wchar_t/wint_t types, which is tedious to manage across targets with XFAIL so I've decided to enable it only for AIX/zOS targets where it is expected to pass.

[xingxue]

I am taking over this revision because @cebowleratibm is working on a different project.

[xingxue]

Hi @ldionne, It would be appreciated if you could take a look at this patch and provide guidance on how to proceed.

[hubert.reinterpretcast]

In D124555#3571553, @hubert.reinterpretcast wrote:

From the AIX and z/OS perspective, the replacement of the two members with a member having two fields is not 100% binary compatible for all usage. At least if the basic_ios specialization is used as a non-virtual base class, any derived class members that could be allocated in the padding following the bool would now move. This could be preemptively avoided if the new class type is given a user-provided copy constructor or destructor and the new member made [[no_unique_address]]. I don't know if [[no_unique_address]] is usable is some form under C++98/03 though.

We may be able to use some form of packed too.

[xingxue]

In D124555#4651438, @hubert.reinterpretcast wrote:

In D124555#3571553, @hubert.reinterpretcast wrote:

From the AIX and z/OS perspective, the replacement of the two members with a member having two fields is not 100% binary compatible for all usage. At least if the basic_ios specialization is used as a non-virtual base class, any derived class members that could be allocated in the padding following the bool would now move. This could be preemptively avoided if the new class type is given a user-provided copy constructor or destructor and the new member made [[no_unique_address]]. I don't know if [[no_unique_address]] is usable is some form under C++98/03 though.

We may be able to use some form of packed too.

Thanks for the suggestion, I will look into it.

[xingxue]

Updated the patch using the approach suggested by @ldionne in comment https://reviews.llvm.org/D124555#3566746 and by @hubert.reinterpretcast in comment https://reviews.llvm.org/D124555#4651438. The layout of this implementation is compatible with what IBM has done (downstream) with prior libc++ on AIX (32-bit and 64-bit) and zOS (64-bit only).

[xingxue]

Changes:

• fix the problem in C++03 mode
• use the condition as suggested for determining the fill type

[xingxue]

Fixed a typo.

[xingxue]

Use _LIBCPP_PACKED instead of __attribute__((packed)).

[xingxue]

Fixed the header file order.

[xingxue]

Removed _LIBCPP_ABI_VERSION check.

libcxx/include/__config

 #    endif
 #  endif // __sun__
 
 #  if defined(_AIX) && !defined(__64BIT__)
 // The size of wchar is 2 byte on 32-bit mode on AIX.
 #    define _LIBCPP_SHORT_WCHAR 1
 #  endif
 
+#  if _LIBCPP_ABI_VERSION < 2
+#    if defined(_AIX) || (defined(__MVS__) && defined(__64BIT__))
+#      define _LIBCPP_BASIC_IOS_HAS_FILL_SET 1
+#    elif defined(_WIN32) || defined(__sun__) || || defined(__APPLE__) || defined(__linux__) ||                        \
+          defined(__FreeBSD__) || defined(__NetBSD__)
+#      define _LIBCPP_BASIC_IOS_HAS_FILL_SET 0
+#    endif
+#  endif
+
 // Libc++ supports various implementations of std::random_device.
 //
 // _LIBCPP_USING_DEV_RANDOM
 //      Read entropy from the given file, by default `/dev/urandom`.
 //      If a token is provided, it is assumed to be the path to a file
 //      to read entropy from. This is the default behavior if nothing
 //      else is specified. This implementation requires storing state
 //      inside `std::random_device`.

libcxx/include/ios

protected:
     _LIBCPP_INLINE_VISIBILITY
     void move(basic_ios&& __rhs) {move(__rhs);}
     _LIBCPP_INLINE_VISIBILITY
     void swap(basic_ios& __rhs) _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY
     void set_rdbuf(basic_streambuf<char_type, traits_type>* __sb);
 private:
     basic_ostream<char_type, traits_type>* __tie_;
+  
+#if defined(_LIBCPP_BASIC_IOS_HAS_FILL_SET)
+    static const bool __has_fill_set = _LIBCPP_BASIC_IOS_HAS_FILL_SET;
+#else
+    static const bool __has_fill_set = sizeof(char_type) >= sizeof(int_type);
+#endif
+
     mutable int_type __fill_;
+    mutable bool __fill_set_[__has_fill_set];
 };
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_ios<_CharT, _Traits>::basic_ios(basic_streambuf<char_type,traits_type>* __sb)
 {
     init(__sb);
 }
 
 template <class _CharT, class _Traits>
 basic_ios<_CharT, _Traits>::~basic_ios()
 {
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_ios<_CharT, _Traits>::init(basic_streambuf<char_type, traits_type>* __sb)
 {
     ios_base::init(__sb);
     __tie_ = nullptr;
+    if (__has_fill_set) 
+    {
+        __fill_set_[0] = false;
+    }
+    else
+    {
         __fill_ = traits_type::eof();
     }
+}
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_ostream<_CharT, _Traits>*
 basic_ios<_CharT, _Traits>::tie() const
 {
     return __tie_;
 }
basic_ios<_CharT, _Traits>::imbue(const locale& __loc)
     return __r;
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 char
 basic_ios<_CharT, _Traits>::narrow(char_type __c, char __dfault) const
 {
     return use_facet<ctype<char_type> >(getloc()).narrow(__c, __dfault);
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 _CharT
 basic_ios<_CharT, _Traits>::widen(char __c) const
 {
     return use_facet<ctype<char_type> >(getloc()).widen(__c);
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 _CharT
 basic_ios<_CharT, _Traits>::fill() const
 {
-    if (traits_type::eq_int_type(traits_type::eof(), __fill_))
+    if (__has_fill_set)
+    {
+        if (!__fill_set_[0])
+        {
             __fill_ = widen(' ');
+            __fill_set_[0] = true;
+        }
+    }
+    else if (traits_type::eq_int_type(traits_type::eof(), __fill_))
+    {
+        __fill_ = widen(' ');
+    }
     return __fill_;
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 _CharT
 basic_ios<_CharT, _Traits>::fill(char_type __ch)
 {
-    if (traits_type::eq_int_type(traits_type::eof(), __fill_))
-        __fill_ = widen(' ');
-    char_type __r = __fill_;
+    char_type __r = fill();
     __fill_ = __ch;
     return __r;
 }
 
 template <class _CharT, class _Traits>
 basic_ios<_CharT, _Traits>&
 basic_ios<_CharT, _Traits>::copyfmt(const basic_ios& __rhs)
 {
     if (this != &__rhs)
     {
         __call_callbacks(erase_event);
         ios_base::copyfmt(__rhs);
         __tie_ = __rhs.__tie_;
         __fill_ = __rhs.__fill_;
+        if (__has_fill_set)
+        {
+            __fill_set_[0] = __rhs.__fill_set_[0];
+        }
         __call_callbacks(copyfmt_event);
         exceptions(__rhs.exceptions());
     }
     return *this;
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_ios<_CharT, _Traits>::move(basic_ios& __rhs)
 {
     ios_base::move(__rhs);
     __tie_ = __rhs.__tie_;
     __rhs.__tie_ = nullptr;
     __fill_ = __rhs.__fill_;
+    if (__has_fill_set)
+    {
+        __fill_set_[0] = __rhs.__fill_set_[0];
+    }
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_ios<_CharT, _Traits>::swap(basic_ios& __rhs) _NOEXCEPT
 {
     ios_base::swap(__rhs);
     _VSTD::swap(__tie_, __rhs.__tie_);
     _VSTD::swap(__fill_, __rhs.__fill_);
+    if (__has_fill_set)
+    {
+        _VSTD::swap(__fill_set_[0], __rhs.__fill_set_[0]);
+    }
 }
 
 template <class _CharT, class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 basic_ios<_CharT, _Traits>::set_rdbuf(basic_streambuf<char_type, traits_type>* __sb)
 {
     ios_base::set_rdbuf(__sb);

libcxx/test/std/input.output/iostream.format/std.manip/setfill_eof.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: no-wide-characters
+
+// XFAIL: stdlib=libc++ && target=aarch64-linux-gnu
+// XFAIL: stdlib=libc++ && target=armv7l-linux-gnueabihf
+// XFAIL: stdlib=libc++ && target=armv8l-linux-gnueabihf
+// XFAIL: stdlib=libc++ && target=x86_64-pc-windows-msvc
+// XFAIL: stdlib=libc++ && target=x86_64-w64-windows-gnu
+
+// <iomanip>
+
+// template<charT> unspecified setfill(charT c);
+
+#include <iomanip>
+#include <ostream>
+#include <cassert>
+#include <string>
+
+#include "test_macros.h"
+
+template <class CharT>
+struct testbuf : public std::basic_streambuf<CharT> {
+  testbuf() {}
+};
+
+int main(int, char**) {
+  testbuf<wchar_t> sb;
+  std::wostream os(&sb);
+  os << std::setfill((wchar_t)std::char_traits<wchar_t>::eof());
+  assert(os.fill() == (wchar_t)std::char_traits<wchar_t>::eof());
+
+  return 0;
+}