[libc] Add unistd overlay #118882
Merged
[libc] Add unistd overlay #118882
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Tristan Ross (RossComputerGuy) ChangesFixes failures like this which I experienced: Full diff: https://github.com/llvm/llvm-project/pull/118882.diff 7 Files Affected:
diff --git a/libc/hdr/unistd_macros.h b/libc/hdr/unistd_macros.h
index 132e123280139f..5c2b24354dd3ee 100644
--- a/libc/hdr/unistd_macros.h
+++ b/libc/hdr/unistd_macros.h
@@ -15,7 +15,7 @@
#else // Overlay mode
-#include <unistd.h>
+#include "unistd_overlay.h"
#endif // LLVM_LIBC_FULL_BUILD
diff --git a/libc/hdr/unistd_overlay.h b/libc/hdr/unistd_overlay.h
new file mode 100644
index 00000000000000..f2c9ed8e34ff60
--- /dev/null
+++ b/libc/hdr/unistd_overlay.h
@@ -0,0 +1,69 @@
+//===-- Including unistd.h in overlay mode ---------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_HDR_UNISTD_OVERLAY_H
+#define LLVM_LIBC_HDR_UNISTD_OVERLAY_H
+
+#ifdef LIBC_FULL_BUILD
+#error "This header should only be included in overlay mode"
+#endif
+
+// Overlay mode
+
+// glibc <unistd.h> header might provide extern inline definitions for few
+// functions, causing external alias errors. They are guarded by
+// `__USE_EXTERN_INLINES` macro. We temporarily disable `__USE_EXTERN_INLINES`
+// macro by defining `__NO_INLINE__` before including <stdio.h>.
+// And the same with `__USE_FORTIFY_LEVEL`, which will be temporarily disabled
+// with `_FORTIFY_SOURCE`.
+
+#ifdef _FORTIFY_SOURCE
+#define LIBC_OLD_FORTIFY_SOURCE _FORTIFY_SOURCE
+#undef _FORTIFY_SOURCE
+#endif
+
+#ifdef __USE_EXTERN_INLINES
+#define LIBC_OLD_USE_EXTERN_INLINES
+#undef __USE_EXTERN_INLINES
+#endif
+
+#ifdef __USE_FORTIFY_LEVEL
+#define LIBC_OLD_USE_FORTIFY_LEVEL __USE_FORTIFY_LEVEL
+#undef __USE_FORTIFY_LEVEL
+#define __USE_FORTIFY_LEVEL 0
+#endif
+
+#ifndef __NO_INLINE__
+#define __NO_INLINE__ 1
+#define LIBC_SET_NO_INLINE
+#endif
+
+#include <unistd.h>
+
+#ifdef LIBC_OLD_FORTIFY_SOURCE
+#define _FORTIFY_SOURCE LIBC_OLD_FORTIFY_SOURCE
+#undef LIBC_OLD_FORTIFY_SOURCE
+#endif
+
+#ifdef LIBC_SET_NO_INLINE
+#undef __NO_INLINE__
+#undef LIBC_SET_NO_INLINE
+#endif
+
+#ifdef LIBC_OLD_USE_FORTIFY_LEVEL
+#undef __USE_FORTIFY_LEVEL
+#define __USE_FORTIFY_LEVEL LIBC_OLD_USE_FORTIFY_LEVEL
+#undef LIBC_OLD_USE_FORTIFY_LEVEL
+#endif
+
+#ifdef LIBC_OLD_USE_EXTERN_INLINES
+#define __USE_EXTERN_INLINES
+#undef LIBC_OLD_USE_EXTERN_INLINES
+#endif
+
+#endif // LLVM_LIBC_HDR_UNISTD_OVERLAY_H
diff --git a/libc/src/unistd/getcwd.h b/libc/src/unistd/getcwd.h
index 8b63a91c26b5c7..8331944490b721 100644
--- a/libc/src/unistd/getcwd.h
+++ b/libc/src/unistd/getcwd.h
@@ -10,7 +10,7 @@
#define LLVM_LIBC_SRC_UNISTD_GETCWD_H
#include "src/__support/macros/config.h"
-#include <unistd.h>
+#include "hdr/unistd_macros.h"
namespace LIBC_NAMESPACE_DECL {
diff --git a/libc/src/unistd/pread.h b/libc/src/unistd/pread.h
index 4723675e82a20a..e0bdc471b12219 100644
--- a/libc/src/unistd/pread.h
+++ b/libc/src/unistd/pread.h
@@ -10,7 +10,7 @@
#define LLVM_LIBC_SRC_UNISTD_PREAD_H
#include "src/__support/macros/config.h"
-#include <unistd.h>
+#include "hdr/unistd_macros.h"
namespace LIBC_NAMESPACE_DECL {
diff --git a/libc/src/unistd/read.h b/libc/src/unistd/read.h
index 01231cb82e35e5..3d2ecd556d111e 100644
--- a/libc/src/unistd/read.h
+++ b/libc/src/unistd/read.h
@@ -10,7 +10,7 @@
#define LLVM_LIBC_SRC_UNISTD_READ_H
#include "src/__support/macros/config.h"
-#include <unistd.h>
+#include "hdr/unistd_macros.h"
namespace LIBC_NAMESPACE_DECL {
diff --git a/libc/src/unistd/readlink.h b/libc/src/unistd/readlink.h
index a73e9740c74637..562936150512a7 100644
--- a/libc/src/unistd/readlink.h
+++ b/libc/src/unistd/readlink.h
@@ -10,7 +10,7 @@
#define LLVM_LIBC_SRC_UNISTD_READLINK_H
#include "src/__support/macros/config.h"
-#include <unistd.h>
+#include "hdr/unistd_macros.h"
namespace LIBC_NAMESPACE_DECL {
diff --git a/libc/src/unistd/readlinkat.h b/libc/src/unistd/readlinkat.h
index 6bdd48b537fc8c..2e0e33f77586eb 100644
--- a/libc/src/unistd/readlinkat.h
+++ b/libc/src/unistd/readlinkat.h
@@ -10,7 +10,7 @@
#define LLVM_LIBC_SRC_UNISTD_READLINKAT_H
#include "src/__support/macros/config.h"
-#include <unistd.h>
+#include "hdr/unistd_macros.h"
namespace LIBC_NAMESPACE_DECL {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Can you also update |
RossComputerGuy
force-pushed
the
feat/llvm-libc-fixes-unistd
branch
from
8e2299c to
50ef7fb
Compare
|
Alright, I've added that in. |
libc/hdr/unistd_overlay.h
Outdated
Comment on lines
1
to
2
| //===-- Including unistd.h in overlay mode | ||
| //---------------------------------===// |
There was a problem hiding this comment.
nit: fix formatting
There was a problem hiding this comment.
Oh, what about the CI complaining about the bad formatting then lol
There was a problem hiding this comment.
Fixed it, I think we'll just ignore the format CI fail then lol.
| @@ -9,8 +9,8 @@ | |||
| #ifndef LLVM_LIBC_SRC_UNISTD_PREAD_H | |||
| #define LLVM_LIBC_SRC_UNISTD_PREAD_H | |||
|
|
|||
| #include "hdr/unistd_macros.h" | |||
There was a problem hiding this comment.
could you apply this to the other unistd functions as well?
RossComputerGuy
force-pushed
the
feat/llvm-libc-fixes-unistd
branch
2 times, most recently
from
d22d01c to
99cd546
Compare
libc/src/unistd/linux/lseek.cpp
Outdated
| @@ -14,8 +14,8 @@ | |||
| #include "src/__support/OSUtil/syscall.h" // For internal syscall function. | |||
| #include "src/__support/common.h" | |||
|
|
|||
| #include <sys/syscall.h> // For syscall numbers. | |||
| #include <unistd.h> // For off_t. | |||
| #include "hdr/unistd_macros.h" // For off_t. | |||
There was a problem hiding this comment.
if this is for off_t it may need the correct type header instead of unistd_macros.h
libc/src/unistd/swab.h
Outdated
| @@ -9,8 +9,8 @@ | |||
| #ifndef LLVM_LIBC_SRC_UNISTD_SWAB_H | |||
| #define LLVM_LIBC_SRC_UNISTD_SWAB_H | |||
|
|
|||
| #include "hdr/unistd_macros.h" // For ssize_t | |||
There was a problem hiding this comment.
this also needs the correct type header
RossComputerGuy
force-pushed
the
feat/llvm-libc-fixes-unistd
branch
2 times, most recently
from
28682ec to
e64ffcf
Compare
libc/src/unistd/linux/lseek.cpp
Outdated
| @@ -14,8 +14,8 @@ | |||
| #include "src/__support/OSUtil/syscall.h" // For internal syscall function. | |||
| #include "src/__support/common.h" | |||
|
|
|||
| #include <sys/syscall.h> // For syscall numbers. | |||
| #include <unistd.h> // For off_t. | |||
| #include "include/llvm-libc-types/off_t.h" // For off_t. | |||
There was a problem hiding this comment.
this should use the proxy header at hdr/types/off_t.h
Suggested change
| #include "include/llvm-libc-types/off_t.h" // For off_t. | |
| #include "hdr/types/off_t.h" |
libc/src/unistd/swab.h
Outdated
| @@ -9,8 +9,8 @@ | |||
| #ifndef LLVM_LIBC_SRC_UNISTD_SWAB_H | |||
| #define LLVM_LIBC_SRC_UNISTD_SWAB_H | |||
|
|
|||
| #include "include/llvm-libc-types/ssize_t.h" // For ssize_t | |||
There was a problem hiding this comment.
same here
Suggested change
| #include "include/llvm-libc-types/ssize_t.h" // For ssize_t | |
| #include "hdr/types/ssize_t.h" |
RossComputerGuy
force-pushed
the
feat/llvm-libc-fixes-unistd
branch
from
e64ffcf to
9f4c213
Compare
michaelrj-google
approved these changes
Dec 5, 2024
RossComputerGuy
force-pushed
the
feat/llvm-libc-fixes-unistd
branch
from
9f4c213 to
81b43fd
Compare
|
|
libc/hdr/unistd_overlay.h
Outdated
| @@ -0,0 +1,69 @@ | |||
| //===-- Including unistd.h in overlay mode ---------------------------------===// | |||
There was a problem hiding this comment.
nit: this line is too long
There was a problem hiding this comment.
How should we fix it since clang-format probably is complaining about other header files with header lines that are too long? Just letting clang-format do it's thing makes it look awkward.
There was a problem hiding this comment.
Remove - until clang-format doesn't try to wrap it.
Yeah, I don't have merge perms. |
RossComputerGuy
force-pushed
the
feat/llvm-libc-fixes-unistd
branch
from
81b43fd to
1c8f920
Compare
|
LGTM, I will merge once presubmits are done. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/71/builds/12229 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/171/builds/11978 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/179/builds/11959 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/12037 Here is the relevant piece of the build log for the reference |
|
Fun, I'll get to fixing these later tonight after work. |
michaelrj-google
added a commit
that referenced
this pull request
Dec 10, 2024
This reverts commit 7db970f.
michaelrj-google
added a commit
that referenced
this pull request
Dec 10, 2024
Reverts #118882 Several functions are now missing necessary types in fullbuild, e.g. `off_t`, `ssize_t`. Reverting for now.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/2140 Here is the relevant piece of the build log for the reference |
michaelrj-google
pushed a commit
that referenced
this pull request
Dec 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes failures like this which I experienced: