[SystemZ][ZOS] Provide PATH_MAX macro for libcxx

[zibi]

Summary

Defining PATH_MAX to _XOPEN_PATH_MAX which is the closest macro available on z/OS.
Note that this value is 1024 which is 4 times smaller from same macro on Linux.

[ldionne]

libcxx/src/filesystem/operations.cpp

1193

We already try to workaround the lack of PATH_MAX. Where are you seeing errors?

[zibi]

libcxx/src/filesystem/operations.cpp:549:13: error: use of undeclared identifier 'PATH_MAX'

char buff[PATH_MAX + 1];
          ^

1 error generated.

[zibi]

Alternatively we can do this:

-#if defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112
+#if defined(MVS) || (defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112)

[zibi]

In D92110#2419029, @zibi wrote:

Alternatively we can do this:

-#if defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112
+#if defined(MVS) || (defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112)

Actually we can not do this because out implementation of realpath does not do malloc.
We are forced to defined PATH_MAX as I originally proposed.

[zibi]

ping

[hubert.reinterpretcast]

libcxx/src/filesystem/operations.cpp

52

I've been told that PATH_MAX is indeed limited to _XOPEN_PATH_MAX and that seems to be the case (based on experimentation) with realpath, but I have not seen documentation that supports such an assertion. The scope of this macro definition may be too broad as a solution of dealing with realpath.

[zibi]

libcxx/src/filesystem/operations.cpp

52

Will avoid defining the macro altogether.

[zibi]

Avoid using PATH_MAX on z/OS.

[hubert.reinterpretcast]

Thanks; the targeted fix for handling realpath (where we have experimentally verified that the operation does fail with paths longer than 1024) seems reasonable to me. Looks good from my end.

libcxx/src/filesystem/operations.cpp

638

Minor nit: style.

[ldionne]

libcxx/src/filesystem/operations.cpp

638

Hardcoding buffer sizes like that is a great way to trigger a buffer overflow when assumptions are not respected. I would much rather we don't introduce this sort of code in libc++, to be honest. Is there a reason why zOS can't just provide PATH_MAX?

Or, even much better, why doesn't zOS satisfy defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112? Then, we could use the safe and non-buggy version of realpath. The buffer-taking version is documented in various places, including realpath's manpage itself, as being buggy.

[zibi]

Handing nit.

[hubert.reinterpretcast]

libcxx/src/filesystem/operations.cpp

638

Hardcoding buffer sizes like that is a great way to trigger a buffer overflow when assumptions are not respected.

Precisely because violating the assumptions mean that buffer overflow would be introduced into compiled programs, changes that would cause the assumption to not be respected cannot realistically be made.

I would much rather we don't introduce this sort of code in libc++, to be honest. Is there a reason why zOS can't just provide PATH_MAX?

The POSIX specification provides rationale for not defining PATH_MAX in <limits.h>.

Or, even much better, why doesn't zOS satisfy defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112? Then, we could use the safe and non-buggy version of realpath. The buffer-taking version is documented in various places, including realpath's manpage itself, as being buggy.

Deployed systems have stability considerations that may be incompatible with such a change. The IBM documentation for released versions clearly state that passing NULL to the second parameter produces an error. Indeed the bugginess of the buffer-taking version is why the hardcoded 1024 that the implementation of realpath on z/OS uses is preferable to an implementation that does not implement a fixed upper bound on the buffer-taking form of realpath.

[ldionne]

libcxx/src/filesystem/operations.cpp

638

What are your deployment goals? What versions of z/OS do you want libc++ to work on? Also, where's the documentation you're referring to?

[hubert.reinterpretcast]

libcxx/src/filesystem/operations.cpp

638

re: EINVAL when using NULL for the second argument, see:
https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.4.0/com.ibm.zos.v2r4.bpxbd00/rpath.htm

re: ENAMETOOLONG, the fixed limits is documented for what could be thought of as the "system call interface":
https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.4.0/com.ibm.zos.v2r4.bpxb100/rph.htm

re: deployment goals, I am not in a position to provide that information (@zibi, can you say more?)

[zibi]

libcxx/src/filesystem/operations.cpp

638

Unfortunately we do not have the safe version of realpath and the value of _POSIZ_VERSOIN is much lower. The deployment goals are still in flux.

Here is another proposal:

#if defines(__MVS__) && !defined(PATH_MAX) 
  char buff[ _XOPEN_PATH_MAX + 1 ];

[zibi]

On z/OS we have multiple file systems with different limits which does not allow to have just one constant for PATH_MAX. Same is true for other related limits like NAME_MAX. See the following references which justify the z/OS choice.

POSIX.1 document:
The values in the following list may be constants within an implementation or may vary from one pathname to another. For example, file systems or directories may have different characteristics.

