[libc] Add struct cmsghdr and associated macros#193756
[libc] Add struct cmsghdr and associated macros#193756
Conversation
The macros are the main source of subtlety. The interesting aspects are: - some implementations CMSG_ALIGN the size of struct cmsghdr, but this is a noop. Instead of doing that, I added an assertion in the test. - POSIX permits CMSG_NXTHDR to return null if the buffer has no space for the data array, and this behavior differs between implementations. This implementation does not do that in order to match CMSG_FIRSTHDR, which doesn't have such an option. - some implementations redirect the CMSG_NXTHDR macro to an (extern or static inline) function. I implemented this inside the macro to avoid having to define a (private ?) entry point for that function.
|
@llvm/pr-subscribers-libc Author: Pavel Labath (labath) ChangesThe macros are the main source of subtlety. The interesting aspects are:
Full diff: https://github.com/llvm/llvm-project/pull/193756.diff 8 Files Affected:
diff --git a/libc/hdr/types/CMakeLists.txt b/libc/hdr/types/CMakeLists.txt
index 40054b294a781..a3057397725bd 100644
--- a/libc/hdr/types/CMakeLists.txt
+++ b/libc/hdr/types/CMakeLists.txt
@@ -394,6 +394,15 @@ add_proxy_header_library(
libc.include.sys_socket
)
+add_proxy_header_library(
+ struct_cmsghdr
+ HDRS
+ struct_cmsghdr.h
+ FULL_BUILD_DEPENDS
+ libc.include.llvm-libc-types.struct_cmsghdr
+ libc.include.sys_socket
+)
+
add_proxy_header_library(
struct_msghdr
HDRS
diff --git a/libc/hdr/types/struct_cmsghdr.h b/libc/hdr/types/struct_cmsghdr.h
new file mode 100644
index 0000000000000..b84a1a2ede798
--- /dev/null
+++ b/libc/hdr/types/struct_cmsghdr.h
@@ -0,0 +1,21 @@
+//===-- Proxy for struct cmsghdr ------------------------------------------===//
+//
+// 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_TYPES_STRUCT_CMSGHDR_H
+#define LLVM_LIBC_HDR_TYPES_STRUCT_CMSGHDR_H
+
+#ifdef LIBC_FULL_BUILD
+
+#include "include/llvm-libc-types/struct_cmsghdr.h"
+
+#else
+
+#include <sys/socket.h>
+
+#endif // LIBC_FULL_BUILD
+
+#endif // LLVM_LIBC_HDR_TYPES_STRUCT_CMSGHDR_H
diff --git a/libc/include/llvm-libc-macros/linux/sys-socket-macros.h b/libc/include/llvm-libc-macros/linux/sys-socket-macros.h
index 7fb12dace5fd6..0d4460df0196b 100644
--- a/libc/include/llvm-libc-macros/linux/sys-socket-macros.h
+++ b/libc/include/llvm-libc-macros/linux/sys-socket-macros.h
@@ -50,4 +50,23 @@
#define SHUT_WR 1
#define SHUT_RDWR 2
+#define SCM_RIGHTS 1
+
+#define CMSG_ALIGN(len) (((len) + sizeof(size_t) - 1) & ~(sizeof(size_t) - 1))
+#define CMSG_LEN(len) (sizeof(struct cmsghdr) + (len))
+#define CMSG_SPACE(len) (sizeof(struct cmsghdr) + CMSG_ALIGN(len))
+
+#define CMSG_FIRSTHDR(msg) \
+ ((msg)->msg_controllen >= sizeof(struct cmsghdr) \
+ ? (struct cmsghdr *)(msg)->msg_control \
+ : 0)
+#define __CMSG_NXTHDR_CANDIDATE(cmsg) \
+ ((struct cmsghdr *)((unsigned char *)(cmsg) + CMSG_ALIGN((cmsg)->cmsg_len)))
+#define CMSG_NXTHDR(msg, cmsg) \
+ ((char *)(__CMSG_NXTHDR_CANDIDATE(cmsg) + 1) <= \
+ ((char *)((msg)->msg_control) + (msg)->msg_controllen) \
+ ? __CMSG_NXTHDR_CANDIDATE(cmsg) \
+ : 0)
+#define CMSG_DATA(cmsg) ((unsigned char *)((struct cmsghdr *)(cmsg) + 1))
+
#endif // LLVM_LIBC_MACROS_LINUX_SYS_SOCKET_MACROS_H
diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index 6c2a001103250..b4e04f9a8b810 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -199,6 +199,7 @@ add_header(struct_sockaddr HDR struct_sockaddr.h DEPENDS .sa_family_t)
add_header(struct_sockaddr_storage HDR struct_sockaddr_storage.h DEPENDS .sa_family_t)
add_header(struct_iovec HDR struct_iovec.h DEPENDS .size_t)
add_header(struct_linger HDR struct_linger.h)
+add_header(struct_cmsghdr HDR struct_cmsghdr.h DEPENDS .size_t)
add_header(struct_msghdr HDR struct_msghdr.h DEPENDS .size_t .socklen_t .struct_iovec)
add_header(ACTION HDR ACTION.h)
add_header(ENTRY HDR ENTRY.h)
diff --git a/libc/include/llvm-libc-types/struct_cmsghdr.h b/libc/include/llvm-libc-types/struct_cmsghdr.h
new file mode 100644
index 0000000000000..baefcf8da6c25
--- /dev/null
+++ b/libc/include/llvm-libc-types/struct_cmsghdr.h
@@ -0,0 +1,22 @@
+//===-- Definition of struct cmsghdr --------------------------------------===//
+//
+// 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_TYPES_STRUCT_CMSGHDR_H
+#define LLVM_LIBC_TYPES_STRUCT_CMSGHDR_H
+
+#include "size_t.h"
+
+struct cmsghdr {
+ // NB: This needs to match the kernel definition (which uses size_t), even
+ // though POSIX says it should be socklen_t
+ size_t cmsg_len; // data byte count, including header
+ int cmsg_level; // originating protocol
+ int cmsg_type; // protocol-specific type
+};
+
+#endif // LLVM_LIBC_TYPES_STRUCT_CMSGHDR_H
diff --git a/libc/include/sys/socket.yaml b/libc/include/sys/socket.yaml
index 4bb18538b293a..b1a1a2adb7ebe 100644
--- a/libc/include/sys/socket.yaml
+++ b/libc/include/sys/socket.yaml
@@ -9,6 +9,7 @@ types:
- type_name: struct_sockaddr_storage
- type_name: socklen_t
- type_name: sa_family_t
+ - type_name: struct_cmsghdr
- type_name: struct_msghdr
- type_name: struct_iovec
- type_name: struct_linger
diff --git a/libc/test/src/sys/socket/linux/CMakeLists.txt b/libc/test/src/sys/socket/linux/CMakeLists.txt
index f4bd361258e51..ef88e7e6b9499 100644
--- a/libc/test/src/sys/socket/linux/CMakeLists.txt
+++ b/libc/test/src/sys/socket/linux/CMakeLists.txt
@@ -160,7 +160,11 @@ add_libc_unittest(
sendmsg_recvmsg_test.cpp
DEPENDS
libc.include.sys_socket
+ libc.hdr.types.struct_cmsghdr
libc.src.errno.errno
+ libc.src.string.memcpy
+ libc.src.string.memset
+ libc.src.sys.socket.getsockopt
libc.src.sys.socket.socketpair
libc.src.sys.socket.sendmsg
libc.src.sys.socket.recvmsg
diff --git a/libc/test/src/sys/socket/linux/sendmsg_recvmsg_test.cpp b/libc/test/src/sys/socket/linux/sendmsg_recvmsg_test.cpp
index ae0d0557573ea..321794cb5eba9 100644
--- a/libc/test/src/sys/socket/linux/sendmsg_recvmsg_test.cpp
+++ b/libc/test/src/sys/socket/linux/sendmsg_recvmsg_test.cpp
@@ -6,6 +6,12 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/sys_socket_macros.h"
+#include "hdr/types/struct_cmsghdr.h"
+#include "include/llvm-libc-macros/linux/sys-socket-macros.h"
+#include "src/string/memcpy.h"
+#include "src/string/memset.h"
+#include "src/sys/socket/getsockopt.h"
#include "src/sys/socket/recvmsg.h"
#include "src/sys/socket/sendmsg.h"
#include "src/sys/socket/socketpair.h"
@@ -14,6 +20,7 @@
#include "test/UnitTest/ErrnoCheckingTest.h"
#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/LibcTest.h"
#include "test/UnitTest/Test.h"
#include <sys/socket.h> // For AF_UNIX and SOCK_DGRAM
@@ -73,6 +80,122 @@ TEST_F(LlvmLibcSendMsgRecvMsgTest, SucceedsWithSocketPair) {
ASSERT_THAT(LIBC_NAMESPACE::close(sockpair[1]), Succeeds(0));
}
+TEST_F(LlvmLibcSendMsgRecvMsgTest, CmsgDetails) {
+ ASSERT_EQ(CMSG_ALIGN(0), static_cast<size_t>(0));
+ ASSERT_EQ(CMSG_ALIGN(1), sizeof(size_t));
+
+ // Some implementations align struct cmsghdr in various size computations, but
+ // this is a noop. This verifies that.
+ ASSERT_EQ(CMSG_ALIGN(sizeof(struct cmsghdr)), sizeof(struct cmsghdr));
+
+ char buf[0x100] = {};
+
+ struct msghdr msg;
+ msg.msg_control = buf;
+
+ // We shouldn't be able to get the first header if there's not enough space
+ // for it.
+ msg.msg_controllen = 0;
+ ASSERT_EQ(CMSG_FIRSTHDR(&msg), nullptr);
+ msg.msg_controllen = sizeof(struct cmsghdr) - 1;
+ ASSERT_EQ(CMSG_FIRSTHDR(&msg), nullptr);
+ msg.msg_controllen = sizeof(struct cmsghdr);
+ ASSERT_EQ(CMSG_FIRSTHDR(&msg), reinterpret_cast<struct cmsghdr *>(buf));
+ msg.msg_controllen = sizeof(buf);
+ struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
+ ASSERT_EQ(cmsg, reinterpret_cast<struct cmsghdr *>(buf));
+
+ // We shouldn't be able to get the next header if this one is too big.
+ cmsg->cmsg_len = 0x1000;
+ ASSERT_EQ(CMSG_NXTHDR(&msg, cmsg), nullptr);
+ cmsg->cmsg_len = sizeof(buf) - sizeof(struct cmsghdr) + 1;
+ ASSERT_EQ(CMSG_NXTHDR(&msg, cmsg), nullptr);
+
+ cmsg->cmsg_len = sizeof(buf) - sizeof(struct cmsghdr);
+ struct cmsghdr *cmsg2 = CMSG_NXTHDR(&msg, cmsg);
+ ASSERT_LT(buf, reinterpret_cast<char *>(cmsg2));
+ ASSERT_LT(reinterpret_cast<char *>(cmsg2), buf + sizeof(buf));
+
+ // POSIX allows explicitly does not specify whether CMSG_NXTHDR returns the
+ // next header if its data array would extend beyond the end of the buffer.
+ // Our implementation does.
+ cmsg2->cmsg_len = sizeof(struct cmsghdr) + 1;
+ ASSERT_EQ(CMSG_NXTHDR(&msg, cmsg), cmsg2);
+}
+
+TEST_F(LlvmLibcSendMsgRecvMsgTest, SendAndReceiveFileDescriptor) {
+ int sockpair[2] = {0, 0};
+
+ ASSERT_THAT(LIBC_NAMESPACE::socketpair(AF_UNIX, SOCK_STREAM, 0, sockpair),
+ Succeeds(0));
+
+ iovec iov;
+ iov.iov_base = reinterpret_cast<void *>(const_cast<char *>("x"));
+ iov.iov_len = 1;
+
+ char control_buf[CMSG_SPACE(sizeof(int))] = {};
+
+ msghdr msg;
+ msg.msg_name = nullptr;
+ msg.msg_namelen = 0;
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+ msg.msg_flags = 0;
+ msg.msg_control = control_buf;
+ msg.msg_controllen = CMSG_LEN(sizeof(int));
+
+ cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
+ cmsg->cmsg_level = SOL_SOCKET;
+ cmsg->cmsg_type = SCM_RIGHTS;
+ cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+ LIBC_NAMESPACE::memcpy(CMSG_DATA(cmsg), sockpair + 1, sizeof(int));
+
+ ASSERT_THAT(LIBC_NAMESPACE::sendmsg(sockpair[0], &msg, 0),
+ Succeeds(static_cast<ssize_t>(1)));
+
+ char buffer[256];
+
+ iov.iov_base = buffer;
+ iov.iov_len = sizeof(buffer);
+
+ LIBC_NAMESPACE::memset(control_buf, 0, sizeof(control_buf));
+
+ msg.msg_name = nullptr;
+ msg.msg_namelen = 0;
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+ msg.msg_control = control_buf;
+ msg.msg_controllen = sizeof(control_buf);
+ msg.msg_flags = 0;
+
+ ASSERT_THAT(LIBC_NAMESPACE::recvmsg(sockpair[1], &msg, 0),
+ Succeeds(static_cast<ssize_t>(1)));
+
+ ASSERT_EQ(buffer[0], 'x');
+
+ cmsg = CMSG_FIRSTHDR(&msg);
+
+ ASSERT_TRUE(cmsg != nullptr);
+ ASSERT_EQ(cmsg->cmsg_level, SOL_SOCKET);
+ ASSERT_EQ(cmsg->cmsg_type, SCM_RIGHTS);
+ ASSERT_EQ(cmsg->cmsg_len, CMSG_LEN(sizeof(int)));
+ ASSERT_EQ(CMSG_NXTHDR(&msg, cmsg), nullptr);
+
+ int new_fd;
+ LIBC_NAMESPACE::memcpy(&new_fd, CMSG_DATA(cmsg), sizeof(int));
+
+ int new_sock_type = 0;
+ socklen_t optlen = sizeof(int);
+ ASSERT_THAT(LIBC_NAMESPACE::getsockopt(new_fd, SOL_SOCKET, SO_TYPE,
+ &new_sock_type, &optlen),
+ Succeeds(0));
+ ASSERT_EQ(new_sock_type, SOCK_STREAM);
+
+ ASSERT_THAT(LIBC_NAMESPACE::close(sockpair[0]), Succeeds(0));
+ ASSERT_THAT(LIBC_NAMESPACE::close(sockpair[1]), Succeeds(0));
+ ASSERT_THAT(LIBC_NAMESPACE::close(new_fd), Succeeds(0));
+}
+
TEST_F(LlvmLibcSendMsgRecvMsgTest, SendFails) {
const char TEST_MESSAGE[] = "connection terminated";
const size_t MESSAGE_LEN = sizeof(TEST_MESSAGE);
|
| #define CMSG_LEN(len) (sizeof(struct cmsghdr) + (len)) | ||
| #define CMSG_SPACE(len) (sizeof(struct cmsghdr) + CMSG_ALIGN(len)) | ||
|
|
||
| #define CMSG_FIRSTHDR(msg) \ |
There was a problem hiding this comment.
I'm curious for type safety if we could do something like have these be static inline in the header, and have the define point to those functions so that we satisfy the POSIX needs for this to be a macro.
There was a problem hiding this comment.
We'd have to make sure the function compiles both as C and C++, but yes, that should be doable. I haven't seen anybody do it for FIRSTHDR, but there are libcs (glibc included) which do it for NXTHDR.
I don't think it helps much with type safety, as both macros contain direct member access (so the only way this would compile is if a user passes a type with a c which has a msg_control member), but I guess the error message could be better.
Would you like me to do that?
There was a problem hiding this comment.
It might not be worth it, your call. I just get itchy thinking about the times I get confusing things out of macros where a compiler error could have told me how I was holding it wrong. =)
Co-authored-by: Jeff Bailey <jbailey@raspberryginger.com>
The macros are the main source of subtlety. The interesting aspects are: