-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[libc] implement pathconf/fpathconf #87165
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
Conversation
@llvm/pr-subscribers-libc Author: Nhat Nguyen (changkhothuychung) ChangesFull diff: https://github.com/llvm/llvm-project/pull/87165.diff 7 Files Affected:
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 78da7f0b334b1f..be0c4bebfd95ad 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -282,6 +282,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.dup3
libc.src.unistd.execve
libc.src.unistd.fchdir
+ libc.src.unistd.fpathconf
libc.src.unistd.fsync
libc.src.unistd.ftruncate
libc.src.unistd.getcwd
@@ -293,6 +294,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.link
libc.src.unistd.linkat
libc.src.unistd.lseek
+ libc.src.unistd.pathconf
libc.src.unistd.pread
libc.src.unistd.pwrite
libc.src.unistd.read
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 5aae4e246cfb3c..2d8f9b1f6dd82c 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -287,6 +287,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.dup3
libc.src.unistd.execve
libc.src.unistd.fchdir
+ libc.src.unistd.fpathconf
libc.src.unistd.fsync
libc.src.unistd.ftruncate
libc.src.unistd.getcwd
@@ -298,6 +299,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.link
libc.src.unistd.linkat
libc.src.unistd.lseek
+ libc.src.unistd.pathconf
libc.src.unistd.pread
libc.src.unistd.pwrite
libc.src.unistd.read
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 5b428e51aee620..0f00f264ea1490 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -297,6 +297,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.dup3
libc.src.unistd.execve
libc.src.unistd.fchdir
+ libc.src.unistd.fpathconf
libc.src.unistd.fsync
libc.src.unistd.ftruncate
libc.src.unistd.getcwd
@@ -308,6 +309,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.link
libc.src.unistd.linkat
libc.src.unistd.lseek
+ libc.src.unistd.pathconf
libc.src.unistd.pread
libc.src.unistd.pwrite
libc.src.unistd.read
diff --git a/libc/src/unistd/CMakeLists.txt b/libc/src/unistd/CMakeLists.txt
index e22b0e1872caa1..20c87f668b381b 100644
--- a/libc/src/unistd/CMakeLists.txt
+++ b/libc/src/unistd/CMakeLists.txt
@@ -58,6 +58,13 @@ add_entrypoint_object(
.${LIBC_TARGET_OS}.fork
)
+add_entrypoint_object(
+ fpathconf
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.fpathconf
+)
+
add_entrypoint_object(
execv
ALIAS
@@ -149,6 +156,13 @@ add_entrypoint_object(
.${LIBC_TARGET_OS}.lseek
)
+add_entrypoint_object(
+ pathconf
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.pathconf
+)
+
add_entrypoint_object(
pread
ALIAS
diff --git a/libc/src/unistd/linux/CMakeLists.txt b/libc/src/unistd/linux/CMakeLists.txt
index df85d44e9e9edc..0ee59d5391ac5e 100644
--- a/libc/src/unistd/linux/CMakeLists.txt
+++ b/libc/src/unistd/linux/CMakeLists.txt
@@ -105,6 +105,19 @@ add_entrypoint_object(
libc.src.errno.errno
)
+add_entrypoint_object(
+ fpathconf
+ SRCS
+ fpathconf.cpp
+ HDRS
+ ../fpathconf.h
+ DEPENDS
+ libc.include.unistd
+ libc.include.sys_syscall
+ libc.src.__support.OSUtil.osutil
+ libc.src.errno.errno
+)
+
add_entrypoint_object(
execv
SRCS
@@ -273,6 +286,19 @@ add_entrypoint_object(
libc.src.errno.errno
)
+add_entrypoint_object(
+ pathconf
+ SRCS
+ pathconf.cpp
+ HDRS
+ ../pathconf.h
+ DEPENDS
+ libc.include.unistd
+ libc.include.sys_syscall
+ libc.src.__support.OSUtil.osutil
+ libc.src.errno.errno
+)
+
add_entrypoint_object(
pread
SRCS
diff --git a/libc/src/unistd/linux/pathconf.cpp b/libc/src/unistd/linux/pathconf.cpp
new file mode 100644
index 00000000000000..fed94e7d423716
--- /dev/null
+++ b/libc/src/unistd/linux/pathconf.cpp
@@ -0,0 +1,68 @@
+//===-- Linux implementation of pathconf ----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/unistd/pread.h"
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+#include "src/__support/macros/sanitizer.h" // for MSAN_UNPOISON
+#include "src/errno/libc_errno.h"
+#include <stdint.h> // For uint64_t.
+#include <sys/statvfs.h>
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE {
+
+static long pathconfig(const struct statvfs &s, int name) {
+ switch (name) {
+ case _PC_LINK_MAX:
+ return _POSIX_LINK_MAX;
+
+ case _PC_MAX_CANON:
+ return _POSIX_MAX_CANON;
+
+ case _PC_MAX_INPUT:
+ return _POSIX_MAX_INPUT;
+
+ case _PC_NAME_MAX:
+ return _POSIX_NAME_MAX;
+
+ case _PC_PATH_MAX:
+ return _POSIX_PATH_MAX;
+
+ case _PC_PIPE_BUF:
+ return _POSIX_PIPE_BUF;
+
+ case _PC_CHOWN_RESTRICTED:
+ return _POSIX_CHOWN_RESTRICTED;
+
+ case _PC_NO_TRUNC:
+ return _POSIX_NO_TRUNC;
+
+ case _PC_VDISABLE:
+ return _POSIX_VDISABLE;
+ }
+}
+
+LLVM_LIBC_FUNCTION(long, pathconf, (char *path, int name)) {
+ struct statvfs sb;
+ if (statvfs(path, &sb) == -1) {
+ return -1;
+ }
+ return pathconfig(sb, name);
+}
+
+LLVM_LIBC_FUNCTION(long, fpathconf, (int fd, int name)) {
+ struct statvfs sb;
+ if (statvfs(path, &sb) == -1) {
+ return -1;
+ }
+ return pathconfig(sb, name);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/unistd/pathconf.h b/libc/src/unistd/pathconf.h
new file mode 100644
index 00000000000000..a397cd9323bbb8
--- /dev/null
+++ b/libc/src/unistd/pathconf.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for pathconf ----------------------*- C++ -*-===//
+//
+// 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_SRC_UNISTD_PREAD_H
+#define LLVM_LIBC_SRC_UNISTD_PREAD_H
+
+#include <unistd.h>
+
+namespace LIBC_NAMESPACE {
+
+long pathconf(char *path, int name);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_UNISTD_PREAD_H
|
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.
Thanks for the patch!
It needs tests.
any update on this PR? I can PR my preferred changes to your branch if you would like to have them. |
sorry I was busy the past days, will resume on this one soon. But if this is in any urgency, please feel free to add your change . |
@SchrodingerZhu can you re-trigger the CI manually by any chance? my pushes dont seem to re-trigger the CI |
This is something clang-tidy should be able to catch. Maybe you can rerun your cmake with |
@changkhothuychung locally under full build, there is another error (which I believe should be my fault).
Other than that everything looks good to me now. A small patch for the fix: From 66b281bc70410b7359c64dd2f2600f5878c51d6b Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i@zhuyi.fan>
Date: Mon, 17 Jun 2024 22:49:46 -0700
Subject: [PATCH] fix build issue
---
libc/src/sys/statvfs/linux/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/libc/src/sys/statvfs/linux/CMakeLists.txt b/libc/src/sys/statvfs/linux/CMakeLists.txt
index f818863bb470..a6660c02badf 100644
--- a/libc/src/sys/statvfs/linux/CMakeLists.txt
+++ b/libc/src/sys/statvfs/linux/CMakeLists.txt
@@ -8,6 +8,7 @@ add_header_library(
libc.src.__support.common
libc.src.__support.CPP.optional
libc.include.sys_syscall
+ libc.include.llvm-libc-types.struct_statvfs
)
add_entrypoint_object(
--
2.45.2
|
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.
LGTM.
@@ -10,6 +10,7 @@ | |||
#include "hdr/limits_macros.h" | |||
#include "hdr/sys_stat_macros.h" | |||
#include "hdr/unistd_macros.h" | |||
#include "llvm-libc-macros/sys-stat-macros.h" |
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.
Do you have to add this to pass the compilation?
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.
Yes, I remember not having it will cause CI error. Do you wanna double check?
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.
Also, can you help me merge when you see everything is ok? thanks.
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.
You already have hdr/sys_stat_macros.h
so you should not include llvm-libc-macros/sys-stat-macros.h
.
Then you should also remove libc.include.llvm-libc-macros.sys-stat-macros
from the dependency list.
hdr/sys_stat_macros.h
is called proxy header. Once you have that, you should not use the internal llvm-libc-macros/sys-stat-macros.h
Ok. I see this error. I will be investigating it. |
I reproduced the error locally. This is very interesting. The LINK_MAX is right there in the header. |
Please consider apply this patch to fix the build issue: From 18c25535427ed01e30379d0209706e447515978c Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i@zhuyi.fan>
Date: Sat, 6 Jul 2024 16:18:13 -0700
Subject: [PATCH] fix
---
libc/src/unistd/linux/pathconf_utils.cpp | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/libc/src/unistd/linux/pathconf_utils.cpp b/libc/src/unistd/linux/pathconf_utils.cpp
index 5112d2e907cc..c764b7f20a74 100644
--- a/libc/src/unistd/linux/pathconf_utils.cpp
+++ b/libc/src/unistd/linux/pathconf_utils.cpp
@@ -6,12 +6,18 @@
//
//===----------------------------------------------------------------------===//
+// This header must go before limits_macros.h otherwise libc header may choose
+// to undefine LINK_MAX.
+#include <linux/limits.h> // For LINK_MAX and other limits
+
#include "hdr/limits_macros.h"
#include "hdr/unistd_macros.h"
#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
#include "src/__support/common.h"
#include "src/errno/libc_errno.h"
#include "src/sys/statvfs/linux/statfs_utils.h"
+
+// other linux specific includes
#include <linux/bfs_fs.h>
#if __has_include(<linux/ufs_fs.h>)
#include <linux/ufs_fs.h>
@@ -19,8 +25,7 @@
// from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
#define UFS_MAGIC 0x00011954
#endif
-#include <linux/limits.h> // For LINK_MAX and other limits
-#include <linux/magic.h> // For common FS magics
+#include <linux/magic.h> // For common FS magics
namespace LIBC_NAMESPACE {
--
2.45.2
|
Thank you for your patch! |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/2201 Here is the relevant piece of the build log for the reference:
|
This does not seem to be related to the changes inside this patch. |
folks, this patch broke rv32 compilation: https://lab.llvm.org/staging/#/builders/149/builds/8399
May I ask if you can take a look? |
Fix #86544