[libc] Handle allocation failures in the dirent API gracefully.

[sivachandra]

Summary

Along the way, setting of errno has been moved out of the internal code.

[lntue]

libc/src/__support/CPP/CMakeLists.txt

105

Maybe change the name to new_and_delete or new_delete?

libc/src/__support/CPP/new.h

72

Why are these delete operators defined differently compared to new?

[sivachandra]

Add a comment explaining why operator delete is not implemented inline and why
we assign them special linkage names.

libc/src/__support/CPP/CMakeLists.txt

105

The target names here reflect the header file name. So, I am inclined to keep it as is for the sake of consistency.

libc/src/__support/CPP/new.h

72

Thanks for asking - I have added a comment now explaining that aspect.

[lntue]

libc/src/__support/CPP/CMakeLists.txt

105

Yes, that's actually what I meant, since in this patch, the header new.h defines both new and delete operators, and new.cpp only implements delete operators. It's probably better to rename both the header and the cpp files also.

[sivachandra]

libc/src/__support/CPP/CMakeLists.txt

105

Ah - we try to keep the header file names in sync with C++ standard header file names: https://eel.is/c++draft/new.syn

libc/src/__support/CPP/CMakeLists.txt

HDRS
     atomic.h
 )
 
 add_header_library(
   expected
   HDRS
     expected.h
 )
+
+add_object_library(
+  new
+  SRCS
+    new.cpp
+  HDRS
+    new.h
+)

libc/src/__support/CPP/new.h

inline void *operator new[](size_t size,
   return __llvm_libc::AllocChecker::alloc(size, ac);
 }
 
 inline void *operator new[](size_t size, std::align_val_t align,
                             __llvm_libc::AllocChecker &ac) noexcept {
   return __llvm_libc::AllocChecker::aligned_alloc(size, align, ac);
 }
 
+// The ideal situation would be to define the various flavors of operator delete
+// inline like we do with operator new above. However, since we need operator
+// delete prototypes to match those specified by the C++ standard, we cannot
+// define them inline as the C++ standard does not allow inline definitions of
+// replacement operator delete implementations. Note also that we assign a
+// special linkage name to each of these replacement operator delete functions.
+// This is because, if we do not give them a special libc internal linkage name,
+// they will replace operator delete for the entire application. Including this
+// header file in all libc source files where operator delete is called ensures
+// that only libc call sites use these replacement operator delete functions.
+void operator delete(void *) noexcept __asm__("__llvm_libc_delete");
+void operator delete(void *, std::align_val_t) noexcept
+    __asm__("__llvm_libc_delete_aligned");
+void operator delete(void *, size_t) noexcept
+    __asm__("__llvm_libc_delete_sized");
+void operator delete(void *, size_t, std::align_val_t) noexcept
+    __asm__("__llvm_libc_delete_sized_aligned");
+void operator delete[](void *) noexcept __asm__("__llvm_libc_delete_array");
+void operator delete[](void *, std::align_val_t) noexcept
+    __asm__("__llvm_libc_delete_array_aligned");
+void operator delete[](void *, size_t) noexcept
+    __asm__("__llvm_libc_delete_array_sized");
+void operator delete[](void *, size_t, std::align_val_t) noexcept
+    __asm__("__llvm_libc_delete_array_sized_aligned");
+
 #endif // LLVM_LIBC_SRC_SUPPORT_CPP_NEW_H

libc/src/__support/CPP/new.cpp

+//===-- Implementation of custom operator delete --------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "new.h"
+#include <stdlib.h>
+
+void operator delete(void *mem) noexcept { ::free(mem); }
+
+void operator delete(void *mem, std::align_val_t) noexcept { ::free(mem); }
+
+void operator delete(void *mem, size_t) noexcept { ::free(mem); }
+
+void operator delete(void *mem, size_t, std::align_val_t) noexcept {
+  ::free(mem);
+}
+
+void operator delete[](void *mem) noexcept { ::free(mem); }
+
+void operator delete[](void *mem, std::align_val_t) noexcept { ::free(mem); }
+
+void operator delete[](void *mem, size_t) noexcept { ::free(mem); }
+
+void operator delete[](void *mem, size_t, std::align_val_t) noexcept {
+  ::free(mem);
+}

