[Openmp-commits] [libcxx] [openmp] [libc++] basic_ios<wchar_t> cannot store fill character WCHAR_MAX (PR #89305)
Xing Xue via Openmp-commits
openmp-commits at lists.llvm.org
Tue Jun 4 09:22:56 PDT 2024
https://github.com/xingxue-ibm updated https://github.com/llvm/llvm-project/pull/89305
>From 7bd12fb10c31a2ce354a6cfcee63938ad0597013 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/7] 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 3075bd04e958f..74ec83a059d91 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(__VA_ARGS__)
>From ecbcaa8f504f86b138202dd853403166df0f60cd 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/7] 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 a54d54a900d6da9d3164b02cb86d5379f0d70c35 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/7] 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 8fa650aa6a5bf5caa6d235a7fe8cca93e39ad5fd 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/7] 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_;
};
>From c033ea9b0486c5dd8a240cb01aee05848dff4a65 Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Tue, 4 Jun 2024 06:51:49 -0400
Subject: [PATCH 5/7] Add _LIBCPP_HIDE_FROM_ABI.
---
libcxx/include/ios | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/libcxx/include/ios b/libcxx/include/ios
index d056b1ccb8681..3099dbe8d9f96 100644
--- a/libcxx/include/ios
+++ b/libcxx/include/ios
@@ -526,10 +526,10 @@ 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 _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_; }
+ _LIBCPP_HIDE_FROM_ABI _OptionalFill() : __set_(false) { }
+ _LIBCPP_HIDE_FROM_ABI _OptionalFill& 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 __fill() const { return __fill_val_; }
private:
typename _Traits::int_type __fill_val_;
@@ -538,10 +538,10 @@ private:
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_; }
+ _LIBCPP_HIDE_FROM_ABI _SentinelValueFill() : __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 __fill() const { return __fill_val_; }
private:
typename _Traits::int_type __fill_val_;
>From 5288cb33586bd5ac22efabfde95ff143222c366c Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Tue, 4 Jun 2024 07:08:40 -0400
Subject: [PATCH 6/7] Removed changes in unrelated test case.
---
openmp/runtime/test/worksharing/for/collapse_test.inc | 5 -----
1 file changed, 5 deletions(-)
diff --git a/openmp/runtime/test/worksharing/for/collapse_test.inc b/openmp/runtime/test/worksharing/for/collapse_test.inc
index 74ec83a059d91..3075bd04e958f 100644
--- a/openmp/runtime/test/worksharing/for/collapse_test.inc
+++ b/openmp/runtime/test/worksharing/for/collapse_test.inc
@@ -15,12 +15,7 @@
#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(__VA_ARGS__)
>From 00f4cd285b2a8281168486377a170246a33e1c34 Mon Sep 17 00:00:00 2001
From: Xing Xue <xingxue at outlook.com>
Date: Tue, 4 Jun 2024 12:20:39 -0400
Subject: [PATCH 7/7] Add description of ABI change in ReleaseNote.
---
libcxx/docs/ReleaseNotes/19.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index 0bc343acd281c..935d065a5e0ac 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -146,6 +146,13 @@ ABI Affecting Changes
``random_device`` could throw a ``system_error`` with this value. It now
throws ``ENOMSG``.
+- 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. This is an ABI break on platforms where the size of ``char_type`` is equal to or greater
+ than the size of ``int_type``. Existing targets affected by this change can choose to keep the existing ABI
+ by undefining macro ``_LIBCXX_IOS_MAY_USE_OPTIONAL_FILL``.
Build System Changes
--------------------
More information about the Openmp-commits
mailing list