[libc-commits] [libc] [libc] Update libc_errno to work correctly in both overlay and full build modes. (PR #80177)
via libc-commits
libc-commits at lists.llvm.org
Wed Jan 31 10:50:07 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: None (lntue)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/80177.diff
14 Files Affected:
- (modified) libc/include/errno.h.def (+8)
- (modified) libc/src/errno/CMakeLists.txt (+11)
- (modified) libc/src/errno/libc_errno.cpp (+34-26)
- (modified) libc/src/errno/libc_errno.h (+25-35)
- (modified) libc/test/integration/startup/linux/tls_test.cpp (+2-2)
- (modified) libc/test/src/errno/errno_test.cpp (+1-1)
- (modified) libc/test/src/stdlib/StrtolTest.h (+4-8)
- (modified) libc/test/src/sys/mman/linux/madvise_test.cpp (+1-1)
- (modified) libc/test/src/sys/mman/linux/mlock_test.cpp (+1-1)
- (modified) libc/test/src/sys/mman/linux/mmap_test.cpp (+1-1)
- (modified) libc/test/src/sys/mman/linux/mprotect_test.cpp (+1-1)
- (modified) libc/test/src/sys/mman/linux/posix_madvise_test.cpp (+1-1)
- (modified) libc/test/src/time/asctime_r_test.cpp (+3-3)
- (modified) libc/test/src/time/asctime_test.cpp (+6-6)
``````````diff
diff --git a/libc/include/errno.h.def b/libc/include/errno.h.def
index d8f79dd47a0d1..90bd8bfecf2f1 100644
--- a/libc/include/errno.h.def
+++ b/libc/include/errno.h.def
@@ -44,7 +44,15 @@
#endif
#if !defined(__AMDGPU__) && !defined(__NVPTX__)
+
+#ifdef __cplusplus
+extern "C" {
+extern thread_local int __llvmlibc_errno;
+}
+#else
extern _Thread_local int __llvmlibc_errno;
+#endif // __cplusplus
+
#define errno __llvmlibc_errno
#endif
diff --git a/libc/src/errno/CMakeLists.txt b/libc/src/errno/CMakeLists.txt
index e8868dc48c5e9..e19285809e245 100644
--- a/libc/src/errno/CMakeLists.txt
+++ b/libc/src/errno/CMakeLists.txt
@@ -1,9 +1,20 @@
+# If we are in full build mode, we will provide the errno definition ourselves,
+# and if we are in overlay mode, we will just re-use the system's errno.
+# We are passing LIBC_FULL_BUILD flag in full build mode so that the
+# implementation of libc_errno will know if we are in full build mode or not.
+set(full_build_flag "")
+if(LLVM_LIBC_FULL_BUILD)
+ set(full_build_flag "-DLIBC_FULL_BUILD")
+endif()
+
add_entrypoint_object(
errno
SRCS
libc_errno.cpp
HDRS
libc_errno.h # Include this
+ COMPILE_OPTIONS
+ ${full_build_flag}
DEPENDS
libc.include.errno
libc.src.__support.common
diff --git a/libc/src/errno/libc_errno.cpp b/libc/src/errno/libc_errno.cpp
index c8f0bffd0962e..e54bdd156d6bc 100644
--- a/libc/src/errno/libc_errno.cpp
+++ b/libc/src/errno/libc_errno.cpp
@@ -1,4 +1,4 @@
-//===-- Implementation of errno -------------------------------------------===//
+//===-- Implementation of libc_errno --------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,35 +6,43 @@
//
//===----------------------------------------------------------------------===//
-#include "src/__support/macros/attributes.h"
-#include "src/__support/macros/properties/architectures.h"
-
-namespace LIBC_NAMESPACE {
+#include "libc_errno.h"
#ifdef LIBC_TARGET_ARCH_IS_GPU
-struct ErrnoConsumer {
- void operator=(int) {}
-};
-#endif
+// LIBC_THREAD_LOCAL on GPU currently does nothing. So essentially this is just
+// a global errno for gpu to use for now.
+extern "C" {
+LIBC_THREAD_LOCAL int __llvmlibc_gpu_errno;
+}
+void LIBC_NAMESPACE::Errno::operator=(int a) { __llvmlibc_gpu_errno = a; }
+LIBC_NAMESPACE::Errno::operator int() { return __llvmlibc_gpu_errno; }
+
+#elif !defined(LIBC_COPT_PUBLIC_PACKAGING)
+// This mode is for unit testing. We just use our internal errno.
+LIBC_THREAD_LOCAL int __llvmlibc_internal_errno;
+
+void LIBC_NAMESPACE::Errno::operator=(int a) { __llvmlibc_internal_errno = a; }
+LIBC_NAMESPACE::Errno::operator int() { return __llvmlibc_internal_errno; }
+
+#elif defined(LIBC_FULL_BUILD)
+// This mode is for public libc archive, hermetic, and integration tests.
+// In full build mode, we provide the errno storage ourselves.
extern "C" {
-#ifdef LIBC_COPT_PUBLIC_PACKAGING
-// TODO: Declare __llvmlibc_errno only under LIBC_COPT_PUBLIC_PACKAGING and
-// __llvmlibc_internal_errno otherwise.
-// In overlay mode, this will be an unused thread local variable as libc_errno
-// will resolve to errno from the system libc's errno.h. In full build mode
-// however, libc_errno will resolve to this thread local variable via the errno
-// macro defined in LLVM libc's public errno.h header file.
-// TODO: Use a macro to distinguish full build and overlay build which can be
-// used to exclude __llvmlibc_errno under overlay build.
-#ifdef LIBC_TARGET_ARCH_IS_GPU
-ErrnoConsumer __llvmlibc_errno;
-#else
LIBC_THREAD_LOCAL int __llvmlibc_errno;
-#endif // LIBC_TARGET_ARCH_IS_GPU
+}
+
+void LIBC_NAMESPACE::Errno::operator=(int a) { __llvmlibc_errno = a; }
+LIBC_NAMESPACE::Errno::operator int() { return __llvmlibc_errno; }
+
#else
-LIBC_THREAD_LOCAL int __llvmlibc_internal_errno;
-#endif
-} // extern "C"
+// In overlay mode, we simply use the system errno.
+#include <errno.h>
+
+void LIBC_NAMESPACE::Errno::operator=(int a) { errno = a; }
+LIBC_NAMESPACE::Errno::operator int() { return errno; }
+
+#endif // LIBC_FULL_BUILD
-} // namespace LIBC_NAMESPACE
+// Define the global `libc_errno` instance.
+LIBC_NAMESPACE::Errno libc_errno;
diff --git a/libc/src/errno/libc_errno.h b/libc/src/errno/libc_errno.h
index fbcd1c3395cd1..632faeaaeff66 100644
--- a/libc/src/errno/libc_errno.h
+++ b/libc/src/errno/libc_errno.h
@@ -1,4 +1,4 @@
-//===-- Implementation header for errno -------------------------*- C++ -*-===//
+//===-- Implementation header for libc_errno --------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -12,45 +12,35 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/properties/architectures.h"
+// TODO: https://github.com/llvm/llvm-project/issues/80172
+// Separate just the definition of errno numbers in
+// include/llvm-libc-macros/* and only include that instead of the system
+// <errno.h>.
#include <errno.h>
-// If we are targeting the GPU we currently don't support 'errno'. We simply
-// consume it.
-#ifdef LIBC_TARGET_ARCH_IS_GPU
-namespace LIBC_NAMESPACE {
-struct ErrnoConsumer {
- void operator=(int) {}
-};
-} // namespace LIBC_NAMESPACE
-#endif
-
-// All of the libc runtime and test code should use the "libc_errno" macro. They
-// should not refer to the "errno" macro directly.
-#ifdef LIBC_COPT_PUBLIC_PACKAGING
-#ifdef LIBC_TARGET_ARCH_IS_GPU
-extern "C" LIBC_NAMESPACE::ErrnoConsumer __llvmlibc_errno;
-#define libc_errno __llvmlibc_errno
-#else
-// This macro will resolve to errno from the errno.h file included above. Under
-// full build, this will be LLVM libc's errno. In overlay build, it will be
-// system libc's errno.
-#define libc_errno errno
-#endif
-#else
-namespace LIBC_NAMESPACE {
+// This header is to be consumed by internal implementations, in which all of
+// them should refer to `libc_errno` instead of using `errno` directly from
+// <errno.h> header.
-// TODO: On the GPU build this will be mapped to a single global value. We need
-// to ensure that tests are not run with multiple threads that depend on errno
-// until we have true 'thread_local' support on the GPU.
-extern "C" LIBC_THREAD_LOCAL int __llvmlibc_internal_errno;
+// Unit and hermetic tests should:
+// - #include "src/errno/libc_errno.h"
+// - NOT #include <errno.h>
+// - Only use `libc_errno` in the code
+// - Depend on libc.src.errno.errno
-// TODO: After all of libc/src and libc/test are switched over to use
-// libc_errno, this header file will be "shipped" via an add_entrypoint_object
-// target. At which point libc_errno, should point to __llvmlibc_internal_errno
-// if LIBC_COPT_PUBLIC_PACKAGING is not defined.
-#define libc_errno LIBC_NAMESPACE::__llvmlibc_internal_errno
+// Integration tests should:
+// - NOT #include "src/errno/libc_errno.h"
+// - #include <errno.h>
+// - Use regular `errno` in the code
+// - Still depend on libc.src.errno.errno
+namespace LIBC_NAMESPACE {
+struct Errno {
+ void operator=(int);
+ operator int();
+};
} // namespace LIBC_NAMESPACE
-#endif
+
+extern LIBC_NAMESPACE::Errno libc_errno;
#endif // LLVM_LIBC_SRC_ERRNO_LIBC_ERRNO_H
diff --git a/libc/test/integration/startup/linux/tls_test.cpp b/libc/test/integration/startup/linux/tls_test.cpp
index 5f235a96006d6..cf2ff94931ba6 100644
--- a/libc/test/integration/startup/linux/tls_test.cpp
+++ b/libc/test/integration/startup/linux/tls_test.cpp
@@ -28,11 +28,11 @@ TEST_MAIN(int argc, char **argv, char **envp) {
// set in errno. Since errno is implemented using a thread
// local var, this helps us test setting of errno and
// reading it back.
- ASSERT_TRUE(libc_errno == 0);
+ ASSERT_ERRNO_SUCCESS();
void *addr = LIBC_NAMESPACE::mmap(nullptr, 0, PROT_READ,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
ASSERT_TRUE(addr == MAP_FAILED);
- ASSERT_TRUE(libc_errno == EINVAL);
+ ASSERT_ERRNO_SUCCESS();
return 0;
}
diff --git a/libc/test/src/errno/errno_test.cpp b/libc/test/src/errno/errno_test.cpp
index 33185c2bcf6f5..876ebfc0ac269 100644
--- a/libc/test/src/errno/errno_test.cpp
+++ b/libc/test/src/errno/errno_test.cpp
@@ -12,5 +12,5 @@
TEST(LlvmLibcErrnoTest, Basic) {
int test_val = 123;
libc_errno = test_val;
- ASSERT_EQ(test_val, libc_errno);
+ ASSERT_ERRNO_EQ(test_val);
}
diff --git a/libc/test/src/stdlib/StrtolTest.h b/libc/test/src/stdlib/StrtolTest.h
index 8f1723b038612..50ed4cca3950e 100644
--- a/libc/test/src/stdlib/StrtolTest.h
+++ b/libc/test/src/stdlib/StrtolTest.h
@@ -331,8 +331,7 @@ struct StrtoTest : public LIBC_NAMESPACE::testing::Test {
((is_signed_v<ReturnT> && sizeof(ReturnT) == 4)
? T_MAX
: ReturnT(0xFFFFFFFF)));
- ASSERT_EQ(libc_errno,
- is_signed_v<ReturnT> && sizeof(ReturnT) == 4 ? ERANGE : 0);
+ ASSERT_ERRNO_EQ(is_signed_v<ReturnT> && sizeof(ReturnT) == 4 ? ERANGE : 0);
EXPECT_EQ(str_end - max_32_bit_value, ptrdiff_t(10));
const char *negative_max_32_bit_value = "-0xFFFFFFFF";
@@ -341,8 +340,7 @@ struct StrtoTest : public LIBC_NAMESPACE::testing::Test {
((is_signed_v<ReturnT> && sizeof(ReturnT) == 4)
? T_MIN
: -ReturnT(0xFFFFFFFF)));
- ASSERT_EQ(libc_errno,
- is_signed_v<ReturnT> && sizeof(ReturnT) == 4 ? ERANGE : 0);
+ ASSERT_ERRNO_EQ(is_signed_v<ReturnT> && sizeof(ReturnT) == 4 ? ERANGE : 0);
EXPECT_EQ(str_end - negative_max_32_bit_value, ptrdiff_t(11));
// Max size for signed 32 bit numbers
@@ -368,8 +366,7 @@ struct StrtoTest : public LIBC_NAMESPACE::testing::Test {
(is_signed_v<ReturnT> || sizeof(ReturnT) < 8
? T_MAX
: ReturnT(0xFFFFFFFFFFFFFFFF)));
- ASSERT_EQ(libc_errno,
- (is_signed_v<ReturnT> || sizeof(ReturnT) < 8 ? ERANGE : 0));
+ ASSERT_ERRNO_EQ((is_signed_v<ReturnT> || sizeof(ReturnT) < 8 ? ERANGE : 0));
EXPECT_EQ(str_end - max_64_bit_value, ptrdiff_t(18));
// See the end of CleanBase10Decode for an explanation of how this large
@@ -381,8 +378,7 @@ struct StrtoTest : public LIBC_NAMESPACE::testing::Test {
(is_signed_v<ReturnT>
? T_MIN
: (sizeof(ReturnT) < 8 ? T_MAX : -ReturnT(0xFFFFFFFFFFFFFFFF))));
- ASSERT_EQ(libc_errno,
- (is_signed_v<ReturnT> || sizeof(ReturnT) < 8 ? ERANGE : 0));
+ ASSERT_ERRNO_EQ((is_signed_v<ReturnT> || sizeof(ReturnT) < 8 ? ERANGE : 0));
EXPECT_EQ(str_end - negative_max_64_bit_value, ptrdiff_t(19));
// Max size for signed 64 bit numbers
diff --git a/libc/test/src/sys/mman/linux/madvise_test.cpp b/libc/test/src/sys/mman/linux/madvise_test.cpp
index 83c73f5454de1..e45cf19f8913e 100644
--- a/libc/test/src/sys/mman/linux/madvise_test.cpp
+++ b/libc/test/src/sys/mman/linux/madvise_test.cpp
@@ -23,7 +23,7 @@ TEST(LlvmLibcMadviseTest, NoError) {
libc_errno = 0;
void *addr = LIBC_NAMESPACE::mmap(nullptr, alloc_size, PROT_READ,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- EXPECT_EQ(0, libc_errno);
+ ASSERT_ERRNO_SUCCESS();
EXPECT_NE(addr, MAP_FAILED);
EXPECT_THAT(LIBC_NAMESPACE::madvise(addr, alloc_size, MADV_RANDOM),
diff --git a/libc/test/src/sys/mman/linux/mlock_test.cpp b/libc/test/src/sys/mman/linux/mlock_test.cpp
index a4e1682ff32bc..f1d1af1e76920 100644
--- a/libc/test/src/sys/mman/linux/mlock_test.cpp
+++ b/libc/test/src/sys/mman/linux/mlock_test.cpp
@@ -123,7 +123,7 @@ TEST(LlvmLibcMlockTest, InvalidFlag) {
libc_errno = 0;
void *addr = LIBC_NAMESPACE::mmap(nullptr, alloc_size, PROT_READ,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- EXPECT_EQ(0, libc_errno);
+ ASSERT_ERRNO_SUCCESS();
EXPECT_NE(addr, MAP_FAILED);
// Invalid mlock2 flags.
diff --git a/libc/test/src/sys/mman/linux/mmap_test.cpp b/libc/test/src/sys/mman/linux/mmap_test.cpp
index 9b13b8bd8057f..b996f26db8605 100644
--- a/libc/test/src/sys/mman/linux/mmap_test.cpp
+++ b/libc/test/src/sys/mman/linux/mmap_test.cpp
@@ -22,7 +22,7 @@ TEST(LlvmLibcMMapTest, NoError) {
libc_errno = 0;
void *addr = LIBC_NAMESPACE::mmap(nullptr, alloc_size, PROT_READ,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- EXPECT_EQ(0, libc_errno);
+ ASSERT_ERRNO_SUCCESS();
EXPECT_NE(addr, MAP_FAILED);
int *array = reinterpret_cast<int *>(addr);
diff --git a/libc/test/src/sys/mman/linux/mprotect_test.cpp b/libc/test/src/sys/mman/linux/mprotect_test.cpp
index 7127f77714d64..96f625984101d 100644
--- a/libc/test/src/sys/mman/linux/mprotect_test.cpp
+++ b/libc/test/src/sys/mman/linux/mprotect_test.cpp
@@ -24,7 +24,7 @@ TEST(LlvmLibcMProtectTest, NoError) {
libc_errno = 0;
void *addr = LIBC_NAMESPACE::mmap(nullptr, alloc_size, PROT_READ,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- EXPECT_EQ(0, libc_errno);
+ ASSERT_ERRNO_SUCCESS();
EXPECT_NE(addr, MAP_FAILED);
int *array = reinterpret_cast<int *>(addr);
diff --git a/libc/test/src/sys/mman/linux/posix_madvise_test.cpp b/libc/test/src/sys/mman/linux/posix_madvise_test.cpp
index 59cf01ac74695..d20db69042b7a 100644
--- a/libc/test/src/sys/mman/linux/posix_madvise_test.cpp
+++ b/libc/test/src/sys/mman/linux/posix_madvise_test.cpp
@@ -23,7 +23,7 @@ TEST(LlvmLibcPosixMadviseTest, NoError) {
libc_errno = 0;
void *addr = LIBC_NAMESPACE::mmap(nullptr, alloc_size, PROT_READ,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- EXPECT_EQ(0, libc_errno);
+ ASSERT_ERRNO_SUCCESS();
EXPECT_NE(addr, MAP_FAILED);
EXPECT_EQ(LIBC_NAMESPACE::posix_madvise(addr, alloc_size, POSIX_MADV_RANDOM),
diff --git a/libc/test/src/time/asctime_r_test.cpp b/libc/test/src/time/asctime_r_test.cpp
index 1abaa135350c1..f3aadbb39de4d 100644
--- a/libc/test/src/time/asctime_r_test.cpp
+++ b/libc/test/src/time/asctime_r_test.cpp
@@ -27,17 +27,17 @@ static inline char *call_asctime_r(struct tm *tm_data, int year, int month,
TEST(LlvmLibcAsctimeR, Nullptr) {
char *result;
result = LIBC_NAMESPACE::asctime_r(nullptr, nullptr);
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
ASSERT_STREQ(nullptr, result);
char buffer[TimeConstants::ASCTIME_BUFFER_SIZE];
result = LIBC_NAMESPACE::asctime_r(nullptr, buffer);
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
ASSERT_STREQ(nullptr, result);
struct tm tm_data;
result = LIBC_NAMESPACE::asctime_r(&tm_data, nullptr);
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
ASSERT_STREQ(nullptr, result);
}
diff --git a/libc/test/src/time/asctime_test.cpp b/libc/test/src/time/asctime_test.cpp
index 4b5ceb596aa46..169a7463a3037 100644
--- a/libc/test/src/time/asctime_test.cpp
+++ b/libc/test/src/time/asctime_test.cpp
@@ -22,7 +22,7 @@ static inline char *call_asctime(struct tm *tm_data, int year, int month,
TEST(LlvmLibcAsctime, Nullptr) {
char *result;
result = LIBC_NAMESPACE::asctime(nullptr);
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
ASSERT_STREQ(nullptr, result);
}
@@ -40,7 +40,7 @@ TEST(LlvmLibcAsctime, InvalidWday) {
0, // sec
-1, // wday
0); // yday
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
// Test with wday = 7.
call_asctime(&tm_data,
@@ -52,7 +52,7 @@ TEST(LlvmLibcAsctime, InvalidWday) {
0, // sec
7, // wday
0); // yday
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
}
// Months are from January to December. Test passing invalid value in month.
@@ -69,7 +69,7 @@ TEST(LlvmLibcAsctime, InvalidMonth) {
0, // sec
4, // wday
0); // yday
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
// Test with month = 13.
call_asctime(&tm_data,
@@ -81,7 +81,7 @@ TEST(LlvmLibcAsctime, InvalidMonth) {
0, // sec
4, // wday
0); // yday
- ASSERT_EQ(EINVAL, libc_errno);
+ ASSERT_ERRNO_EQ(EINVAL);
}
TEST(LlvmLibcAsctime, ValidWeekdays) {
@@ -209,6 +209,6 @@ TEST(LlvmLibcAsctime, Max64BitYear) {
50, // sec
2, // wday
50); // yday
- ASSERT_EQ(EOVERFLOW, libc_errno);
+ ASSERT_ERRNO_EQ(EOVERFLOW);
ASSERT_STREQ(nullptr, result);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/80177
More information about the libc-commits
mailing list