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

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

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D140078

libc/src/__support/CPP/CMakeLists.txt

@@ -100,3 +100,11 @@ add_header_library(
   HDRS
     expected.h
 )
+
+add_object_library(
+  new
+  SRCS
+    new.cpp
+  HDRS
+    new.h
+)

libc/src/__support/CPP/new.cpp

@@ -0,0 +1,30 @@
+//===-- 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/CPP/new.h

@@ -69,4 +69,29 @@ inline void *operator new[](size_t size, std::align_val_t align,
   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/File/CMakeLists.txt

@@ -24,6 +24,7 @@ add_object_library(
   HDRS
     dir.h
   DEPENDS
+    libc.src.__support.CPP.new
     libc.src.__support.CPP.span
     libc.src.__support.threads.mutex
 )
@@ -56,6 +57,6 @@ if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS}_dir.cpp)
       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.cpp

@@ -8,32 +8,38 @@
 
 #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.
@@ -48,10 +54,11 @@ struct ::dirent *Dir::read() {
 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;
 }
 

libc/src/__support/File/dir.h

@@ -10,6 +10,7 @@
 #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>
@@ -18,17 +19,17 @@
 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
@@ -46,20 +47,26 @@ class Dir {
 
   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; }

libc/src/__support/File/linux_dir.cpp

@@ -9,14 +9,14 @@
 #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);
@@ -28,29 +28,26 @@ int platform_opendir(const char *name) {
 #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

@@ -8,6 +8,7 @@ add_entrypoint_object(
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
+    libc.src.errno.errno
 )
 
 add_entrypoint_object(
@@ -32,6 +33,7 @@ add_entrypoint_object(
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
+    libc.src.errno.errno
 )
 
 add_entrypoint_object(
@@ -44,4 +46,5 @@ add_entrypoint_object(
     libc.include.dirent
     libc.src.__support.File.dir
     libc.src.__support.File.platform_dir
+    libc.src.errno.errno
 )

libc/src/dirent/closedir.cpp

@@ -12,12 +12,18 @@
 #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

@@ -12,11 +12,17 @@
 #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

@@ -12,12 +12,18 @@
 #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