[SystemZ][ZOS] Porting the time functions within libc++ to z/OS

[ldionne]

Summary

This patch is one part of many steps required to build libc++ and libc++abi libraries on z/OS. This particular deals with time related functions and consists of the following 3 parts.

1. Initialization of :timeval within libc++ library need to be adjusted to work on z/OS.

The following is z/OS definition from time.h which includes additional aggregate member.
typedef signed int suseconds_t;
struct timeval {
time_t tv_sec;
char tv_usec_pad[4];
suseconds_t tv_usec;
};

In contracts the following is definition from time.h on Linux.

typedef long int suseconds_t;
struct timeval
{
time_t tv_sec;
__suseconds_t tv_usec;
};

2. In addition, retrieving ::timespec within libc++ library needs to be adjusted to compensate the difference of some of the members of ::stat depending of the target host.

Here are the 2 members in conflict on z/OS extracted from stat.h.
struct stat {
...
time_t st_atime;
time_t st_mtime;
...
};
In contract here is Linux equivalent from stat.h.
struct stat
{
...
struct timespec st_atim;
struct timespec st_mtim;
...
};

3. On Linux both members are of type timespec whereas on z/OS an object of type timespec need to be constructed first before retrieving it within libc++ library.

The libc++ header file __threading_support calls nanosleep, which is not available on z/OS.
The equivalent functionality will be implemented by using both sleep() and usleep().

[ldionne]

Assuming you already have them, I'd love to see the rest of the patches in the queue to know what we're getting into.

Also, if we're supporting a new platform, it needs to be tested. This means you need to add a bot testing libcxx on your system to the llvm builders.

libcxx/include/__threading_support

5

Can z/OS implement nanosleep() like this, then? Sounds better to implement it once in your system library than in all the places it might be used.

libcxx/src/filesystem/filesystem_common.h

15

So IIUC, timeval on z/OS is non-conforming to https://pubs.opengroup.org/onlinepubs/007908775/xsh/systime.h.html?

If we start having such differences, I think it would be better to introduce a __libcpp_timeval typedef and a __libcpp_make_timeval(sec, nsec) function. We have similar "abstraction" layers in the threading library.

[zibi]

I introduced TimeVal type and make_timeval(TimeSpec const&).

I did not introduced __libcpp_timeval since that would be struct ::timeval on all plafromts anyway.. Definition of ::timeval on z/OS is conforming to the standard since it provides 2 required members. Please refer to the actual definition in description.

[zibi]

@ldionne Louis please have a look again. I was not sure if you got the notification.

libcxx/include/__threading_support

5

Yes, this is our goal to eventually have nanosleep() part of the OS and eventually remove this workaround.

libcxx/src/filesystem/filesystem_common.h

15

I will experiment with your suggestion and post a new patch if successful.

As for the non-conforming I'm not sure because z/OS do provide both tv_sev and tv_usec. It all boils down to the underlying type of suseconds_t.

[zibi]

There are multiple Pre-merge checks warnings coming from clang-tidy. Half of them are for existing function posix_utimes and another half for new function make_timeval as follows:

1. defining function in the header
2. invalid case for function
3. invalid case for parameter

I intend to ignore them otherwise, the style of the file will be mixed and hard to read. Perhaps this file should be excluded from tidy checks.

There is also the following build failure not related to this patch which I'm not sure how to report it to eventually fix it. It looks like it has been happening for couple months now.

error: ld.lld: error: unable to find library -lcxxabi_shared

[zibi]

ping

[hubert.reinterpretcast]

libcxx/include/__threading_support

538–541

Minor comment: Wording suggestion.

546–547

If it was worthwhile to write the test as > 999999, then it is worthwhile to write the reduction in microseconds as -= 1000000.
Also, minor nit: Indentation.

552

Unfortunately, if a signal occurs, we have no convenient way of knowing how much time is left. We can still loop though.

libcxx/src/filesystem/filesystem_common.h

408–417

Let's use a more generic change here instead of a change that regresses z/OS under 31-bit.

[zibi]

Thank you Hubert for comments. I specially like the part which eliminated #if statement.

[hubert.reinterpretcast]

libcxx/include/__threading_support

552

This is the loop I mentioned. It ensures we do sleep as long as requested but has the problem of not knowing how long we have already slept for.

[zibi]

Thank you Hubert, now usleep will loop in case it was unsuccessful.

