Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX #89305

Merged
merged 24 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7bd12fb
Use lower max threads to reduce the testing time on AIX.
xingxue-ibm Apr 10, 2024
ecbcaa8
Changes for cases where wint_t and wchar_t have the same width. The c…
xingxue-ibm Apr 18, 2024
a54d54a
Addressed comments.
xingxue-ibm Apr 19, 2024
8fa650a
Addressed comments.
xingxue-ibm Jun 3, 2024
c033ea9
Add _LIBCPP_HIDE_FROM_ABI.
xingxue-ibm Jun 4, 2024
5288cb3
Removed changes in unrelated test case.
xingxue-ibm Jun 4, 2024
00f4cd2
Add description of ABI change in ReleaseNote.
xingxue-ibm Jun 4, 2024
e71aafc
Address comment.
xingxue-ibm Jun 5, 2024
b2ba84c
Use helper class _FillHelper by default in ABI version 2.
xingxue-ibm Jun 7, 2024
33e58ed
Remove unnecessary include of __type_traits/conditional.h.
xingxue-ibm Jun 7, 2024
933f888
Addressed comments.
xingxue-ibm Jun 11, 2024
7ef7b82
Rename init() to __init().
xingxue-ibm Jun 11, 2024
9c493f3
Addressed comments.
xingxue-ibm Jun 14, 2024
a9280c0
Update libcxx/include/__configuration/abi.h
xingxue-ibm Jul 4, 2024
68edd80
Update libcxx/include/__configuration/abi.h
xingxue-ibm Jul 4, 2024
c0f97c3
Update libcxx/include/__configuration/abi.h
xingxue-ibm Jul 4, 2024
49f202f
Update libcxx/include/ios
xingxue-ibm Jul 4, 2024
29a4635
Addressed comments.
xingxue-ibm Jul 4, 2024
88fc9eb
Remove the unused macro definition for AIX/zOS in abi.h.
xingxue-ibm Jul 4, 2024
446f895
Merge branch 'main' into ios-fill
xingxue-ibm Jul 4, 2024
9ea946e
setfill() WCHAR_MAX instead of WEOF for the test.
xingxue-ibm Jul 12, 2024
bf8e923
Fixed format.
xingxue-ibm Jul 12, 2024
ab1cbf7
XFAIL targets where WEOF is the same as WCHAR_MAX in libcpp ABI versi…
xingxue-ibm Jul 15, 2024
5338b67
Update libcxx/test/std/input.output/iostream.format/std.manip/setfill…
xingxue-ibm Jul 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions libcxx/docs/ReleaseNotes/19.rst
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ ABI Affecting Changes
``random_device`` could throw a ``system_error`` with this value. It now
throws ``ENOMSG``.

- libcxx ``std::basic_ios`` uses ``WEOF`` to indicate that the fill value is uninitialized. However, on platforms
xingxue-ibm marked this conversation as resolved.
Show resolved Hide resolved
where the size of ``char_type`` is equal to or greater than the size of ``int_type``,
``std::char_traits<char_type>::eq_int_type()`` cannot distinguish between ``WEOF`` and ``WCHAR_MAX``. Helper
class ``_OptionalFill`` is used for targets where a variable is needed to indicate whether the fill value
has been initialized. This is an ABI break on platforms where the size of ``char_type`` is equal to or greater
than the size of ``int_type``. Existing targets affected by this change can choose to keep the existing ABI
by undefining macro ``_LIBCXX_IOS_MAY_USE_OPTIONAL_FILL``.

Build System Changes
--------------------
Expand Down
13 changes: 13 additions & 0 deletions libcxx/include/__configuration/abi.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@
# define _LIBCPP_ABI_USE_WRAP_ITER_IN_STD_STRING_VIEW
// Dont' add an inline namespace for `std::filesystem`
# define _LIBCPP_ABI_NO_FILESYSTEM_INLINE_NAMESPACE
// libcxx std::basic_ios uses WEOF to indicate that the fill value is
xingxue-ibm marked this conversation as resolved.
Show resolved Hide resolved
// uninitialized. However, on platforms where the size of char_type is
// equal to or greater than the size of int_type,
// std::char_traits<char_type>::eq_int_type() cannot distinguish between WEOF
// and WCHAR_MAX. Helper class _OptionalFill is used for targets where a
// variable is needed to indicate whether the fill value has been initialized.
// Existing targets where this would break ABI compatibility can choose to keep
// the existing ABI by undefining macro _LIBCXX_IOS_MAY_USE_OPTIONAL_FILL.
# define _LIBCXX_IOS_MAY_USE_OPTIONAL_FILL
#elif _LIBCPP_ABI_VERSION == 1
# if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
// Enable compiling copies of now inline methods into the dylib to support
Expand All @@ -108,6 +117,10 @@
# if defined(__FreeBSD__) && __FreeBSD__ < 14
# define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR
# endif
// AIX and 64-bit MVS must use _OptionalFill for ABI backward compatibility.
# if defined(_AIX) || (defined(__MVS__) && defined(__64BIT__))
# define _LIBCXX_IOS_FORCE_OPTIONAL_FILL
# endif
#endif