A definition of one of the symbolic constants in the following list shall be omitted from the <limits.h> header on specific implementations where the corresponding value is equal to or greater than the stated minimum, but where the value can vary depending on the file to which it is applied. The actual value supported for a specific pathname shall be provided by the pathconf() function.

GNU document
If the system allows different file systems or files to have different limits, then the macro is undefined; use pathconf or fpathconf to find out the limit that applies to a particular file. See Pathconf.

[zibi]

ping

[zibi]

We have to use _XOPEN_PATH_MAX instead of PATH_MAX on z/OS.

[zibi]

fixing typo

[hubert.reinterpretcast]

libcxx/src/filesystem/operations.cpp

638

I don't see why _XOPEN_PATH_MAX is better. Yes, it is defined to have 1024 as its value, but it's not defined (in purpose) to be directly related for this usage. The realpath callable service describes the maximum length returned without \0 termination as literally 1023 with no indirection of referring to some named constant involved.

[zibi]

libcxx/src/filesystem/operations.cpp

638

The only advantage is that _XOPEN_PATH_MAX is defined by system header outside of LLVM with the value we want to use anyway and avoid hardcoding the value here. However, I'm fine with any proposal as long we approve and move on.

[zibi]

@ldionne which suggestion do you prefer?

[ldionne]

In D92110#2496031, @zibi wrote:

@ldionne which suggestion do you prefer?

Neither.

The documentation Hubert pointed to clearly mentions that PATH_MAX can be used, so I don't understand the issue. If the issue is that you want to be able to compile libc++ on systems so old that they don't have PATH_MAX defined even though the documentation says it is provided, that doesn't sound compelling to me.

[zibi]

Neither.

The documentation Hubert pointed to clearly mentions that PATH_MAX can be used, so I don't understand the issue. If the issue is that you want to be able to compile libc++ on systems so old that they don't have PATH_MAX defined even though the documentation says it is provided, that doesn't sound compelling to me.

I'm sorry if the comments mislead you but the fact is that PATH_MAX is NOT provided on z/OS for the reasons I provided on Dec 9, 2020.
We have to use hard-coded value 1024 or _XOPEN_PATH_MAX macro. I discussed this issue internally the rational was provided before on Dec. 9.

[zibi]

rebase and resubmit to fix the build

[ldionne]

This is why i think that's okay (from the documentation you linked on Dec 9):

{PATH_MAX}
Maximum number of bytes the implementation will store as a pathname in a user-supplied buffer of unspecified size, including the terminating null character. Minimum number the implementation will accept as the maximum number of bytes in a pathname.
Minimum Acceptable Value: {_POSIX_PATH_MAX}

I still think it would be better to use realpath instead and bump your OS requirements (since we might want to drop the PATH_MAX codepath in libcxx eventually). But enough time has been wasted for such a small change.

libcxx/src/filesystem/operations.cpp

 # include <sys/time.h> // for gettimeofday and timeval
 #endif
 
 #if defined(__ELF__) && defined(_LIBCPP_LINK_RT_LIB)
 # pragma comment(lib, "rt")
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace {
 
 bool isSeparator(path::value_type C) {
   if (C == '/')
     return true;
 #if defined(_LIBCPP_WIN32API)
   if (C == '\\')
     return true;
path __canonical(path const& orig_p, error_code* ec) {
   path p = __do_absolute(orig_p, &cwd, ec);
 #if defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112
   std::unique_ptr<char, decltype(&::free)>
     hold(::realpath(p.c_str(), nullptr), &::free);
   if (hold.get() == nullptr)
     return err.report(capture_errno());
   return {hold.get()};
 #else
+  #if defined(__MVS__) && !defined(PATH_MAX)
+    char buff[ _XOPEN_PATH_MAX + 1 ];
+  #else
     char buff[PATH_MAX + 1];
+  #endif
   char* ret;
   if ((ret = ::realpath(p.c_str(), buff)) == nullptr)
     return err.report(capture_errno());
   return {ret};
 #endif
 }
 
 void __copy(const path& from, const path& to, copy_options options,
if (::chmod(p.c_str(), real_perms) == -1) {
     return err.report(capture_errno());
   }
 #endif
 }
 
 path __read_symlink(const path& p, error_code* ec) {
   ErrorHandler<path> err("read_symlink", ec, &p);
 
 #ifdef PATH_MAX
   struct NullDeleter { void operator()(void*) const {} };
   const size_t size = PATH_MAX + 1;
   char stack_buff[size];
   auto buff = std::unique_ptr<char[], NullDeleter>(stack_buff);
 #else
   StatT sb;
   if (::lstat(p.c_str(), &sb) == -1) {
     return err.report(capture_errno());