[libc-commits] [libc] [libc] improve error handling and compiler hints (PR #109026)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Tue Sep 17 11:42:15 PDT 2024
https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/109026
>From 3a1c128942dec20df1636c9b600f1976e3684850 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Tue, 17 Sep 2024 14:32:59 -0400
Subject: [PATCH 1/2] [libc] improve error handling and compiler hints
---
libc/src/__support/CPP/string.h | 6 +-
.../src/__support/OSUtil/linux/CMakeLists.txt | 1 +
libc/src/__support/OSUtil/linux/io.h | 59 +++++++-
libc/src/__support/libc_assert.h | 129 ++++++++++++------
4 files changed, 146 insertions(+), 49 deletions(-)
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index 64d4c276e84281..dfc78c0b3a935b 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -11,6 +11,7 @@
#include "src/__support/CPP/string_view.h"
#include "src/__support/integer_to_string.h" // IntegerToString
+#include "src/__support/libc_assert.h"
#include "src/__support/macros/config.h"
#include "src/string/memory_utils/inline_memcpy.h"
#include "src/string/memory_utils/inline_memset.h"
@@ -133,9 +134,8 @@ class string {
new_capacity)) {
buffer_ = static_cast<char *>(Ptr);
capacity_ = new_capacity;
- } else {
- __builtin_unreachable(); // out of memory
- }
+ } else
+ LIBC_CHECK_UNREACHABLE();
}
LIBC_INLINE void resize(size_t size) {
diff --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index 6c7014940407d8..95b474faf33d4b 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -22,6 +22,7 @@ add_object_library(
libc.hdr.types.struct_flock64
libc.hdr.types.struct_f_owner_ex
libc.hdr.types.off_t
+ libc.hdr.errno_macros
)
add_header_library(
diff --git a/libc/src/__support/OSUtil/linux/io.h b/libc/src/__support/OSUtil/linux/io.h
index 63e5b120d5c67e..684487a8f94763 100644
--- a/libc/src/__support/OSUtil/linux/io.h
+++ b/libc/src/__support/OSUtil/linux/io.h
@@ -9,17 +9,68 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_IO_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_IO_H
+#include "hdr/errno_macros.h"
#include "src/__support/CPP/string_view.h"
#include "src/__support/macros/config.h"
-#include "syscall.h" // For internal syscall function.
-
+#include "syscall.h" // For internal syscall function.
#include <sys/syscall.h> // For syscall numbers.
namespace LIBC_NAMESPACE_DECL {
LIBC_INLINE void write_to_stderr(cpp::string_view msg) {
- LIBC_NAMESPACE::syscall_impl<long>(SYS_write, 2 /* stderr */, msg.data(),
- msg.size());
+ size_t written = 0;
+ const char *msg_data = msg.data();
+ while (written != msg.size()) {
+ auto delta = LIBC_NAMESPACE::syscall_impl<long>(SYS_write, 2 /* stderr */,
+ msg_data, msg.size());
+ // If the write syscall was interrupted, try again. Otherwise, this is an
+ // error routine, we do not handle further errors.
+ if (delta == -EINTR)
+ continue;
+ if (delta < 0)
+ return;
+ written += delta;
+ msg_data += delta;
+ }
+}
+
+template <size_t N>
+LIBC_INLINE void write_all_to_stderr(const cpp::string_view (&msgs)[N]) {
+ struct IOVec {
+ const char *base;
+ size_t len;
+ } iovs[N];
+
+ size_t total_len = 0;
+ for (size_t i = 0; i < N; ++i) {
+ iovs[i].base = msgs[i].data();
+ iovs[i].len = msgs[i].size();
+ total_len += msgs[i].size();
+ }
+
+ size_t written = 0;
+ while (written != total_len) {
+ auto delta =
+ LIBC_NAMESPACE::syscall_impl<long>(SYS_writev, 2 /* stderr */, iovs, N);
+
+ // If the write syscall was interrupted, try again. Otherwise, this is an
+ // error routine, we do not handle further errors.
+ if (delta == -EINTR)
+ continue;
+ if (delta < 0)
+ return;
+
+ size_t udelta = delta;
+ written += udelta;
+ for (size_t i = 0; i < N; ++i) {
+ if (udelta == 0)
+ break;
+ auto change = udelta < iovs[i].len ? udelta : iovs[i].len;
+ udelta -= change;
+ iovs[i].base += change;
+ iovs[i].len -= change;
+ }
+ }
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/libc_assert.h b/libc/src/__support/libc_assert.h
index e21a58a0c8aad9..5110c3cc0351e2 100644
--- a/libc/src/__support/libc_assert.h
+++ b/libc/src/__support/libc_assert.h
@@ -9,43 +9,87 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_LIBC_ASSERT_H
#define LLVM_LIBC_SRC___SUPPORT_LIBC_ASSERT_H
-#include "src/__support/macros/config.h"
-#if defined(LIBC_COPT_USE_C_ASSERT) || !defined(LIBC_FULL_BUILD)
-
-// The build is configured to just use the public <assert.h> API
-// for libc's internal assertions.
-
-#include <assert.h>
-
-#define LIBC_ASSERT(COND) assert(COND)
-
-#else // Not LIBC_COPT_USE_C_ASSERT
-
+#include "src/__support/CPP/string_view.h"
#include "src/__support/OSUtil/exit.h"
#include "src/__support/OSUtil/io.h"
+#include "src/__support/OSUtil/linux/io.h"
#include "src/__support/integer_to_string.h"
#include "src/__support/macros/attributes.h" // For LIBC_INLINE
+#include "src/__support/macros/config.h"
namespace LIBC_NAMESPACE_DECL {
+LIBC_INLINE void report_assertion_failure(cpp::string_view assertion,
+ cpp::string_view filename,
+ cpp::string_view line,
+ cpp::string_view funcname) {
+ write_all_to_stderr({filename, ":", line, ": Assertion failed: '", assertion,
+ "' in function: '", funcname, "'\n"});
+}
-// This is intended to be removed in a future patch to use a similar design to
-// below, but it's necessary for the external assert.
-LIBC_INLINE void report_assertion_failure(const char *assertion,
- const char *filename, unsigned line,
- const char *funcname) {
+LIBC_INLINE void report_assertion_failure(cpp::string_view assertion,
+ cpp::string_view filename,
+ unsigned line,
+ cpp::string_view funcname) {
const IntegerToString<unsigned> line_buffer(line);
- write_to_stderr(filename);
- write_to_stderr(":");
- write_to_stderr(line_buffer.view());
- write_to_stderr(": Assertion failed: '");
- write_to_stderr(assertion);
- write_to_stderr("' in function: '");
- write_to_stderr(funcname);
- write_to_stderr("'\n");
+ report_assertion_failure(assertion, filename, line_buffer.view(), funcname);
+}
+
+// Direct calls to the unsafe_unreachable are highly discouraged.
+// Use the macro versions to provide at least a check in debug builds.
+LIBC_INLINE void unsafe_unreachable() {
+#if __has_builtin(__builtin_unreachable)
+ __builtin_unreachable();
+#endif
+}
+
+// Direct calls to the unsafe_assume are highly discouraged.
+// Use the macro versions to provide at least a check in debug builds.
+LIBC_INLINE void unsafe_assume([[maybe_unused]] bool cond) {
+#if __has_builtin(__builtin_assume)
+ __builtin_assume(cond);
+#elif __has_builtin(__builtin_unreachable)
+ if (!cond)
+ __builtin_unreachable();
+#endif
}
} // namespace LIBC_NAMESPACE_DECL
+// Macros:
+// - LIBC_ASSERT(COND): similar to `assert(COND)` but may use libc's internal
+// implementation.
+// - LIBC_CHECK(COND): similar to LIBC_ASSERT(COND) but will not be disabled in
+// release builds.
+// - LIBC_ASSUME(COND): LIBC_ASSERT + __builtin_assume.
+// - LIBC_UNREACHABLE(): similar to `__builtin_unreachable()` but checks in
+// debug mode.
+// - LIBC_CHECK_UNREACHABLE(): checks in both debug and release builds.
+
+// Convert __LINE__ to a string using macros. The indirection is necessary
+// because otherwise it will turn "__LINE__" into a string, not its value. The
+// value is evaluated in the indirection step.
+#define __LIBC_MACRO_TO_STR(x) #x
+#define __LIBC_MACRO_TO_STR_INDIR(y) __LIBC_MACRO_TO_STR(y)
+#define __LIBC_LINE_STR__ __LIBC_MACRO_TO_STR_INDIR(__LINE__)
+
+#define LIBC_CHECK(COND) \
+ do { \
+ if (!(COND)) { \
+ LIBC_NAMESPACE::report_assertion_failure( \
+ #COND, __FILE__, __LIBC_LINE_STR__, __PRETTY_FUNCTION__); \
+ LIBC_NAMESPACE::internal::exit(0xFF); \
+ } \
+ } while (false)
+
+#if defined(LIBC_COPT_USE_C_ASSERT) || !defined(LIBC_FULL_BUILD)
+// The build is configured to just use the public <assert.h> API
+// for libc's internal assertions.
+
+#include <assert.h>
+
+#define LIBC_ASSERT(COND) assert(COND)
+
+#else // Not LIBC_COPT_USE_C_ASSERT
#ifdef LIBC_ASSERT
#error "Unexpected: LIBC_ASSERT macro already defined"
#endif
@@ -61,27 +105,28 @@ LIBC_INLINE void report_assertion_failure(const char *assertion,
do { \
} while (false)
#else
+#define LIBC_ASSERT(COND) LIBC_CHECK(COND)
+#endif // NDEBUG
+#endif // LIBC_COPT_USE_C_ASSERT
-// Convert __LINE__ to a string using macros. The indirection is necessary
-// because otherwise it will turn "__LINE__" into a string, not its value. The
-// value is evaluated in the indirection step.
-#define __LIBC_MACRO_TO_STR(x) #x
-#define __LIBC_MACRO_TO_STR_INDIR(y) __LIBC_MACRO_TO_STR(y)
-#define __LIBC_LINE_STR__ __LIBC_MACRO_TO_STR_INDIR(__LINE__)
+#define LIBC_ASSUME(COND) \
+ do { \
+ LIBC_ASSERT(COND); \
+ LIBC_NAMESPACE::unsafe_assume(COND); \
+ } while (false)
-#define LIBC_ASSERT(COND) \
+#define LIBC_CHECK_UNREACHABLE() \
do { \
- if (!(COND)) { \
- LIBC_NAMESPACE::write_to_stderr(__FILE__ ":" __LIBC_LINE_STR__ \
- ": Assertion failed: '" #COND \
- "' in function: '"); \
- LIBC_NAMESPACE::write_to_stderr(__PRETTY_FUNCTION__); \
- LIBC_NAMESPACE::write_to_stderr("'\n"); \
- LIBC_NAMESPACE::internal::exit(0xFF); \
- } \
+ LIBC_NAMESPACE::report_assertion_failure("unreachable region", __FILE__, \
+ __LIBC_LINE_STR__, \
+ __PRETTY_FUNCTION__); \
+ LIBC_NAMESPACE::internal::exit(0xFF); \
} while (false)
-#endif // NDEBUG
-#endif // LIBC_COPT_USE_C_ASSERT
+#ifdef NDEBUG
+#define LIBC_UNREACHABLE() LIBC_NAMESPACE::unsafe_unreachable()
+#else
+#define LIBC_UNREACHABLE() LIBC_CHECK_UNREACHABLE()
+#endif // NDEBUG
#endif // LLVM_LIBC_SRC___SUPPORT_LIBC_ASSERT_H
>From 7ffc70e1622fd2a3c35e5aa0186f5c889e78d021 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Tue, 17 Sep 2024 14:42:01 -0400
Subject: [PATCH 2/2] change casting style
---
libc/src/__support/OSUtil/linux/io.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/src/__support/OSUtil/linux/io.h b/libc/src/__support/OSUtil/linux/io.h
index 684487a8f94763..ee9fd7c3ab1cfe 100644
--- a/libc/src/__support/OSUtil/linux/io.h
+++ b/libc/src/__support/OSUtil/linux/io.h
@@ -60,7 +60,7 @@ LIBC_INLINE void write_all_to_stderr(const cpp::string_view (&msgs)[N]) {
if (delta < 0)
return;
- size_t udelta = delta;
+ auto udelta = static_cast<size_t>(delta);
written += udelta;
for (size_t i = 0; i < N; ++i) {
if (udelta == 0)
More information about the libc-commits
mailing list