[libc-commits] [libc] 9b29436 - [libc] Handle allocation failures in the dirent API gracefully.
Siva Chandra Reddy via libc-commits
libc-commits at lists.llvm.org
Thu Dec 15 13:05:49 PST 2022
Author: Siva Chandra Reddy
Date: 2022-12-15T21:05:40Z
New Revision: 9b29436a1c546bb16632d19e5a6f1edde3b6b2b2
URL: https://github.com/llvm/llvm-project/commit/9b29436a1c546bb16632d19e5a6f1edde3b6b2b2
DIFF: https://github.com/llvm/llvm-project/commit/9b29436a1c546bb16632d19e5a6f1edde3b6b2b2.diff
LOG: [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
Added:
libc/src/__support/CPP/new.cpp
Modified:
libc/src/__support/CPP/CMakeLists.txt
libc/src/__support/CPP/new.h
libc/src/__support/File/CMakeLists.txt
libc/src/__support/File/dir.cpp
libc/src/__support/File/dir.h
libc/src/__support/File/linux_dir.cpp
libc/src/dirent/CMakeLists.txt
libc/src/dirent/closedir.cpp
libc/src/dirent/opendir.cpp
libc/src/dirent/readdir.cpp
Removed:
################################################################################
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index d84492fd0195a..3bab78c4000cc 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/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
+)
diff --git a/libc/src/__support/CPP/new.cpp b/libc/src/__support/CPP/new.cpp
new file mode 100644
index 0000000000000..5a40d4a6d3b27
--- /dev/null
+++ b/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);
+}
diff --git a/libc/src/__support/CPP/new.h b/libc/src/__support/CPP/new.h
index 3becb9f26671f..7b23f0877c149 100644
--- a/libc/src/__support/CPP/new.h
+++ b/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
diff --git a/libc/src/__support/File/CMakeLists.txt b/libc/src/__support/File/CMakeLists.txt
index dd99e0df21105..ef56a767135bd 100644
--- a/libc/src/__support/File/CMakeLists.txt
+++ b/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()
diff --git a/libc/src/__support/File/dir.cpp b/libc/src/__support/File/dir.cpp
index bfd9b4cf446f6..a47370459b3ff 100644
--- a/libc/src/__support/File/dir.cpp
+++ b/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;
}
diff --git a/libc/src/__support/File/dir.h b/libc/src/__support/File/dir.h
index c5f3750f96884..57d31935551bc 100644
--- a/libc/src/__support/File/dir.h
+++ b/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; }
diff --git a/libc/src/__support/File/linux_dir.cpp b/libc/src/__support/File/linux_dir.cpp
index 82274e1fce94c..86aaaae907d22 100644
--- a/libc/src/__support/File/linux_dir.cpp
+++ b/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
diff --git a/libc/src/dirent/CMakeLists.txt b/libc/src/dirent/CMakeLists.txt
index e4227a63747ca..eb4184693a43c 100644
--- a/libc/src/dirent/CMakeLists.txt
+++ b/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
)
diff --git a/libc/src/dirent/closedir.cpp b/libc/src/dirent/closedir.cpp
index f4925db00c7a2..39dfc0ccc5f0c 100644
--- a/libc/src/dirent/closedir.cpp
+++ b/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
diff --git a/libc/src/dirent/opendir.cpp b/libc/src/dirent/opendir.cpp
index 0222944de4535..cf5ee18d3f050 100644
--- a/libc/src/dirent/opendir.cpp
+++ b/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
diff --git a/libc/src/dirent/readdir.cpp b/libc/src/dirent/readdir.cpp
index 968985061d5dc..b9ce30140ee0e 100644
--- a/libc/src/dirent/readdir.cpp
+++ b/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
More information about the libc-commits
mailing list