[libc-commits] [libc] [libc] Port remaining socket functions to syscall_wrappers (PR #198463)

[Pavel Labath via libc-commits]

================
@@ -1,32 +1,33 @@
-//===-- Linux implementation of bind --------------------------------------===//
+//===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
-
+///
+/// \file
+/// Linux implementation of bind.
+///
+//===----------------------------------------------------------------------===//
 #include "src/sys/socket/bind.h"
-
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "hdr/types/socklen_t.h"
+#include "hdr/types/struct_sockaddr.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/bind.h"
 #include "src/__support/common.h"
-
 #include "src/__support/libc_errno.h"
-#include "src/__support/macros/config.h"
-
-#include <sys/syscall.h> // For syscall numbers.
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(int, bind,
                    (int socket, const struct sockaddr *address,
                     socklen_t address_len)) {
-  int ret = syscall_impl<int>(SYS_bind, socket, address, address_len);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto result = linux_syscalls::bind(socket, address, address_len);
+  if (!result.has_value()) {
----------------
labath wrote:

Are you sure about that? I understand how that makes sense for functions implemented inside libc, where deterministically crashing is better than.. you know... mining some bitcoin and hiring a hitman to kill you.

However, this function is not accessing the pointer and it's not UB to pass a null (or any other) pointer through the syscall boundary. The kernel will just return an EFAULT. This isn't specified by POSIX, but it is documented in the linux manpage and both glibc and musl work that way.

Granted, a nullptr here would most likely indicate a bug in user code, but I have also seen code which uses this property (syscall returning EFAULT) to check for validity of pointers (you can imagine a beefed up version of LIBC_CRASH_ON_NULLPTR, which uses a syscall to check the validity of a pointer and crashes if it gets EFAULT).

If it was up to me, I'd say that this should be used only in places where our own code accesses the pointers -- but I don't think there's anything like that here. recvmsg comes close (accesses the pointer to unpoison the data), but I wouldn't put it even there, as this code is only executed on the success path (which will not be triggered by an invalid pointer).

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