[libc][errno] Use macro instead of system header #91150

robincaloudis · 2024-05-05T22:26:00Z

Why

Currently, the system header errno.h is included in libc_errno.h, which is supposed to be consumed by internal implementations only. As unit and hermetic tests should never use #include <errno.h> but instead use #include "src/errno/libc_errno.h", we do not want to implicitly include errno.h. In order to have a clear seperation between those two, we want to pull out the definitions of errno numbers from errno.h.

What

Extract the definitions of errno numbers from include/errno.h.def and place it under include/llvm-libc-macros/linux/error-number-macros.h
- Provide mips-specific errno numbers in include/llvm-libc-macros/linux/mips/error-number-macros.h
  - Find definition of mips errno numbers in glibc here (equally defined in the Linux kernel)
- Provide sparc-specific errno numbers in include/llvm-libc-macros/linux/sparc/error-number-macros.h
  - Find definition of sparc errno numbers in glibc here (equally defined in the Linux kernel)
Include proxy header errno_macros.h instead of the system header errno.h in libc_errno.h/libc_errno.cpp

Closes #80172

llvmbot · 2024-05-12T10:24:02Z

@llvm/pr-subscribers-libc

Author: Robin Caloudis (robincaloudis)

Changes

Why

Currently, the system header errno.h is included in libc_errno.h, which is supposed to be consumed by internal implementations only. As unit and hermetic tests should never use #include <errno.h> but instead use #include "src/errno/libc_errno.h", we do not want to implicitly include errno.h. In order to have a clear seperation between those two, we want to pull out the definitions of errno numbers from errno.h.

What

Extract the definition of errno numbers from include/errno.h.def and place it under include/llvm-libc-macros/linux/error-number-macros.h
- Provide mips-specific errno numbers in include/llvm-libc-macros/linux/mips/error-number-macros.h
  - Find definition of mips errno numbers in glibc here (equally defined in the Linux kernel)
- Provide sparc-specific errno numbers in include/llvm-libc-macros/linux/sparc/error-number-macros.h
  - Find definition of sparc errno numbers in glibc here (equally defined in the Linux kernel)
Include error-number-macros.h instead of the system header errno.h in libc_errno.h

Closes #80172

Full diff: https://github.com/llvm/llvm-project/pull/91150.diff

12 Files Affected:

(modified) libc/include/errno.h.def (+1-19)
(modified) libc/include/llvm-libc-macros/CMakeLists.txt (+6)
(added) libc/include/llvm-libc-macros/error-number-macros.h (+8)
(modified) libc/include/llvm-libc-macros/generic-error-number-macros.h (+2)
(modified) libc/include/llvm-libc-macros/linux/CMakeLists.txt (+12)
(added) libc/include/llvm-libc-macros/linux/error-number-macros.h (+32)
(added) libc/include/llvm-libc-macros/linux/mips/CMakeLists.txt (+5)
(added) libc/include/llvm-libc-macros/linux/mips/error-number-macros.h (+24)
(added) libc/include/llvm-libc-macros/linux/sparc/CMakeLists.txt (+5)
(added) libc/include/llvm-libc-macros/linux/sparc/error-number-macros.h (+24)
(modified) libc/src/errno/CMakeLists.txt (+2)
(modified) libc/src/errno/libc_errno.h (+2-5)

diff --git a/libc/include/errno.h.def b/libc/include/errno.h.def
index d7ae90ad45247..3ffcd3fe4c721 100644
--- a/libc/include/errno.h.def
+++ b/libc/include/errno.h.def
@@ -15,29 +15,11 @@
 
 #include <linux/errno.h>
 
-#ifndef ERFKILL
-#define ERFKILL 132
-#endif // ERFKILL
-
-#ifndef EOWNERDEAD
-#define EOWNERDEAD 130
-#endif // EOWNERDEAD
-
-#ifndef EHWPOISON
-#define EHWPOISON 133
-#endif // EHWPOISON
-
-#ifndef ECANCELED
-#define ECANCELED 125
-#endif // ECANCELED
-
 #ifndef ENOTSUP
 #define ENOTSUP EOPNOTSUPP
 #endif // ENOTSUP
 
-#ifndef ENOTRECOVERABLE
-#define ENOTRECOVERABLE 131
-#endif // ENOTRECOVERABLE
+#include "llvm-libc-macros/linux/error-number-macros.h"
 
 #else // __linux__
 #include "llvm-libc-macros/generic-error-number-macros.h"
