[libc-commits] [libc] [libc] Use the dup2 syscall wrapper in posix_spawn (PR #207879)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Mikhail R. Gadelha (mikhailramalho)

<details>
<summary>Changes</summary>

posix_spawn's local dup2 helper fell back to a bare dup3 on targets without SYS_dup2. dup3 fails with EINVAL when oldfd == newfd, where dup2 must instead return oldfd if it is valid, so posix_spawn_file_actions_adddup2 with equal fds misbehaved on those targets. linux_syscalls::dup2 already implements the correct fallback, so call it directly and drop the local helper.

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


2 Files Affected:

- (modified) libc/src/spawn/linux/CMakeLists.txt (+1) 
- (modified) libc/src/spawn/linux/posix_spawn.cpp (+4-14) 


``````````diff
diff --git a/libc/src/spawn/linux/CMakeLists.txt b/libc/src/spawn/linux/CMakeLists.txt
index 42057e9f12fec..910705af85b3d 100644
--- a/libc/src/spawn/linux/CMakeLists.txt
+++ b/libc/src/spawn/linux/CMakeLists.txt
@@ -11,6 +11,7 @@ add_entrypoint_object(
     libc.include.sys_syscall
     libc.include.signal
     libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.dup2
     libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.spawn.file_actions
     libc.src.signal.linux.signal_utils
diff --git a/libc/src/spawn/linux/posix_spawn.cpp b/libc/src/spawn/linux/posix_spawn.cpp
index 301be6d7c8762..2961dd5c6b82c 100644
--- a/libc/src/spawn/linux/posix_spawn.cpp
+++ b/libc/src/spawn/linux/posix_spawn.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/spawn/posix_spawn.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/dup2.h"
 #include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
@@ -41,18 +42,6 @@ pid_t fork() {
 
 void close(int fd) { LIBC_NAMESPACE::syscall_impl<long>(SYS_close, fd); }
 
-// We use dup3 if dup2 is not available, similar to our implementation of dup2
-bool dup2(int fd, int newfd) {
-#ifdef SYS_dup2
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_dup2, fd, newfd);
-#elif defined(SYS_dup3)
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_dup3, fd, newfd, 0);
-#else
-#error "dup2 and dup3 syscalls not available."
-#endif
-  return ret < 0 ? false : true;
-}
-
 // All exits from child_process are error exits. So, we use a simple
 // exit implementation which exits with code 127.
 void exit() {
@@ -84,7 +73,8 @@ void child_process(const char *__restrict path,
           exit();
         int actual_fd = *fd;
         if (actual_fd != open_act->fd) {
-          bool dup2_result = dup2(actual_fd, open_act->fd);
+          bool dup2_result =
+              linux_syscalls::dup2(actual_fd, open_act->fd).has_value();
           close(actual_fd); // The old fd is not needed anymore.
           if (!dup2_result)
             exit();
@@ -98,7 +88,7 @@ void child_process(const char *__restrict path,
       }
       case BaseSpawnFileAction::DUP2: {
         auto *dup2_act = reinterpret_cast<SpawnFileDup2Action *>(act);
-        if (!dup2(dup2_act->fd, dup2_act->newfd))
+        if (!linux_syscalls::dup2(dup2_act->fd, dup2_act->newfd).has_value())
           exit();
         break;
       }

``````````

</details>


https://github.com/llvm/llvm-project/pull/207879