From 7760e3a374d9eb1a4adae8209c07517252b6a4c4 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 17 Dec 2024 14:54:20 +0100 Subject: [PATCH] CLEANUP: quic: replace ALREADY_CHECKED() with ASSUME_NONNULL() at a few places There were 4 instances of ALREADY_CHECKED() used to tell the compiler that the argument couldn't be NULL by design. Let's change them to the cleaner ASSUME_NONNULL(). Functions like qc_snd_buf() were slightly reduced in size (-24 bytes). Apparently gcc-13 sees a potential case that others don't see, and it's likely a bug since depending what is masked, it will completely change the output warnings to the point of contradicting itself. After many attempts, it appears that just checking that CMSG_FIRSTHDR(msg) is not null suffices to calm it down, so the strange warnings might have been the result of an overoptimization based on a supposed UB in the first place. At least now all versions up to 13.2 as well as clang are happy. --- src/quic_loss.c | 2 +- src/quic_sock.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/quic_loss.c b/src/quic_loss.c index affa6f206..1c32d92c4 100644 --- a/src/quic_loss.c +++ b/src/quic_loss.c @@ -317,7 +317,7 @@ int qc_release_lost_pkts(struct quic_conn *qc, struct quic_pktns *pktns, * that list is not empty. Without this, GCC 12.2.0 reports a * possible overflow on a 0 byte region with O2 optimization. */ - ALREADY_CHECKED(oldest_lost); + ASSUME_NONNULL(oldest_lost); quic_tx_packet_refdec(oldest_lost); if (newest_lost != oldest_lost) quic_tx_packet_refdec(newest_lost); diff --git a/src/quic_sock.c b/src/quic_sock.c index 327deb492..b7ee8f8f4 100644 --- a/src/quic_sock.c +++ b/src/quic_sock.c @@ -613,8 +613,11 @@ static void cmsg_set_saddr(struct msghdr *msg, struct cmsghdr **cmsg, /* Set first msg_controllen to be able to use CMSG_* macros. */ msg->msg_controllen += CMSG_SPACE(sz); + /* seems necessary to please gcc-13 */ + ASSUME_NONNULL(CMSG_FIRSTHDR(msg)); + *cmsg = !(*cmsg) ? CMSG_FIRSTHDR(msg) : CMSG_NXTHDR(msg, *cmsg); - ALREADY_CHECKED(*cmsg); + ASSUME_NONNULL(*cmsg); c = *cmsg; c->cmsg_len = CMSG_LEN(sz); @@ -666,8 +669,11 @@ static void cmsg_set_gso(struct msghdr *msg, struct cmsghdr **cmsg, /* Set first msg_controllen to be able to use CMSG_* macros. */ msg->msg_controllen += CMSG_SPACE(sz); + /* seems necessary to please gcc-13 */ + ASSUME_NONNULL(CMSG_FIRSTHDR(msg)); + *cmsg = !(*cmsg) ? CMSG_FIRSTHDR(msg) : CMSG_NXTHDR(msg, *cmsg); - ALREADY_CHECKED(*cmsg); + ASSUME_NONNULL(*cmsg); c = *cmsg; c->cmsg_len = CMSG_LEN(sz); @@ -884,7 +890,7 @@ int qc_rcv_buf(struct quic_conn *qc) TRACE_STATE("datagram for other connection on quic-conn socket, requeue it", QUIC_EV_CONN_RCV, qc); rxbuf = MT_LIST_POP(&l->rx.rxbuf_list, typeof(rxbuf), rxbuf_el); - ALREADY_CHECKED(rxbuf); + ASSUME_NONNULL(rxbuf); cspace = b_contig_space(&rxbuf->buf); tmp_dgram = quic_rxbuf_purge_dgrams(rxbuf); -- 2.47.3