[libcxx-commits] [libcxx] [openmp] [libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX (PR #89305)

Xing Xue via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jun 3 10:49:36 PDT 2024


https://github.com/xingxue-ibm updated https://github.com/llvm/llvm-project/pull/89305

>From 68c3976a4c3210dbfb8f9d1e9cafbba4c5e7f27b Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Wed, 10 Apr 2024 13:32:16 -0400
Subject: [PATCH 1/4] Use lower max threads to reduce the testing time on AIX.

---
 openmp/runtime/test/worksharing/for/collapse_test.inc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/openmp/runtime/test/worksharing/for/collapse_test.inc b/openmp/runtime/test/worksharing/for/collapse_test.inc
index de0e7e4e57f30..37d80a56a68db 100644
--- a/openmp/runtime/test/worksharing/for/collapse_test.inc
+++ b/openmp/runtime/test/worksharing/for/collapse_test.inc
@@ -15,7 +15,12 @@
 #define LOOP_TYPE2 LOOP_TYPES
 #define LOOP_STYPE2 LOOP_TYPES
 
+#if defined _AIX
+// Use lower max threads to reduce the testing time
+#define MAX_THREADS 64
+#else
 #define MAX_THREADS 256
+#endif
 
 #if defined VERBOSE
 #define PRINTF printf

>From 0d63bb8e681cbb8376836a08501bf4bf37c08a74 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 2/4] 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 00c1d5c2d4bc5..bc31f30f14acf 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 0000000000000..042e07cb80bbd
--- /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;
+}

>From 841c815b90a22383361ed2fd7b3dbaeba8253a7a Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Fri, 19 Apr 2024 11:05:18 -0400
Subject: [PATCH 3/4] Addressed comments. - update a comment

---
 libcxx/include/ios | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/ios b/libcxx/include/ios
index bc31f30f14acf..3961504f62b39 100644
--- a/libcxx/include/ios
+++ b/libcxx/include/ios
@@ -524,7 +524,7 @@ inline _LIBCPP_HIDE_FROM_ABI void ios_base::exceptions(iostate __iostate) {
 
 template <class _Traits>
 // Attribute 'packed' is used to keep the layout compatible with the previous
-// definition of '__set_' in basic_ios on AIX.
+// definition of the '__fill_' and '_set_' pair in basic_ios on AIX & z/OS.
 struct _LIBCPP_PACKED _OptionalFill {
   _OptionalFill() : __set_(false) { }
   _OptionalFill& operator=(typename _Traits::int_type __x) { __set_ = true; __fill_val_ = __x; return *this; }

>From 18cd6817d1f5cfa5d4199636a91e195924bb8829 Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Mon, 3 Jun 2024 13:23:10 -0400
Subject: [PATCH 4/4] Addressed comments. -define and use macros
 _LIBCXX_IOS_MAY_USE_OPTIONAL_FILL and _LIBCXX_IOS_FORCE_OPTIONAL_FILL.

---
 libcxx/include/__configuration/abi.h | 13 +++++++++++++
 libcxx/include/ios                   | 15 +++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h
index 17aceb042f524..b83e7bf97a559 100644
--- a/libcxx/include/__configuration/abi.h
+++ b/libcxx/include/__configuration/abi.h
@@ -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
+// 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
@@ -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,
diff --git a/libcxx/include/ios b/libcxx/include/ios
index 3961504f62b39..d056b1ccb8681 100644
--- a/libcxx/include/ios
+++ b/libcxx/include/ios
@@ -617,21 +617,16 @@ protected:
 private:
   basic_ostream<char_type, traits_type>* __tie_;
 
-#if defined(_AIX) || (defined(__MVS__) && defined(__64BIT__))
-// AIX and 64-bit MVS must use _OptionalFill for ABI backward compatibility.
+#if defined(_LIBCXX_IOS_FORCE_OPTIONAL_FILL)
   using _FillType = _OptionalFill<_Traits>;
-#else
-#if defined(_WIN32)
-  static const bool _OptOutForABICompat = true;
-#else
-  static const bool _OptOutForABICompat = false;
-#endif
-
+#elif defined(_LIBCXX_IOS_MAY_USE_OPTIONAL_FILL)
   using _FillType = _If<
-      sizeof(char_type) >= sizeof(int_type) && !_OptOutForABICompat,
+      sizeof(char_type) >= sizeof(int_type),
       _OptionalFill<_Traits>,
       _SentinelValueFill<_Traits>
   >;
+#else
+  using _FillType = _SentinelValueFill<_Traits>;
 #endif
   mutable _FillType __fill_;
 };



More information about the libcxx-commits mailing list