// We had some bugs where we use [[no_unique_address]] together with construct_at,
Expand Down
50 changes: 44 additions & 6 deletions libcxx/include/ios
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ storage-class-specifier const error_category& iostream_category() noexcept;
#include <__system_error/error_code.h>
#include <__system_error/error_condition.h>
#include <__system_error/system_error.h>
#include <__type_traits/conditional.h>
#include <__utility/swap.h>
#include <__verbose_abort>
#include <version>
Expand Down Expand Up @@ -521,6 +522,31 @@ inline _LIBCPP_HIDE_FROM_ABI void ios_base::exceptions(iostate __iostate) {
clear(__rdstate_);
}

template <class _Traits>
// Attribute 'packed' is used to keep the layout compatible with the previous
xingxue-ibm marked this conversation as resolved.
Show resolved Hide resolved
// definition of the '__fill_' and '_set_' pair in basic_ios on AIX & z/OS.
struct _LIBCPP_PACKED _OptionalFill {
_LIBCPP_HIDE_FROM_ABI _OptionalFill() : __set_(false) { }
_LIBCPP_HIDE_FROM_ABI _OptionalFill& operator=(typename _Traits::int_type __x) { __set_ = true; __fill_val_ = __x; return *this; }
_LIBCPP_HIDE_FROM_ABI bool __is_set() const { return __set_; }
_LIBCPP_HIDE_FROM_ABI typename _Traits::int_type __fill() const { return __fill_val_; }
xingxue-ibm marked this conversation as resolved.
Show resolved Hide resolved

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

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

private:
typename _Traits::int_type __fill_val_;
};

template <class _CharT, class _Traits>
class _LIBCPP_TEMPLATE_VIS basic_ios : public ios_base {
public:
Expand Down Expand Up @@ -590,7 +616,19 @@ protected:

private:
basic_ostream<char_type, traits_type>* __tie_;
mutable int_type __fill_;

#if defined(_LIBCXX_IOS_FORCE_OPTIONAL_FILL)
using _FillType = _OptionalFill<_Traits>;
#elif defined(_LIBCXX_IOS_MAY_USE_OPTIONAL_FILL)
using _FillType = _If<
sizeof(char_type) >= sizeof(int_type),
_OptionalFill<_Traits>,
_SentinelValueFill<_Traits>
>;
#else
using _FillType = _SentinelValueFill<_Traits>;
#endif
mutable _FillType __fill_;
};

template <class _CharT, class _Traits>
Expand All @@ -605,7 +643,7 @@ template <class _CharT, class _Traits>
inline _LIBCPP_HIDE_FROM_ABI void basic_ios<_CharT, _Traits>::init(basic_streambuf<char_type, traits_type>* __sb) {
ios_base::init(__sb);
__tie_ = nullptr;
__fill_ = traits_type::eof();
__fill_ = widen(' ');
xingxue-ibm marked this conversation as resolved.
Show resolved Hide resolved
}

template <class _CharT, class _Traits>
Expand Down Expand Up @@ -655,16 +693,16 @@ inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::widen(char __c)

template <class _CharT, class _Traits>
inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::fill() const {
if (traits_type::eq_int_type(traits_type::eof(), __fill_))
if (!__fill_.__is_set())
__fill_ = widen(' ');
return __fill_;
return __fill_.__fill();
}

template <class _CharT, class _Traits>
inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::fill(char_type __ch) {
if (traits_type::eq_int_type(traits_type::eof(), __fill_))
if (!__fill_.__is_set())
__fill_ = widen(' ');
char_type __r = __fill_;
char_type __r = __fill_.__fill();
__fill_ = __ch;
return __r;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//===----------------------------------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//

// Test that weof as a wchar_t value can be set as the fill character.

// UNSUPPORTED: no-wide-characters
// REQUIRES: target=powerpc{{(64)?}}-ibm-aix || target=s390x-ibm-zos
xingxue-ibm marked this conversation as resolved.
Show resolved Hide resolved

#include <iomanip>
#include <ostream>
#include <cassert>
#include <string>

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;
}
Loading