From 71f05244aab17ce9f7e1cb3f0d001d4152b0fe21 Mon Sep 17 00:00:00 2001 From: Xing Xue Date: Wed, 17 Jul 2024 14:47:13 -0400 Subject: [PATCH] [libc++] basic_ios cannot store fill character WCHAR_MAX (#89305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: `libcxx std::basic_ios` uses `WEOF` to indicate the `fill` value is uninitialized. On some platforms (e.g AIX and zOS in 64-bit mode) `wchar_t` is 4 bytes `unsigned` and `wint_t` is also 4 bytes which means `WEOF` cannot be distinguished from `WCHAR_MAX` by `std::char_traits::eq_int_type()`, meaning this valid character value cannot be stored on affected platforms (as the implementation triggers reinitialization to `widen(’ ’)`). This patch introduces a new helper class `_FillHelper` uses a boolean variable to indicate whether the fill character has been initialized, which is used by default in libcxx ABI version 2. The patch does not affect ABI version 1 except for targets AIX in 32- and 64-bit and z/OS in 64-bit (so that the layout of the implementation is compatible with the current IBM system provided libc++) This is a continuation of Phabricator patch [D124555](https://reviews.llvm.org/D124555). This patch uses a modified version of the [approach](https://reviews.llvm.org/D124555#3566746) suggested by @ldionne . --------- Co-authored-by: Louis Dionne Co-authored-by: David Tenty Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250925 --- libcxx/cmake/caches/AIX.cmake | 1 + libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake | 1 + libcxx/cmake/caches/s390x-ibm-zos.cmake | 1 + libcxx/include/__configuration/abi.h | 7 +++ libcxx/include/ios | 50 ++++++++++++++++--- .../std.manip/setfill_wchar_max.pass.cpp | 38 ++++++++++++++ 6 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp diff --git a/libcxx/cmake/caches/AIX.cmake b/libcxx/cmake/caches/AIX.cmake index c01aa5b14df065..4ec78f9bbd5923 100644 --- a/libcxx/cmake/caches/AIX.cmake +++ b/libcxx/cmake/caches/AIX.cmake @@ -15,3 +15,4 @@ set(LIBCXXABI_ENABLE_STATIC OFF CACHE BOOL "") set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "") set(LIBUNWIND_ENABLE_SHARED ON CACHE BOOL "") set(LIBUNWIND_ENABLE_STATIC OFF CACHE BOOL "") +set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE" CACHE STRING "") diff --git a/libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake b/libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake index 95b7cbe776e05f..68b1cc8eff9b06 100644 --- a/libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake +++ b/libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake @@ -20,3 +20,4 @@ set(LIBCXX_CXX_ABI system-libcxxabi CACHE STRING "") set(LIBCXX_ADDITIONAL_COMPILE_FLAGS "-fzos-le-char-mode=ascii" CACHE STRING "") set(LIBCXX_ADDITIONAL_LIBRARIES "-L../s390x-ibm-zos/lib -Wl,../s390x-ibm-zos/lib/libunwind.x" CACHE STRING "") +set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE" CACHE STRING "") diff --git a/libcxx/cmake/caches/s390x-ibm-zos.cmake b/libcxx/cmake/caches/s390x-ibm-zos.cmake index 3eaed3e67fc162..e51d5ff31ebfe4 100644 --- a/libcxx/cmake/caches/s390x-ibm-zos.cmake +++ b/libcxx/cmake/caches/s390x-ibm-zos.cmake @@ -15,3 +15,4 @@ set(LIBCXX_DLL_NAME CRTEQCXE CACHE STRING "") set(LIBCXXABI_DLL_NAME CRTEQCXA CACHE STRING "") set(LIBCXXABI_ADDITIONAL_LIBRARIES "-Wl,lib/libunwind.x" CACHE STRING "") +set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE" CACHE STRING "") diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h index 513da6e3b81b65..cbde7887becf1e 100644 --- a/libcxx/include/__configuration/abi.h +++ b/libcxx/include/__configuration/abi.h @@ -91,6 +91,13 @@ # 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 +// std::basic_ios uses WEOF to indicate that the fill value is +// uninitialized. However, on platforms where the size of char_type is +// equal to or greater than the size of int_type and char_type is unsigned, +// std::char_traits::eq_int_type() cannot distinguish between WEOF +// and WCHAR_MAX. This ABI setting determines whether we should instead track whether the fill +// value has been initialized using a separate boolean, which changes the ABI. +# define _LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE #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 diff --git a/libcxx/include/ios b/libcxx/include/ios index 0a813c07721fee..d8a3643c7ad50d 100644 --- a/libcxx/include/ios +++ b/libcxx/include/ios @@ -519,6 +519,38 @@ inline _LIBCPP_HIDE_FROM_ABI void ios_base::exceptions(iostate __iostate) { clear(__rdstate_); } +template +// Attribute 'packed' is used to keep the layout compatible with the previous +// definition of the '__fill_' and '_set_' pair in basic_ios on AIX & z/OS. +struct _LIBCPP_PACKED _FillHelper { + _LIBCPP_HIDE_FROM_ABI void __init() { __set_ = false; } + _LIBCPP_HIDE_FROM_ABI _FillHelper& 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 __get() const { return __fill_val_; } + +private: + typename _Traits::int_type __fill_val_; + bool __set_; +}; + +template +struct _LIBCPP_PACKED _SentinelValueFill { + _LIBCPP_HIDE_FROM_ABI void __init() { __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 __get() const { return __fill_val_; } + +private: + typename _Traits::int_type __fill_val_; +}; + template class _LIBCPP_TEMPLATE_VIS basic_ios : public ios_base { public: @@ -588,7 +620,13 @@ protected: private: basic_ostream* __tie_; - mutable int_type __fill_; + +#if defined(_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE) + using _FillType = _FillHelper; +#else + using _FillType = _SentinelValueFill; +#endif + mutable _FillType __fill_; }; template @@ -603,7 +641,7 @@ template inline _LIBCPP_HIDE_FROM_ABI void basic_ios<_CharT, _Traits>::init(basic_streambuf* __sb) { ios_base::init(__sb); __tie_ = nullptr; - __fill_ = traits_type::eof(); + __fill_.__init(); } template @@ -653,16 +691,16 @@ inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::widen(char __c) template 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_.__get(); } template 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_.__get(); __fill_ = __ch; return __r; } diff --git a/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp b/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp new file mode 100644 index 00000000000000..f22850877dd626 --- /dev/null +++ b/libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp @@ -0,0 +1,38 @@ +//===----------------------------------------------------------------------===// +// +// 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 WCHAR_MAX as a wchar_t value can be set as the fill character. + +// UNSUPPORTED: no-wide-characters + +// Expect the test case to fail on targets where WEOF is the same as +// WCHAR_MAX with the libcpp ABI version 1 implementation. The libcpp ABI +// version 2 implementation fixes the problem. + +// XFAIL: target={{.*}}-windows{{.*}} && libcpp-abi-version=1 +// XFAIL: target=armv{{7|8}}l-linux-gnueabihf && libcpp-abi-version=1 +// XFAIL: target=aarch64-linux-gnu && libcpp-abi-version=1 + +#include +#include +#include +#include + +template +struct testbuf : public std::basic_streambuf { + testbuf() {} +}; + +int main(int, char**) { + testbuf sb; + std::wostream os(&sb); + os << std::setfill((wchar_t)WCHAR_MAX); + assert(os.fill() == (wchar_t)WCHAR_MAX); + + return 0; +}