[hubert.reinterpretcast]

@ldionne, does this patch look okay to you? In terms of changes being made for z/OS, it's mainly going to be:

• lack of LSB and BSD or GNU extension APIs,
• older UNIX conformance level,
• EBCDIC/ASCII mode support, and
• versioning for IEEE floating point.

I understand that a lot of context would be needed for the latter two, but I believe that the first two is not particularly special and introduces only the usual amount of maintenance burden.
Has it been a norm to request a full set of patches from the get-go before accepting patches that adjust for API differences?

[zibi]

ping

libcxx/include/__threading_support

552

This is the loop I mentioned. It ensures we do sleep as long as requested but has the problem of not knowing how long we have already slept for.

Thx, Hubert I was not sure if you wanted to make that change when you first time commented out.

[ldionne]

In D87940#2299436, @hubert.reinterpretcast wrote:

@ldionne, does this patch look okay to you? In terms of changes being made for z/OS, it's mainly going to be:

I'm okay with the changes, but I don't think it makes sense to implement nanosleep() upstream. Are you willing to keep this workaround downstream until z/OS implements nanosleep? This also gives you a nice incentive to implement nanosleep(). :-)

• lack of LSB and BSD or GNU extension APIs,
• older UNIX conformance level,
• EBCDIC/ASCII mode support, and
• versioning for IEEE floating point.

I understand that a lot of context would be needed for the latter two, but I believe that the first two is not particularly special and introduces only the usual amount of maintenance burden.
Has it been a norm to request a full set of patches from the get-go before accepting patches that adjust for API differences?

There's been no norm so far, and libc++ has greatly suffered from that laxness. We now "support" several platforms that are not tested and have nobody working on them. That situation is bad for everyone: the platforms are not properly supported, and there's dead (or not dead?) code to account for these platforms everywhere. We can go one patch at a time, but be aware that we might push back on some changes if (1) they are too intrusive and (2) we don't have good ways to isolate those from the rest of the code, and hence they add a lot of unavoidable complexity. We'll try to find solutions to make it reasonable, of course.

I'm trying to start being more principled around new configurations/platform support. For example, are you able to provide a build bot to build on your system?

libcxx/src/filesystem/filesystem_common.h

414

I don't understand this comment.

[zibi]

Removing confusing comment is the only delta.

@ldionne There are plans to have an external buildbot setup on z/OS. I’ll update with more details after discussing with the people trying to figure out the logistics.

As for the nanosleep() we would like to keep it upstream it should not be a burden since it's consolidated in one location and clearly guarded for z/OS only. The nanosleep() is on our list to implement downstream so the fact we are upstreaming will not minimize our effort to make that function available as part of OS.

[ldionne]

In D87940#2322671, @zibi wrote:

Removing confusing comment is the only delta.

@ldionne There are plans to have an external buildbot setup on z/OS. I’ll update with more details after discussing with the people trying to figure out the logistics.

As for the nanosleep() we would like to keep it upstream it should not be a burden since it's consolidated in one location and clearly guarded for z/OS only. The nanosleep() is on our list to implement downstream so the fact we are upstreaming will not minimize our effort to make that function available as part of OS.

If you want to keep nanosleep() upstream, please:

1. Put it in a separate header in src/zos/nanosleep.h and call the function nanosleep
2. Just include the header in filesystem_common.h

That way, we won't need the #if in the middle of the logic. I might appear overly strict on not adding new #ifs, but these are the number one thing making libc++'s code complicated.

[zibi]

@ldionne Sorry, I was away for few days. I hid nanosleep() in a separated header to make code in __threading_support cleaner. Please have a look.

[zibi]

removing traces

[zibi]

libcxx/src/filesystem/filesystem_common.h

414

I don't understand this comment.

I don't understand either. The diff is misleading since this is NOT a new comment. I will delete this comment.

[ldionne]

libcxx/include/CMakeLists.txt

165 ↗

