[libcxx-commits] [libcxx] [libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX (PR #89305)
Xing Xue via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Apr 18 13:37:32 PDT 2024
https://github.com/xingxue-ibm created https://github.com/llvm/llvm-project/pull/89305
This is a continuation of Phabricator patch [D124555](https://reviews.llvm.org/D124555).
`libcxx std::basic_ios` uses `WEOF` to indicate that the `fill` value is uninitialized. However, on AIX and zOS in 64-bit mode, `wchar_t` is 4 bytes `unsigned` and `wint_t` is also 4 bytes. 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 and z/OS in 64-bit mode.
The library is implemented correctly with respect to the C++ standard, however, this requirement imposed by C++ on C types is not also imposed by C. An implementation cannot be compliant where `wchar_t` is `unsigned` and the same size as `wint_t`. The current IBM system provided libc++ on AIX and zOS added a member `__fill_set_` in class `basic_ios` to indicate whether or not the fill character has been initialized. `__fill_set_` was used for both 32- and 64-bit on AIX and 64-bit on z/OS although this member could have been added more selectively for 64-bit only.
This patch uses the [approach](https://reviews.llvm.org/D124555#3566746) suggested by @ldionne. Class `_OptionalFill` is used for targets where a variable is needed to indicate whether the fill character has been initialized. The layout of the implementation is compatible with the current IBM system provided libc++. In the interest of ABI compatibility, we use it for 32-bit AIX as well.
Existing targets where this would break the ABI compatibility can opt out by setting `_OptOutForABICompat` to `true`.
>From f191145521f7e4da3a327654985b4a994997e7db Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Thu, 18 Apr 2024 10:59:41 -0400
Subject: [PATCH] Changes for cases where wint_t and wchar_t have the same
width. The changes are compatible with the current IBM provided libc++.
---
libcxx/include/ios | 55 +++++++++++++++++--
.../std.manip/setfill_eof.pass.cpp | 31 +++++++++++
2 files changed, 80 insertions(+), 6 deletions(-)
create mode 100644 libcxx/test/std/input.output/iostream.format/std.manip/setfill_eof.pass.cpp
diff --git a/libcxx/include/ios b/libcxx/include/ios
index 00c1d5c2d4bc58..bc31f30f14acf0 100644
--- a/libcxx/include/ios
+++ b/libcxx/include/ios
@@ -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>
@@ -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
+// definition of '__set_' in basic_ios on AIX.
+struct _LIBCPP_PACKED _OptionalFill {
+ _OptionalFill() : __set_(false) { }
+ _OptionalFill& operator=(typename _Traits::int_type __x) { __set_ = true; __fill_val_ = __x; return *this; }
+ bool __is_set() const { return __set_; }
+ typename _Traits::int_type __fill() const { return __fill_val_; }
+
+private:
+ typename _Traits::int_type __fill_val_;
+ bool __set_;
+};
+
+template <class _Traits>
+struct _LIBCPP_PACKED _SentinelValueFill {
+ _SentinelValueFill() : __fill_val_(_Traits::eof()) { }
+ _SentinelValueFill& operator=(typename _Traits::int_type __x) { __fill_val_ = __x; return *this; }
+ bool __is_set() const { return __fill_val_ != _Traits::eof(); }
+ 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:
@@ -590,7 +616,24 @@ protected:
private:
basic_ostream<char_type, traits_type>* __tie_;
- mutable int_type __fill_;
+
+#if defined(_AIX) || (defined(__MVS__) && defined(__64BIT__))
+// AIX and 64-bit MVS must use _OptionalFill for ABI backward compatibility.
+ using _FillType = _OptionalFill<_Traits>;
+#else
+#if defined(_WIN32)
+ static const bool _OptOutForABICompat = true;
+#else
+ static const bool _OptOutForABICompat = false;
+#endif
+
+ using _FillType = _If<
+ sizeof(char_type) >= sizeof(int_type) && !_OptOutForABICompat,
+ _OptionalFill<_Traits>,
+ _SentinelValueFill<_Traits>
+ >;
+#endif
+ mutable _FillType __fill_;
};
template <class _CharT, class _Traits>
@@ -605,7 +648,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(' ');
}
template <class _CharT, class _Traits>
@@ -655,16 +698,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;
}
diff --git a/libcxx/test/std/input.output/iostream.format/std.manip/setfill_eof.pass.cpp b/libcxx/test/std/input.output/iostream.format/std.manip/setfill_eof.pass.cpp
new file mode 100644
index 00000000000000..042e07cb80bbd0
--- /dev/null
+++ b/libcxx/test/std/input.output/iostream.format/std.manip/setfill_eof.pass.cpp
@@ -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
+
+#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;
+}
More information about the libcxx-commits
mailing list