libc/src/__support/File/CMakeLists.txt

 
 add_object_library(
   dir
   SRCS
     dir.cpp
   HDRS
     dir.h
   DEPENDS
+    libc.src.__support.CPP.new
     libc.src.__support.CPP.span
     libc.src.__support.threads.mutex
 )
 
 if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS}_file.cpp)
   add_object_library(
     platform_file
     SRCS
add_object_library(
     SRCS
       ${LIBC_TARGET_OS}_dir.cpp
     DEPENDS
       .dir
       libc.include.errno
       libc.include.fcntl
       libc.include.sys_syscall
       libc.src.__support.OSUtil.osutil
-      libc.src.errno.errno
+      libc.src.__support.error_or
   )
 endif()

libc/src/__support/File/dir.h

 //===--- A platform independent Dir class ---------------------------------===//
 //
 // 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 LLVM_LIBC_SRC_SUPPORT_FILE_DIR_H
 #define LLVM_LIBC_SRC_SUPPORT_FILE_DIR_H
 
 #include "src/__support/CPP/span.h"
+#include "src/__support/error_or.h"
 #include "src/__support/threads/mutex.h"
 
 #include <dirent.h>
 #include <stdlib.h>
 
 namespace __llvm_libc {
 
 // Platform specific function which will open the directory |name|
-// and return its file descriptor. Upon failure, this function sets the errno
-// value as suitable.
-int platform_opendir(const char *name);
+// and return its file descriptor. Upon failure, the error value is returned.
+ErrorOr<int> platform_opendir(const char *name);
 
 // Platform specific function which will close the directory with
-// file descriptor |fd|. Returns true on success, false on failure.
-bool platform_closedir(int fd);
+// file descriptor |fd|. Returns 0 on success, or the error number on failure.
+int platform_closedir(int fd);
 
 // Platform specific function which will fetch dirents in to buffer.
-// Returns the number of bytes written into buffer
-size_t platform_fetch_dirents(int fd, cpp::span<uint8_t> buffer);
+// Returns the number of bytes written into buffer or the error number on
+// failure.
+ErrorOr<size_t> platform_fetch_dirents(int fd, cpp::span<uint8_t> buffer);
 
 // This class is designed to allow implementation of the POSIX dirent.h API.
 // By itself, it is platform independent but calls platform specific
 // functions to perform OS operations.
 class Dir {
   static constexpr size_t BUFSIZE = 1024;
   int fd;
   size_t readptr = 0;  // The current read pointer.
   size_t fillsize = 0; // The number of valid bytes availabe in the buffer.
 
   // This is a buffer of struct dirent values which will be fetched
   // from the OS. Since the d_name of struct dirent can be of a variable
   // size, we store the data in a byte array.
   uint8_t buffer[BUFSIZE];
 
   Mutex mutex;
 
-public:
   // A directory is to be opened by the static method open and closed
   // by the close method. So, all constructors and destructor are declared
-  // as deleted.
+  // as private. Inappropriate constructors are declared as deleted.
   Dir() = delete;
   Dir(const Dir &) = delete;
-  ~Dir() = delete;
+
+  explicit Dir(int fdesc)
+      : fd(fdesc), readptr(0), fillsize(0), mutex(false, false, false) {}
+  ~Dir() = default;
 
   Dir &operator=(const Dir &) = delete;
 
-  static Dir *open(const char *path);
+public:
+  static ErrorOr<Dir *> open(const char *path);
 
-  struct ::dirent *read();
+  ErrorOr<struct ::dirent *> read();
 
+  // Returns 0 on success or the error number on failure. If an error number
+  // was returned, then the resources associated with the directory are not
+  // cleaned up.
   int close();
 
   int getfd() { return fd; }
 };
 
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_SUPPORT_FILE_DIR_H

libc/src/__support/File/dir.cpp

 //===--- Implementation of a platform independent Dir data structure ------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "dir.h"
 
+#include "src/__support/CPP/new.h"
+#include "src/__support/error_or.h"
+
+#include <errno.h>
 #include <stdlib.h>
 
 namespace __llvm_libc {
 
-Dir *Dir::open(const char *path) {
-  int fd = platform_opendir(path);
-  if (fd < 0)
-    return nullptr;
-
-  Dir *dir = reinterpret_cast<Dir *>(malloc(sizeof(Dir)));
-  dir->fd = fd;
-  dir->readptr = 0;
-  dir->fillsize = 0;
-  Mutex::init(&dir->mutex, false, false, false);
-
+ErrorOr<Dir *> Dir::open(const char *path) {
+  auto fd = platform_opendir(path);
+  if (!fd)
+    return __llvm_libc::Error(fd.error());
+
+  __llvm_libc::AllocChecker ac;
+  Dir *dir = new (ac) Dir(fd);
+  if (!ac)
+    return __llvm_libc::Error(ENOMEM);
   return dir;
 }
 
-struct ::dirent *Dir::read() {
+ErrorOr<struct ::dirent *> Dir::read() {
   MutexLock lock(&mutex);
   if (readptr >= fillsize) {
-    fillsize = platform_fetch_dirents(fd, buffer);
-    if (fillsize == 0)
-      return nullptr;
+    auto readsize = platform_fetch_dirents(fd, buffer);
+    if (!readsize)
+      return __llvm_libc::Error(readsize.error());
+    fillsize = readsize;
     readptr = 0;
   }
+  if (fillsize == 0)
+    return nullptr;
+
   struct ::dirent *d = reinterpret_cast<struct ::dirent *>(buffer + readptr);
 #ifdef __unix__
   // The d_reclen field is available on Linux but not required by POSIX.
   readptr += d->d_reclen;
 #else
   // Other platforms have to implement how the read pointer is to be updated.
 #error "DIR read pointer update is missing."
 #endif
   return d;
 }
 
 int Dir::close() {
   {
     MutexLock lock(&mutex);
-    if (!platform_closedir(fd))
-      return -1;
+    int retval = platform_closedir(fd);
+    if (retval != 0)
+      return retval;
   }
-  free(this);
+  delete this;
   return 0;
 }
 
 } // namespace __llvm_libc

libc/src/__support/File/linux_dir.cpp

 //===--- Linux implementation of the Dir helpers --------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "dir.h"
 
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/error_or.h"
 
-#include <errno.h>
 #include <fcntl.h>       // For open flags
 #include <sys/syscall.h> // For syscall numbers
 
 namespace __llvm_libc {
 
-int platform_opendir(const char *name) {
+ErrorOr<int> platform_opendir(const char *name) {
   int open_flags = O_RDONLY | O_DIRECTORY | O_CLOEXEC;
 #ifdef SYS_open
   int fd = __llvm_libc::syscall_impl(SYS_open, name, open_flags);
 #elif defined(SYS_openat)
   int fd = __llvm_libc::syscall_impl(SYS_openat, AT_FDCWD, name, open_flags);
 #else
 #error                                                                         \
     "SYS_open and SYS_openat syscalls not available to perform an open operation."
 #endif
 
   if (fd < 0) {
-    errno = -fd;
-    return -1;
+    return __llvm_libc::Error(-fd);
   }
   return fd;
 }
 
-size_t platform_fetch_dirents(int fd, cpp::span<uint8_t> buffer) {
+ErrorOr<size_t> platform_fetch_dirents(int fd, cpp::span<uint8_t> buffer) {
   long size =
       __llvm_libc::syscall_impl(SYS_getdents, fd, buffer.data(), buffer.size());
   if (size < 0) {
-    errno = -size;
-    return 0;
+    return __llvm_libc::Error(static_cast<int>(-size));
   }
   return size;
 }
 
-bool platform_closedir(int fd) {
+int platform_closedir(int fd) {
   long ret = __llvm_libc::syscall_impl(SYS_close, fd);
   if (ret < 0) {
-    errno = -ret;
-    return false;
+    return static_cast<int>(-ret);
   }
-  return true;
+  return 0;
 }
 
 } // namespace __llvm_libc

libc/src/dirent/CMakeLists.txt

 add_entrypoint_object(
   opendir
   SRCS
     opendir.cpp
   HDRS
     opendir.h
   DEPENDS
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
+    libc.src.errno.errno
 )
 
 add_entrypoint_object(
   dirfd
   SRCS
     dirfd.cpp
   HDRS
     dirfd.h
   DEPENDS
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
 )
 
 add_entrypoint_object(
   closedir
   SRCS
     closedir.cpp
   HDRS
     closedir.h
   DEPENDS
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
+    libc.src.errno.errno
 )
 
 add_entrypoint_object(
   readdir
   SRCS
     readdir.cpp
   HDRS
     readdir.h
   DEPENDS
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
+    libc.src.errno.errno
 )

libc/src/dirent/closedir.cpp

 //===-- Implementation of closedir ----------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "closedir.h"
 
 #include "src/__support/File/dir.h"
 #include "src/__support/common.h"
 
 #include <dirent.h>
+#include <errno.h>
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, closedir, (::DIR * dir)) {
   auto *d = reinterpret_cast<__llvm_libc::Dir *>(dir);
-  return d->close();
+  int retval = d->close();
+  if (retval != 0) {
+    errno = retval;
+    return -1;
+  }
+  return 0;
 }
 
 } // namespace __llvm_libc

libc/src/dirent/opendir.cpp

 //===-- Implementation of opendir -----------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "opendir.h"
 
 #include "src/__support/File/dir.h"
 #include "src/__support/common.h"
 
 #include <dirent.h>
+#include <errno.h>
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(::DIR *, opendir, (const char *name)) {
-  return reinterpret_cast<::DIR *>(Dir::open(name));
+  auto dir = Dir::open(name);
+  if (!dir) {
+    errno = dir.error();
+    return nullptr;
+  }
+  return reinterpret_cast<DIR *>(dir.value());
 }
 
 } // namespace __llvm_libc

libc/src/dirent/readdir.cpp

 //===-- Implementation of readdir -----------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "readdir.h"
 
 #include "src/__support/File/dir.h"
 #include "src/__support/common.h"
 
 #include <dirent.h>
+#include <errno.h>
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(struct ::dirent *, readdir, (::DIR * dir)) {
   auto *d = reinterpret_cast<__llvm_libc::Dir *>(dir);
-  return d->read();
+  auto dirent_val = d->read();
+  if (!dirent_val) {
+    errno = dirent_val.error();
+    return nullptr;
+  }
+  return dirent_val;
 }
 
 } // namespace __llvm_libc