Skip to content

Commit 194f98c

Browse files
xingxue-ibmldionnedaltenty
authored
[libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX (llvm#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@gmail.com> Co-authored-by: David Tenty <daltenty.dev@gmail.com>
1 parent 130ef73 commit 194f98c

File tree

6 files changed

+92
-6
lines changed

6 files changed

+92
-6
lines changed

libcxx/cmake/caches/AIX.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ set(LIBCXXABI_ENABLE_STATIC OFF CACHE BOOL "")
1515
set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
1616
set(LIBUNWIND_ENABLE_SHARED ON CACHE BOOL "")
1717
set(LIBUNWIND_ENABLE_STATIC OFF CACHE BOOL "")
18+
set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE" CACHE STRING "")

libcxx/cmake/caches/s390x-ibm-zos-ascii.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,4 @@ set(LIBCXX_CXX_ABI system-libcxxabi CACHE STRING "")
2020

2121
set(LIBCXX_ADDITIONAL_COMPILE_FLAGS "-fzos-le-char-mode=ascii" CACHE STRING "")
2222
set(LIBCXX_ADDITIONAL_LIBRARIES "-L../s390x-ibm-zos/lib -Wl,../s390x-ibm-zos/lib/libunwind.x" CACHE STRING "")
23+
set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE" CACHE STRING "")

libcxx/cmake/caches/s390x-ibm-zos.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ set(LIBCXX_DLL_NAME CRTEQCXE CACHE STRING "")
1515

1616
set(LIBCXXABI_DLL_NAME CRTEQCXA CACHE STRING "")
1717
set(LIBCXXABI_ADDITIONAL_LIBRARIES "-Wl,lib/libunwind.x" CACHE STRING "")
18+
set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE" CACHE STRING "")

libcxx/include/__configuration/abi.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@
9191
# define _LIBCPP_ABI_USE_WRAP_ITER_IN_STD_STRING_VIEW
9292
// Dont' add an inline namespace for `std::filesystem`
9393
# define _LIBCPP_ABI_NO_FILESYSTEM_INLINE_NAMESPACE
94+
// std::basic_ios uses WEOF to indicate that the fill value is
95+
// uninitialized. However, on platforms where the size of char_type is
96+
// equal to or greater than the size of int_type and char_type is unsigned,
97+
// std::char_traits<char_type>::eq_int_type() cannot distinguish between WEOF
98+
// and WCHAR_MAX. This ABI setting determines whether we should instead track whether the fill
99+
// value has been initialized using a separate boolean, which changes the ABI.
100+
# define _LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE
94101
#elif _LIBCPP_ABI_VERSION == 1
95102
# if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
96103
// Enable compiling copies of now inline methods into the dylib to support

libcxx/include/ios

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,38 @@ inline _LIBCPP_HIDE_FROM_ABI void ios_base::exceptions(iostate __iostate) {
519519
clear(__rdstate_);
520520
}
521521

522+
template <class _Traits>
523+
// Attribute 'packed' is used to keep the layout compatible with the previous
524+
// definition of the '__fill_' and '_set_' pair in basic_ios on AIX & z/OS.
525+
struct _LIBCPP_PACKED _FillHelper {
526+
_LIBCPP_HIDE_FROM_ABI void __init() { __set_ = false; }
527+
_LIBCPP_HIDE_FROM_ABI _FillHelper& operator=(typename _Traits::int_type __x) {
528+
__set_ = true;
529+
__fill_val_ = __x;
530+
return *this;
531+
}
532+
_LIBCPP_HIDE_FROM_ABI bool __is_set() const { return __set_; }
533+
_LIBCPP_HIDE_FROM_ABI typename _Traits::int_type __get() const { return __fill_val_; }
534+
535+
private:
536+
typename _Traits::int_type __fill_val_;
537+
bool __set_;
538+
};
539+
540+
template <class _Traits>
541+
struct _LIBCPP_PACKED _SentinelValueFill {
542+
_LIBCPP_HIDE_FROM_ABI void __init() { __fill_val_ = _Traits::eof(); }
543+
_LIBCPP_HIDE_FROM_ABI _SentinelValueFill& operator=(typename _Traits::int_type __x) {
544+
__fill_val_ = __x;
545+
return *this;
546+
}
547+
_LIBCPP_HIDE_FROM_ABI bool __is_set() const { return __fill_val_ != _Traits::eof(); }
548+
_LIBCPP_HIDE_FROM_ABI typename _Traits::int_type __get() const { return __fill_val_; }
549+
550+
private:
551+
typename _Traits::int_type __fill_val_;
552+
};
553+
522554
template <class _CharT, class _Traits>
523555
class _LIBCPP_TEMPLATE_VIS basic_ios : public ios_base {
524556
public:
@@ -588,7 +620,13 @@ protected:
588620

589621
private:
590622
basic_ostream<char_type, traits_type>* __tie_;
591-
mutable int_type __fill_;
623+
624+
#if defined(_LIBCPP_ABI_IOS_ALLOW_ARBITRARY_FILL_VALUE)
625+
using _FillType = _FillHelper<traits_type>;
626+
#else
627+
using _FillType = _SentinelValueFill<traits_type>;
628+
#endif
629+
mutable _FillType __fill_;
592630
};
593631

594632
template <class _CharT, class _Traits>
@@ -603,7 +641,7 @@ template <class _CharT, class _Traits>
603641
inline _LIBCPP_HIDE_FROM_ABI void basic_ios<_CharT, _Traits>::init(basic_streambuf<char_type, traits_type>* __sb) {
604642
ios_base::init(__sb);
605643
__tie_ = nullptr;
606-
__fill_ = traits_type::eof();
644+
__fill_.__init();
607645
}
608646

609647
template <class _CharT, class _Traits>
@@ -653,16 +691,16 @@ inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::widen(char __c)
653691

654692
template <class _CharT, class _Traits>
655693
inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::fill() const {
656-
if (traits_type::eq_int_type(traits_type::eof(), __fill_))
694+
if (!__fill_.__is_set())
657695
__fill_ = widen(' ');
658-
return __fill_;
696+
return __fill_.__get();
659697
}
660698

661699
template <class _CharT, class _Traits>
662700
inline _LIBCPP_HIDE_FROM_ABI _CharT basic_ios<_CharT, _Traits>::fill(char_type __ch) {
663-
if (traits_type::eq_int_type(traits_type::eof(), __fill_))
701+
if (!__fill_.__is_set())
664702
__fill_ = widen(' ');
665-
char_type __r = __fill_;
703+
char_type __r = __fill_.__get();
666704
__fill_ = __ch;
667705
return __r;
668706
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// Test that WCHAR_MAX as a wchar_t value can be set as the fill character.
10+
11+
// UNSUPPORTED: no-wide-characters
12+
13+
// Expect the test case to fail on targets where WEOF is the same as
14+
// WCHAR_MAX with the libcpp ABI version 1 implementation. The libcpp ABI
15+
// version 2 implementation fixes the problem.
16+
17+
// XFAIL: target={{.*}}-windows{{.*}} && libcpp-abi-version=1
18+
// XFAIL: target=armv{{7|8}}l-linux-gnueabihf && libcpp-abi-version=1
19+
// XFAIL: target=aarch64-linux-gnu && libcpp-abi-version=1
20+
21+
#include <iomanip>
22+
#include <ostream>
23+
#include <cassert>
24+
#include <string>
25+
26+
template <class CharT>
27+
struct testbuf : public std::basic_streambuf<CharT> {
28+
testbuf() {}
29+
};
30+
31+
int main(int, char**) {
32+
testbuf<wchar_t> sb;
33+
std::wostream os(&sb);
34+
os << std::setfill((wchar_t)WCHAR_MAX);
35+
assert(os.fill() == (wchar_t)WCHAR_MAX);
36+
37+
return 0;
38+
}

0 commit comments

Comments
 (0)