* [PATCH 0/2] Address warnings from gcc 12.2 with -flto
@ 2022-09-28 19:00 Stefano Brivio
2022-09-28 19:00 ` [PATCH 1/2] Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12 Stefano Brivio
2022-09-28 19:00 ` [PATCH 2/2] udp: Replace pragma to ignore bogus stringop-overread warning with workaround Stefano Brivio
0 siblings, 2 replies; 3+ messages in thread
From: Stefano Brivio @ 2022-09-28 19:00 UTC (permalink / raw)
To: passt-dev
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
The first one is a bogus report of an uninitialised field passed as
input to hash functions for TCP connections, already seen with gcc 11.
The second one is a bogus stringop-overread which already happened
with gcc 12.1, but this time a "cute" pragma won't make it go away.
Both harmless, but noisy.
Based on pending series posted by David, if it matters.
Stefano Brivio (2):
Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12
udp: Replace pragma to ignore bogus stringop-overread warning with
workaround
Makefile | 6 +++---
udp.c | 26 ++++++++++++++++++--------
util.h | 23 -----------------------
3 files changed, 21 insertions(+), 34 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12
2022-09-28 19:00 [PATCH 0/2] Address warnings from gcc 12.2 with -flto Stefano Brivio
@ 2022-09-28 19:00 ` Stefano Brivio
2022-09-28 19:00 ` [PATCH 2/2] udp: Replace pragma to ignore bogus stringop-overread warning with workaround Stefano Brivio
1 sibling, 0 replies; 3+ messages in thread
From: Stefano Brivio @ 2022-09-28 19:00 UTC (permalink / raw)
To: passt-dev
[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]
Commit 1a563a0cbd49 ("passt: Address gcc 11 warnings") works around an
issue where the remote address passed to hash functions is seen as
uninitialised by gcc, with -flto and -O2.
It turns out we get the same exact behaviour on gcc 12.1 and 12.2, so
extend the applicability of the same workaround to gcc 12.
Don't go further than that, though: should the issue reported at:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993
happen to be fixed in a later version of gcc, we won't need the
noinline attributes anymore. Otherwise, we'll notice.
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 7a0b829..1d45f17 100644
--- a/Makefile
+++ b/Makefile
@@ -45,12 +45,12 @@ PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
pcap.h port_fwd.h siphash.h tap.h tcp.h tcp_splice.h udp.h util.h
HEADERS = $(PASST_HEADERS) seccomp.h
-# On gcc 11.2, with -O2 and -flto, tcp_hash() and siphash_20b(), if inlined,
-# seem to be hitting something similar to:
+# On gcc 11 and 12, with -O2 and -flto, tcp_hash() and siphash_20b(), if
+# inlined, seem to be hitting something similar to:
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993
# from the pointer arithmetic used from the tcp_tap_handler() path to get the
# remote connection address.
-ifeq ($(shell $(CC) -dumpversion),11)
+ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion)))
ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS)))
ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS)))
FLAGS += -DTCP_HASH_NOINLINE
--
@@ -45,12 +45,12 @@ PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
pcap.h port_fwd.h siphash.h tap.h tcp.h tcp_splice.h udp.h util.h
HEADERS = $(PASST_HEADERS) seccomp.h
-# On gcc 11.2, with -O2 and -flto, tcp_hash() and siphash_20b(), if inlined,
-# seem to be hitting something similar to:
+# On gcc 11 and 12, with -O2 and -flto, tcp_hash() and siphash_20b(), if
+# inlined, seem to be hitting something similar to:
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993
# from the pointer arithmetic used from the tcp_tap_handler() path to get the
# remote connection address.
-ifeq ($(shell $(CC) -dumpversion),11)
+ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion)))
ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS)))
ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS)))
FLAGS += -DTCP_HASH_NOINLINE
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] udp: Replace pragma to ignore bogus stringop-overread warning with workaround
2022-09-28 19:00 [PATCH 0/2] Address warnings from gcc 12.2 with -flto Stefano Brivio
2022-09-28 19:00 ` [PATCH 1/2] Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12 Stefano Brivio
@ 2022-09-28 19:00 ` Stefano Brivio
1 sibling, 0 replies; 3+ messages in thread
From: Stefano Brivio @ 2022-09-28 19:00 UTC (permalink / raw)
To: passt-dev
[-- Attachment #1: Type: text/plain, Size: 3734 bytes --]
Commit c318ffcb4c93 ("udp: Ignore bogus -Wstringop-overread for
write() from gcc 12.1") uses a gcc pragma to ignore a bogus warning,
which started appearing on gcc 12.1 (aarch64 and x86_64) due to:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103483
...but gcc 12.2 has the same issue. Not just that: if LTO is enabled,
the pragma itself is ignored (this wasn't the case with gcc 12.1),
because of:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922
Drop the pragma, and assign a frame (in the networking sense) pointer
from the offset of the Ethernet header in the buffer, then pass it to
write() and pcap(), so that gcc doesn't obsess anymore with the fact
that an Ethernet header is 14 bytes and we're sending more than that.
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
udp.c | 26 ++++++++++++++++++--------
util.h | 23 -----------------------
2 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/udp.c b/udp.c
index bd3dc72..d1be622 100644
--- a/udp.c
+++ b/udp.c
@@ -704,11 +704,20 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh));
if (c->mode == MODE_PASTA) {
- PRAGMA_STRINGOP_OVERREAD_IGNORE
- if (write(c->fd_tap, &b->eh, sizeof(b->eh) + ip_len) < 0)
+ /* If we pass &b->eh directly to write(), starting from
+ * gcc 12.1, at least on aarch64 and x86_64, we get a bogus
+ * stringop-overread warning, due to:
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103483
+ *
+ * but we can't disable it with a pragma, because it will be
+ * ignored if LTO is enabled:
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922
+ */
+ void *frame = (char *)b + offsetof(struct udp4_l2_buf_t, eh);
+
+ if (write(c->fd_tap, frame, sizeof(b->eh) + ip_len) < 0)
debug("tap write: %s", strerror(errno));
- PRAGMA_STRINGOP_OVERREAD_IGNORE_POP
- pcap((char *)&b->eh, sizeof(b->eh) + ip_len);
+ pcap(frame, sizeof(b->eh) + ip_len);
return;
}
@@ -805,11 +814,12 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
b->ip6h.hop_limit = 255;
if (c->mode == MODE_PASTA) {
- PRAGMA_STRINGOP_OVERREAD_IGNORE
- if (write(c->fd_tap, &b->eh, sizeof(b->eh) + ip_len) < 0)
+ /* See udp_sock_fill_data_v4() for the reason behind 'frame' */
+ void *frame = (char *)b + offsetof(struct udp6_l2_buf_t, eh);
+
+ if (write(c->fd_tap, frame, sizeof(b->eh) + ip_len) < 0)
debug("tap write: %s", strerror(errno));
- PRAGMA_STRINGOP_OVERREAD_IGNORE_POP
- pcap((char *)&b->eh, sizeof(b->eh) + ip_len);
+ pcap(frame, sizeof(b->eh) + ip_len);
return;
}
diff --git a/util.h b/util.h
index 0c3c994..0c06e34 100644
--- a/util.h
+++ b/util.h
@@ -96,29 +96,6 @@ void trace_init(int enable);
(void *)(arg)); \
} while (0)
-
-#ifdef __has_warning
-# if __has_warning("-Wstringop-overread")
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE \
- _Pragma("GCC diagnostic ignored \"-Wstringop-overread\"")
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP \
- _Pragma("GCC diagnostic pop")
-# else
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP
-# endif
-#else
-# if defined(__GNUC__) && __GNUC__ >= 11
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE \
- _Pragma("GCC diagnostic ignored \"-Wstringop-overread\"")
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP \
- _Pragma("GCC diagnostic pop")
-# else
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP
-# endif
-#endif
-
#if __BYTE_ORDER == __BIG_ENDIAN
#define L2_BUF_ETH_IP4_INIT \
{ \
--
@@ -96,29 +96,6 @@ void trace_init(int enable);
(void *)(arg)); \
} while (0)
-
-#ifdef __has_warning
-# if __has_warning("-Wstringop-overread")
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE \
- _Pragma("GCC diagnostic ignored \"-Wstringop-overread\"")
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP \
- _Pragma("GCC diagnostic pop")
-# else
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP
-# endif
-#else
-# if defined(__GNUC__) && __GNUC__ >= 11
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE \
- _Pragma("GCC diagnostic ignored \"-Wstringop-overread\"")
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP \
- _Pragma("GCC diagnostic pop")
-# else
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE
-# define PRAGMA_STRINGOP_OVERREAD_IGNORE_POP
-# endif
-#endif
-
#if __BYTE_ORDER == __BIG_ENDIAN
#define L2_BUF_ETH_IP4_INIT \
{ \
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-28 19:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28 19:00 [PATCH 0/2] Address warnings from gcc 12.2 with -flto Stefano Brivio
2022-09-28 19:00 ` [PATCH 1/2] Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12 Stefano Brivio
2022-09-28 19:00 ` [PATCH 2/2] udp: Replace pragma to ignore bogus stringop-overread warning with workaround Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).