[libcxx-commits] [libcxx] 194f98c - [libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX (#89305)
via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jul 17 11:47:17 PDT 2024
Author: Xing Xue
Date: 2024-07-17T14:47:13-04:00
New Revision: 194f98c2210bf40d0490613fddbf83e04c18ad9b
URL: https://github.com/llvm/llvm-project/commit/194f98c2210bf40d0490613fddbf83e04c18ad9b
DIFF: https://github.com/llvm/llvm-project/commit/194f98c2210bf40d0490613fddbf83e04c18ad9b.diff
LOG: [libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX (#89305)
`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<wchar_t>::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 <ldionne.2 at gmail.com>
Co-authored-by: David Tenty <daltenty.dev at gmail.com>
Added:
libcxx/test/std/input.output/iostream.format/std.manip/setfill_wchar_max.pass.cpp
Modified:
libcxx/cmake/caches/AIX.cmake
libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake
libcxx/cmake/caches/s390x-ibm-zos.cmake
libcxx/include/__configuration/abi.h
libcxx/include/ios
Removed:
################################################################################
diff --git a/libcxx/cmake/caches/AIX.cmake b/libcxx/cmake/caches/AIX.cmake
index c01aa5b14df06..4ec78f9bbd592 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 95b7cbe776e05..68b1cc8eff9b0 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 3eaed3e67fc16..e51d5ff31ebfe 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 513da6e3b81b6..cbde7887becf1 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<char_type>::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 0a813c07721fe..d8a3643c7ad50 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 <class _Traits>
+// 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 <class _Traits>
+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 _CharT, class _Traits>
class _LIBCPP_TEMPLATE_VIS basic_ios : public ios_base {
public:
@@ -588,7 +620,13 @@ protected:
private:
basic_ostream<char_type, traits_type>* __tie_;
- mutable int_type __fill_;
+
+#if defined(_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE)
+ using _FillType = _FillHelper<traits_type>;
+#else
+ using _FillType = _SentinelValueFill<traits_type>;
+#endif
+ mutable _FillType __fill_;
};
template <class _CharT, class _Traits>
@@ -603,7 +641,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_.__init();
}
template <class _CharT, class _Traits>
@@ -653,16 +691,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_.__get();
}
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_.__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 0000000000000..f22850877dd62
--- /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 <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)WCHAR_MAX);
+ assert(os.fill() == (wchar_t)WCHAR_MAX);
+
+ return 0;
+}
More information about the libcxx-commits
mailing list