(On Diff #298695)

Please put it below in the if (LIBCXX_INSTALL_SUPPORT_HEADERS), and not dependent on ZOS.

libcxx/include/support/ibm/nanosleep.h

16

This needs to be marked inline to avoid ODR violations.

libcxx/src/filesystem/filesystem_common.h

387

inline.

391

inline.

401

inline.

414

Let's take this occasion to make this inline.

[zibi]

Thank you Louis for the review and suggestions. Please have a look again.

[ldionne]

LGTM except nitpick about header guard. Please rebase onto master, let CI run, and then we're good to go.

libcxx/include/support/ibm/nanosleep.h

11

The header include guard should be _LIBCPP_SUPPORT_IBM_NANOSLEEP_H.

[zibi]

Thank you Louis, I updated as requested.

[zibi]

set correct repo

[zibi]

rebase

[zibi]

I'm resubmitting the same patch to redo CI to see if it will regressions.

[zibi]

Fixing CI.

[thakis]

libcxx/include/CMakeLists.txt

163 ↗

(On Diff #304849)

Did you mean to remove the support headers up here? We now include all of them twice in files (except for nanosleep, which is added only below, not up here)

[ldionne]

Looks like you had some problems with git.

[ldionne]

Re-upload a corrected patch. I'll let CI run and then I'll re-apply it myself.

[ldionne]

Restore commit author to the real author, not me (for credit).

[ldionne]

In the future, please also try to distil your commit message to something that explains the gist of the change. Going into many details might be useful for the Phabricator review, but not in the commit message itself.

[ldionne]

Add missing nanosleep.h header to the header list. This must have been the cause of the confusion when checking-in the patch the first time around.

[zibi]

Thank you Louis for resubmitting it, I already had the patch ready. Also thank you to Nico for spotting the issue.

[zibi]

@ldionne Louis, please use the following as commit message:

This patch is one part of many steps required to build libc++ and libc++abi libraries on z/OS.
This particular deals with time related functions and consists of the following:

1. Initialization of struct imeval within libc++ library need to be adjusted for additional aggregate member.
2. Retrieving struct timespec have to be compensated for the difference of some of the members within `struct stat.
3. The nanosleep() is not available on z/OS. This patch will provide equivalent functionality using both sleep() and usleep().

libcxx/include/__threading_support

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP_THREADING_SUPPORT
 #define _LIBCPP_THREADING_SUPPORT
 
 #include <__config>
 #include <__availability>
 #include <chrono>
 #include <iosfwd>
 #include <errno.h>
 
+#ifdef __MVS__
+# include <support/ibm/nanosleep.h>
+#endif
+
 #ifndef _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #pragma GCC system_header
 #endif
 
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 # include <__external_threading>
 #elif !defined(_LIBCPP_HAS_NO_THREADS)
 

 {
   sched_yield();
 }
 
 void __libcpp_thread_sleep_for(const chrono::nanoseconds& __ns)
 {
    __libcpp_timespec_t __ts = __thread_detail::__convert_to_timespec(__ns);
    while (nanosleep(&__ts, &__ts) == -1 && errno == EINTR);
 }
 
 // Thread local storage
 int __libcpp_tls_create(__libcpp_tls_key *__key, void (*__at_exit)(void *))
 {
   return pthread_key_create(__key, __at_exit);
 }
 
 void *__libcpp_tls_get(__libcpp_tls_key __key)
 {
   return pthread_getspecific(__key);
 }
 
 int __libcpp_tls_set(__libcpp_tls_key __key, void *__p)
 {
     return pthread_setspecific(__key, __p);
 }
 
 #elif defined(_LIBCPP_HAS_THREAD_API_C11)
 
 int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m)
 {
   return mtx_init(__m, mtx_plain | mtx_recursive) == thrd_success ? 0 : EINVAL;

libcxx/include/support/ibm/nanosleep.h

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP_SUPPORT_IBM_NANOSLEEP_H
+#define _LIBCPP_SUPPORT_IBM_NANOSLEEP_H
+
+#include <unistd.h>
+
+inline int nanosleep(const struct timespec* req, struct timespec* rem)
+{
+   // The nanosleep() function is not available on z/OS. Therefore, we will call
+   // sleep() to sleep for whole seconds and usleep() to sleep for any remaining
+   // fraction of a second. Any remaining nanoseconds will round up to the next
+   // microsecond.
+
+   useconds_t __micro_sec = (rem->tv_nsec + 999) / 1000;
+   if (__micro_sec > 999999)
+   {
+     ++rem->tv_sec;
+     __micro_sec -= 1000000;
+   }
+   while (rem->tv_sec)
+      rem->tv_sec = sleep(rem->tv_sec);
+   if (__micro_sec) {
+     rem->tv_nsec = __micro_sec * 1000;
+     return usleep(__micro_sec);
+   }
+   rem->tv_nsec = 0;
+   return 0;
+}
+
+#endif // _LIBCPP_SUPPORT_IBM_NANOSLEEP_H

libcxx/src/filesystem/filesystem_common.h

 //===----------------------------------------------------------------------===////
 //
 // 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
 //
 //===----------------------------------------------------------------------===////
 
 #ifndef FILESYSTEM_COMMON_H
 #define FILESYSTEM_COMMON_H
 
 #include "__config"
 #include "filesystem"
 #include "array"
 #include "chrono"
 #include "climits"
 #include "cstdlib"
 #include "ctime"
 
 #include <unistd.h>
 #include <sys/stat.h>
 #include <sys/statvfs.h>
 #include <sys/time.h> // for ::utimes as used in __last_write_time

 private:
   ErrorHandler(ErrorHandler const&) = delete;
   ErrorHandler& operator=(ErrorHandler const&) = delete;
 };
 
 using chrono::duration;
 using chrono::duration_cast;
 
-using TimeSpec = timespec;
+using TimeSpec = struct timespec;
+using TimeVal = struct timeval;
 using StatT = struct stat;
 
 template <class FileTimeT, class TimeT,
           bool IsFloat = is_floating_point<typename FileTimeT::rep>::value>
 struct time_util_base {
   using rep = typename FileTimeT::rep;
   using fs_duration = typename FileTimeT::duration;
   using fs_seconds = duration<rep>;
if (!is_representable(tp))
       return false;
     return set_times_checked(&dest.tv_sec, &dest.tv_nsec, tp);
   }
 };
 
 using fs_time = time_util<file_time_type, time_t, TimeSpec>;
 
 #if defined(__APPLE__)
-TimeSpec extract_mtime(StatT const& st) { return st.st_mtimespec; }
-TimeSpec extract_atime(StatT const& st) { return st.st_atimespec; }
+inline TimeSpec extract_mtime(StatT const& st) { return st.st_mtimespec; }
+inline TimeSpec extract_atime(StatT const& st) { return st.st_atimespec; }
+#elif defined(__MVS__)
+inline TimeSpec extract_mtime(StatT const& st) {
+  TimeSpec TS = {st.st_mtime, 0};
+  return TS;
+}
+inline TimeSpec extract_atime(StatT const& st) {
+  TimeSpec TS = {st.st_atime, 0};
+  return TS;
+}
 #else
-TimeSpec extract_mtime(StatT const& st) { return st.st_mtim; }
-TimeSpec extract_atime(StatT const& st) { return st.st_atim; }
+inline TimeSpec extract_mtime(StatT const& st) { return st.st_mtim; }
+inline TimeSpec extract_atime(StatT const& st) { return st.st_atim; }
 #endif
 
-// allow the utimes implementation to compile even it we're not going
-// to use it.
-
-bool posix_utimes(const path& p, std::array<TimeSpec, 2> const& TS,
-                  error_code& ec) {
+inline TimeVal make_timeval(TimeSpec const& ts) {
   using namespace chrono;
   auto Convert = [](long nsec) {
-    using int_type = decltype(std::declval< ::timeval>().tv_usec);
+    using int_type = decltype(std::declval<TimeVal>().tv_usec);
     auto dur = duration_cast<microseconds>(nanoseconds(nsec)).count();
     return static_cast<int_type>(dur);
   };
-  struct ::timeval ConvertedTS[2] = {{TS[0].tv_sec, Convert(TS[0].tv_nsec)},
-                                     {TS[1].tv_sec, Convert(TS[1].tv_nsec)}};
+  TimeVal TV = {};
+  TV.tv_sec = ts.tv_sec;
+  TV.tv_usec = Convert(ts.tv_nsec);
+  return TV;
+}
+
+inline bool posix_utimes(const path& p, std::array<TimeSpec, 2> const& TS,
+                  error_code& ec) {
+  TimeVal ConvertedTS[2] = {make_timeval(TS[0]), make_timeval(TS[1])};
   if (::utimes(p.c_str(), ConvertedTS) == -1) {
     ec = capture_errno();
     return true;
   }
   return false;
 }
 
 #if defined(_LIBCPP_USE_UTIMENSAT)
 bool posix_utimensat(const path& p, std::array<TimeSpec, 2> const& TS,