[17/N] [libcxx] Implement the read_symlink function for windows

[compnerd]

libcxx/src/filesystem/operations.cpp

1233

You should use backup semantics here to ensure that directories can be read.

[mstorsjo]

FWIW, after this patch, it should compile successfully for windows, the later patches only fix remaining behaviours.

libcxx/src/filesystem/operations.cpp

1233

This does use backup semantics; see https://reviews.llvm.org/D91141 for the implementation of the WinHandle helper. The flag FILE_FLAG_BACKUP_SEMANTICS is always included, to keep the calling code terse here.

[mstorsjo]

Rebased on top of updated patches.

[amccarth]

This looks like it'll work just fine, but I've added some inline questions for readability and robustness.

libcxx/src/filesystem/operations.cpp

1231

What's the purpose of the + sizeof(wchar_t) here?

1241

This seems like a reasonable time to use const auto *.

1243

I'm wondering whether it's worth doing some bounds checking on out and the offsets and lengths into PathBuffer. Is the data from DeviceIoControl trustworthy? Could somebody create a bad reparse point that would cause this code to access arbitrary data?

I think the answer is probably not.

Does libcxx have a "debug" version with assertions enabled?

1244

const auto &?

[mstorsjo]

libcxx/src/filesystem/operations.cpp

1231

Not entirely sure - I based it on this: https://github.com/microsoft/STL/blob/master/stl/inc/filesystem#L3198

However https://docs.microsoft.com/en-us/windows-hardware/drivers/ifs/fsctl-get-reparse-point says

For a REPARSE_DATA_BUFFER structure, this value must be at least REPARSE_DATA_BUFFER_HEADER_SIZE, plus the size of the expected user-defined data, and it must be less than or equal to MAXIMUM_REPARSE_DATA_BUFFER_SIZE.

So with that in mind, it actually seems incorrect to have it be any larger than MAXIMUM_REPARSE_DATA_BUFFER_SIZE. So I guess I should change it to that. But that use in MS STL seems curious...

Or is that stated from the perspective of the implementation of FSCTL_GET_REPARSE_POINT, saying that the function may only ever set/use up to MAXIMUM_REPARSE_DATA_BUFFER_SIZE bytes of output space, regardless of how large a buffer is allocated?

1241

Sure, will change that

1243

Good point - even in release mode, such checks should be cheap, and the resulting code isn't too bad either.

1244

Sure

[mstorsjo]

Updated according to suggestions.

[amccarth]

LGTM.

libcxx/src/filesystem/operations.cpp

1231

Yes, I think MAXIMUM_REPARSE_DATA_BUFFER_SIZE is the most it'll ever write to the buffer, no matter how large you say the buffer is. I think the corresponding FSCTL_SET_REPARSE_POINT will actually fail if you try to send too much data.

My guess is that the + sizeof(wchar_t) was so that a zero-terminator could be added should anyone every inadvertently treat the buffer as a string.

So I guess this all works, but I wisth

[mstorsjo]

Made a readlink helper in posix_compat.h.

[mstorsjo]

Rebased, retriggering CI.

Avoid using PATH_MAX (which exists in mingw but not in MSVC) and use MAXIMUM_REPARSE_DATA_BUFFER_SIZE as fixed limit for the statically allocated buffer that is passed to the readlink() function.

(When this was implemented directly in __read_symlink() before, the windows codepath didn't need to concern itself with other fixed limits but just do its own thing and return a std::filesystem::path, but using the same readlink() interface now to keep the implementation out of operations.cpp.)

[mstorsjo]

Rebased, added detail::SSizeT instead of ::ssize_t (for the readlink return value).

[ldionne]

LGTM pending CI.

libcxx/src/filesystem/posix_compat.h

44

Just saying, but it looks like those SDKs are pretty terrible to program against for lack of uniformity.

libcxx/src/filesystem/operations.cpp

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
+#if defined(PATH_MAX) || defined(MAX_SYMLINK_SIZE)
   struct NullDeleter { void operator()(void*) const {} };
+#ifdef MAX_SYMLINK_SIZE
+  const size_t size = MAX_SYMLINK_SIZE + 1;
+#else
   const size_t size = PATH_MAX + 1;
-  char stack_buff[size];
-  auto buff = std::unique_ptr<char[], NullDeleter>(stack_buff);
+#endif
+  path::value_type stack_buff[size];
+  auto buff = std::unique_ptr<path::value_type[], NullDeleter>(stack_buff);
 #else
   StatT sb;
   if (detail::lstat(p.c_str(), &sb) == -1) {
     return err.report(capture_errno());
   }
   const size_t size = sb.st_size + 1;
-  auto buff = unique_ptr<char[]>(new char[size]);
+  auto buff = unique_ptr<path::value_type[]>(new path::value_type[size]);
 #endif
-  ::ssize_t ret;
-  if ((ret = ::readlink(p.c_str(), buff.get(), size)) == -1)
+  detail::SSizeT ret;
+  if ((ret = detail::readlink(p.c_str(), buff.get(), size)) == -1)
     return err.report(capture_errno());
   _LIBCPP_ASSERT(ret > 0, "TODO");
   if (static_cast<size_t>(ret) >= size)
     return err.report(errc::value_too_large);
   buff[ret] = 0;
   return {buff.get()};
 }
 

