-
Notifications
You must be signed in to change notification settings - Fork 12k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc] Update libc_errno to work correctly in both overlay and full build modes. #80177
Conversation
@llvm/pr-subscribers-libc Author: None (lntue) ChangesFull diff: https://github.com/llvm/llvm-project/pull/80177.diff 14 Files Affected:
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);
}
|
This was part of #79319 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has the effect of making the GPU errno
externally visible. I remember some discussions with Siva about that. I don't think it will be a huge issue, at least not righ right now. Though most of the GPU implementations of stdio
stuff intentionally ignored errno so it won't be consistent in any case. FWIW I think a global config to ignore errno might be valuable, even if it's against the standard.
No description provided.