diff --git a/libc/include/llvm-libc-macros/CMakeLists.txt b/libc/include/llvm-libc-macros/CMakeLists.txt
index 68ba110aec80f..961830ef97668 100644
--- a/libc/include/llvm-libc-macros/CMakeLists.txt
+++ b/libc/include/llvm-libc-macros/CMakeLists.txt
@@ -37,6 +37,12 @@ add_macro_header(
     assert-macros.h
 )
 
+add_macro_header(
+  error_number_macros
+  HDR
+    error-number-macros.h
+)
+
 add_macro_header(
   generic_error_number_macros
   HDR
diff --git a/libc/include/llvm-libc-macros/error-number-macros.h b/libc/include/llvm-libc-macros/error-number-macros.h
new file mode 100644
index 0000000000000..29bd54d07f2e4
--- /dev/null
+++ b/libc/include/llvm-libc-macros/error-number-macros.h
@@ -0,0 +1,8 @@
+#ifndef LLVM_LIBC_MACROS_ERROR_NUMBER_MACROS_H
+#define LLVM_LIBC_MACROS_ERROR_NUMBER_MACROS_H
+
+#ifdef __linux__
+#include "linux/error-number-macros.h"
+#endif
+
+#endif // LLVM_LIBC_MACROS_ERROR_NUMBER_MACROS_H
diff --git a/libc/include/llvm-libc-macros/generic-error-number-macros.h b/libc/include/llvm-libc-macros/generic-error-number-macros.h
index 7ee0352669b8a..b5b1b676dacc3 100644
--- a/libc/include/llvm-libc-macros/generic-error-number-macros.h
+++ b/libc/include/llvm-libc-macros/generic-error-number-macros.h
@@ -44,5 +44,7 @@
 #define EDOM 33
 #define ERANGE 34
 #define EILSEQ 35
+#define ENAMETOOLONG 36
+#define EOVERFLOW 75
 
 #endif // LLVM_LIBC_MACROS_GENERIC_ERROR_NUMBER_MACROS_H
diff --git a/libc/include/llvm-libc-macros/linux/CMakeLists.txt b/libc/include/llvm-libc-macros/linux/CMakeLists.txt
index 4ee429d1db166..a07803103eefa 100644
--- a/libc/include/llvm-libc-macros/linux/CMakeLists.txt
+++ b/libc/include/llvm-libc-macros/linux/CMakeLists.txt
@@ -1,3 +1,15 @@
+add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/mips)
+add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/sparc)
+
+add_header(
+  error_number_macros
+  HDR
+    error-number-macros.h
+  DEPENDS
+    .mips.error_number_macros
+    .sparc.error_number_macros
+)
+
 add_header(
   fcntl_macros
   HDR
diff --git a/libc/include/llvm-libc-macros/linux/error-number-macros.h b/libc/include/llvm-libc-macros/linux/error-number-macros.h
new file mode 100644
index 0000000000000..4c8b3feb3dc39
--- /dev/null
+++ b/libc/include/llvm-libc-macros/linux/error-number-macros.h
@@ -0,0 +1,32 @@
+#ifndef LLVM_LIBC_MACROS_LINUX_ERROR_NUMBER_MACROS_H
+#define LLVM_LIBC_MACROS_LINUX_ERROR_NUMBER_MACROS_H
+
+#if defined(__mips__)
+#include "mips/error-number-macros.h"
+
+#elif defined(__sparc__)
+#include "sparc/error-number-macros.h"
+
+#else
+#ifndef ECANCELED
+#define ECANCELED 125
+#endif // ECANCELED
+
+#ifndef EOWNERDEAD
+#define EOWNERDEAD 130
+#endif // EOWNERDEAD
+
+#ifndef ENOTRECOVERABLE
+#define ENOTRECOVERABLE 131
+#endif // ENOTRECOVERABLE
+
+#ifndef ERFKILL
+#define ERFKILL 132
+#endif // ERFKILL
+
+#ifndef EHWPOISON
+#define EHWPOISON 133
+#endif // EHWPOISON
+#endif
+
+#endif // LLVM_LIBC_MACROS_LINUX_ERROR_NUMBER_MACROS_H
diff --git a/libc/include/llvm-libc-macros/linux/mips/CMakeLists.txt b/libc/include/llvm-libc-macros/linux/mips/CMakeLists.txt
new file mode 100644
index 0000000000000..eee4cfd193968
--- /dev/null
+++ b/libc/include/llvm-libc-macros/linux/mips/CMakeLists.txt
@@ -0,0 +1,5 @@
+add_header(
+  error_number_macros
+  HDR
+    error-number-macros.h
+)
diff --git a/libc/include/llvm-libc-macros/linux/mips/error-number-macros.h b/libc/include/llvm-libc-macros/linux/mips/error-number-macros.h
new file mode 100644
index 0000000000000..af2a4243e3cea
--- /dev/null
+++ b/libc/include/llvm-libc-macros/linux/mips/error-number-macros.h
@@ -0,0 +1,24 @@
+#ifndef LLVM_LIBC_MACROS_LINUX_MIPS_ERROR_NUMBER_MACROS_H
+#define LLVM_LIBC_MACROS_LINUX_MIPS_ERROR_NUMBER_MACROS_H
+
+#ifndef ECANCELED
+#define ECANCELED 158
+#endif // ECANCELED
+
+#ifndef EOWNERDEAD
+#define EOWNERDEAD 165
+#endif // EOWNERDEAD
+
+#ifndef ENOTRECOVERABLE
+#define ENOTRECOVERABLE 166
+#endif // ENOTRECOVERABLE
+
+#ifndef ERFKILL
+#define ERFKILL 167
+#endif // ERFKILL
+
+#ifndef EHWPOISON
+#define EHWPOISON 168
+#endif // EHWPOISON
+
+#endif // LLVM_LIBC_MACROS_LINUX_MIPS_ERROR_NUMBER_MACROS_H
diff --git a/libc/include/llvm-libc-macros/linux/sparc/CMakeLists.txt b/libc/include/llvm-libc-macros/linux/sparc/CMakeLists.txt
new file mode 100644
index 0000000000000..eee4cfd193968
--- /dev/null
+++ b/libc/include/llvm-libc-macros/linux/sparc/CMakeLists.txt
@@ -0,0 +1,5 @@
+add_header(
+  error_number_macros
+  HDR
+    error-number-macros.h
+)
diff --git a/libc/include/llvm-libc-macros/linux/sparc/error-number-macros.h b/libc/include/llvm-libc-macros/linux/sparc/error-number-macros.h
new file mode 100644
index 0000000000000..76a1408bf7601
--- /dev/null
+++ b/libc/include/llvm-libc-macros/linux/sparc/error-number-macros.h
@@ -0,0 +1,24 @@
+#ifndef LLVM_LIBC_MACROS_LINUX_SPARC_ERROR_NUMBER_MACROS_H
+#define LLVM_LIBC_MACROS_LINUX_SPARC_ERROR_NUMBER_MACROS_H
+
+#ifndef ECANCELED
+#define ECANCELED 127
+#endif // ECANCELED
+
+#ifndef EOWNERDEAD
+#define EOWNERDEAD 132
+#endif // EOWNERDEAD
+
+#ifndef ENOTRECOVERABLE
+#define ENOTRECOVERABLE 133
+#endif // ENOTRECOVERABLE
+
+#ifndef ERFKILL
+#define ERFKILL 134
+#endif // ERFKILL
+
+#ifndef EHWPOISON
+#define EHWPOISON 135
+#endif // EHWPOISON
+
+#endif // LLVM_LIBC_MACROS_LINUX_SPARC_ERROR_NUMBER_MACROS_H
diff --git a/libc/src/errno/CMakeLists.txt b/libc/src/errno/CMakeLists.txt
index d9b8d9957c170..5fc8a0263ea4a 100644
--- a/libc/src/errno/CMakeLists.txt
+++ b/libc/src/errno/CMakeLists.txt
@@ -19,5 +19,7 @@ add_entrypoint_object(
     ${full_build_flag}
   DEPENDS
     libc.include.errno
+    libc.include.llvm-libc-macros.error_number_macros
+    libc.include.llvm-libc-macros.generic_error_number_macros
     libc.src.__support.common
 )
diff --git a/libc/src/errno/libc_errno.h b/libc/src/errno/libc_errno.h
index 5afc0a41d348a..387a3fc31234e 100644
--- a/libc/src/errno/libc_errno.h
+++ b/libc/src/errno/libc_errno.h
@@ -12,11 +12,8 @@
 #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>
+#include <include/llvm-libc-macros/error-number-macros.h>
+#include <include/llvm-libc-macros/generic-error-number-macros.h>
 
 // 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

lntue

Thanks for fixing the errno header issue! Do you mind adding a proxy header libc/hdr/errno_macros.h to separate overlay and full build modes, similar to libc/hdr/fenv_macros.h?

lntue

Thanks for the update! Do you mind removing #include <errno.h> in https://github.com/llvm/llvm-project/blob/main/libc/src/errno/libc_errno.cpp#L40 also.

robincaloudis · 2024-05-13T20:38:06Z

Thanks for the update! Do you mind removing #include <errno.h> in https://github.com/llvm/llvm-project/blob/main/libc/src/errno/libc_errno.cpp#L40 also.

Good catch! Done.

lntue

Thanks! The patch looks good to me! Feel free to merge it once the buildkite finishes.

robincaloudis · 2024-05-13T21:53:18Z

Buildkite finished successfully. Do you mind merging this PR? I still lack write access. Thanks!

Patch llvm#91150 added a proxy header for errno macros. This patch fixes the bazel build since it needs to be added as a dependency.

Patch #91150 added a proxy header for errno macros. This patch fixes the bazel build since it needs to be added as a dependency.

Since #91150, a proxy header for the errno macros is available and gets included in `libc_errno.h` since then. As `libc_errno.cpp` includes `libc_errno.h`, which already includes the proxy header `hdr/errno_macros.h`, there's no need to include it in `libc_errno.cpp` if we are in overlay mode, because the proxy header takes care to either include our header from libc/include/ (fullbuild) or the corresponding underlying system header (overlay).

Introduced in #91150. Not needed anymore as #92041 fixed the root cause. `ENAMETOOLONG` and `EOVERFLOW` are well defined in `<linux/errno.h>`. Post mortem: Due to the previously missing inclusion of `<linux/errno.h>` (fixed with #92041), I misinterpreted an undefined macro issue during the development of #91150 as being caused by a missing definition rather than by the missing inclusion of the linux header. I realized too late that `ENAMETOOLONG` and `EOVERFLOW` were correctly defined in `<linux/errno.h>` and that it was my missing inclusion that caused the problem.

nickdesaulniers · 2024-05-20T17:53:40Z

err...we don't plan to support mips or sparc. We should not be adding headers for them. We aren't testing those platforms.

These are untested and unsupported platforms. The pattern used makes sense for platform specific error numbers, but these are platforms we do not support. Excise this code. Link: llvm#91150

These are untested and unsupported platforms. The pattern used makes sense for platform specific error numbers, but these are platforms we do not support. Excise this code. Link: #91150

robincaloudis force-pushed the rc-error-number-macros branch 3 times, most recently from 0b220af to 31c82ac Compare May 5, 2024 23:09

[libc][errno] Use macro instead of system header

b6105cc

robincaloudis force-pushed the rc-error-number-macros branch 2 times, most recently from ddcd096 to acf8a34 Compare May 5, 2024 23:40

Add additional error number

892714a

robincaloudis force-pushed the rc-error-number-macros branch from acf8a34 to 892714a Compare May 5, 2024 23:52

robincaloudis marked this pull request as ready for review May 12, 2024 10:23

llvmbot added the libc label May 12, 2024

lntue self-requested a review May 12, 2024 23:40

lntue requested changes May 13, 2024

View reviewed changes

lntue reviewed May 13, 2024

View reviewed changes

Extract errno macros proxy header

3184724

robincaloudis force-pushed the rc-error-number-macros branch from d282ef2 to 3184724 Compare May 13, 2024 20:36

lntue approved these changes May 13, 2024

View reviewed changes

lntue merged commit 561c42d into llvm:main May 13, 2024
4 checks passed

michaelrj-google mentioned this pull request May 13, 2024

[libc] add errno_macro header to bazel build #92044

Merged

michaelrj-google added a commit that referenced this pull request May 13, 2024

[libc] add errno_macro header to bazel build (#92044)

4c79d38

Patch #91150 added a proxy header for errno macros. This patch fixes the bazel build since it needs to be added as a dependency.

robincaloudis mentioned this pull request May 14, 2024

[libc][errno] Remove unnecessary include #92063

Merged

robincaloudis mentioned this pull request May 14, 2024

[libc][errno] Remove previously added errno numbers #92163

Merged

nickdesaulniers mentioned this pull request May 20, 2024

[libc][errno] remove mips+sparc specific errnos #92798

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc][errno] Use macro instead of system header #91150

[libc][errno] Use macro instead of system header #91150

robincaloudis commented May 5, 2024 •

edited

Loading

llvmbot commented May 12, 2024

Why

What

lntue left a comment

lntue left a comment

robincaloudis commented May 13, 2024

lntue left a comment

robincaloudis commented May 13, 2024

nickdesaulniers commented May 20, 2024

[libc][errno] Use macro instead of system header #91150

[libc][errno] Use macro instead of system header #91150

Conversation

robincaloudis commented May 5, 2024 • edited Loading

Why

What

llvmbot commented May 12, 2024

Why

What

lntue left a comment

Choose a reason for hiding this comment

lntue left a comment

Choose a reason for hiding this comment

robincaloudis commented May 13, 2024

lntue left a comment

Choose a reason for hiding this comment

robincaloudis commented May 13, 2024

nickdesaulniers commented May 20, 2024

robincaloudis commented May 5, 2024 •

edited

Loading