libcxx/src/filesystem/posix_compat.h

 
 #include "filesystem_common.h"
 
 #if defined(_LIBCPP_WIN32API)
 # define WIN32_LEAN_AND_MEAN
 # define NOMINMAX
 # include <windows.h>
 # include <io.h>
+# include <winioctl.h>
 #else
 # include <unistd.h>
 # include <sys/stat.h>
 # include <sys/statvfs.h>
 #endif
 #include <time.h>
 
+#if defined(_LIBCPP_WIN32API)
+// This struct isn't defined in the normal Windows SDK, but only in the
+// Windows Driver Kit.
+struct LIBCPP_REPARSE_DATA_BUFFER {
+  unsigned long  ReparseTag;
+  unsigned short ReparseDataLength;
+  unsigned short Reserved;
+  union {
+    struct {
+      unsigned short SubstituteNameOffset;
+      unsigned short SubstituteNameLength;
+      unsigned short PrintNameOffset;
+      unsigned short PrintNameLength;
+      unsigned long  Flags;
+      wchar_t        PathBuffer[1];
+    } SymbolicLinkReparseBuffer;
+    struct {
+      unsigned short SubstituteNameOffset;
+      unsigned short SubstituteNameLength;
+      unsigned short PrintNameOffset;
+      unsigned short PrintNameLength;
+      wchar_t        PathBuffer[1];
+    } MountPointReparseBuffer;
+    struct {
+      unsigned char DataBuffer[1];
+    } GenericReparseBuffer;
+  };
+};
+#endif
+
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
 namespace {
 
 #if defined(_LIBCPP_WIN32API)
 
 // Various C runtime header sets provide more or less of these. As we
int fchmodat(int fd, const wchar_t *path, int perms, int flag) {
   return 0;
 }
 
 int fchmod(int fd, int perms) {
   HANDLE h = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   return fchmod_handle(h, perms);
 }
 
+#define MAX_SYMLINK_SIZE MAXIMUM_REPARSE_DATA_BUFFER_SIZE
+using SSizeT = ::int64_t;
+
+SSizeT readlink(const wchar_t *path, wchar_t *ret_buf, size_t bufsize) {
+  uint8_t buf[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
+  detail::WinHandle h(path, FILE_READ_ATTRIBUTES, FILE_FLAG_OPEN_REPARSE_POINT);
+  if (!h)
+    return set_errno();
+  DWORD out;
+  if (!DeviceIoControl(h, FSCTL_GET_REPARSE_POINT, nullptr, 0, buf, sizeof(buf),
+                       &out, 0))
+    return set_errno();
+  const auto *reparse = reinterpret_cast<LIBCPP_REPARSE_DATA_BUFFER *>(buf);
+  size_t path_buf_offset = offsetof(LIBCPP_REPARSE_DATA_BUFFER,
+                                    SymbolicLinkReparseBuffer.PathBuffer[0]);
+  if (out < path_buf_offset) {
+    errno = EINVAL;
+    return -1;
+  }
+  if (reparse->ReparseTag != IO_REPARSE_TAG_SYMLINK) {
+    errno = EINVAL;
+    return -1;
+  }
+  const auto &symlink = reparse->SymbolicLinkReparseBuffer;
+  unsigned short name_offset, name_length;
+  if (symlink.PrintNameLength == 0) {
+    name_offset = symlink.SubstituteNameOffset;
+    name_length = symlink.SubstituteNameLength;
+  } else {
+    name_offset = symlink.PrintNameOffset;
+    name_length = symlink.PrintNameLength;
+  }
+  // name_offset/length are expressed in bytes, not in wchar_t
+  if (path_buf_offset + name_offset + name_length > out) {
+    errno = EINVAL;
+    return -1;
+  }
+  if (name_length / sizeof(wchar_t) > bufsize) {
+    errno = ENOMEM;
+    return -1;
+  }
+  memcpy(ret_buf, &symlink.PathBuffer[name_offset / sizeof(wchar_t)],
+         name_length);
+  return name_length / sizeof(wchar_t);
+}
+
 #else
 int symlink_file(const char *oldname, const char *newname) {
   return ::symlink(oldname, newname);
 }
 int symlink_dir(const char *oldname, const char *newname) {
   return ::symlink(oldname, newname);
 }
 using ::chdir;
 using ::close;
 using ::fchmod;
 using ::fchmodat;
 using ::fstat;
 using ::ftruncate;
 using ::getcwd;
 using ::link;
 using ::lstat;
 using ::mkdir;
 using ::open;
+using ::readlink;
 using ::realpath;
 using ::remove;
 using ::rename;
 using ::stat;
 using ::statvfs;
 using ::truncate;
 
 #define O_BINARY 0
 
 using StatVFS = struct statvfs;
 using ModeT = ::mode_t;
+using SSizeT = ::ssize_t;
 
 #endif
 
 } // namespace
 } // end namespace detail
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 
 #endif // POSIX_COMPAT_H