treewide: Disable gcc strict aliasing rules as needed, drop workarounds

Recently, commit 4ddbcb9c0c55 ("tcp: Disable optimisations
for tcp_hash()") worked around yet another issue we hit with gcc 12
and '-flto -O2': some stores affecting the input data to siphash_20b()
were omitted altogether, and tcp_hash() wouldn't get the correct hash
for incoming connections.

Digging further into this revealed that, at least according to gcc's
interpretation of C99 aliasing rules, passing pointers to functions
with different types compared to the effective type of the object
(for example, a uint8_t pointer to an anonymous struct, as it happens
in tcp_hash()), doesn't guarantee that stores are not reordered
across the function call.

This means that, in general, our checksum and hash functions might
not see parts of input data that was intended to be provided by
callers.

Not even switching from uint8_t to character types, which should be
appropriate here, according to C99 (ISO/IEC 9899, TC3, draft N1256),
section 6.5, "Expressions", paragraph 7:

  An object shall have its stored value accessed only by an lvalue
  expression that has one of the following types:

  [...]

  — a character type.

does the trick. I guess this is also subject to interpretation:
casting passed pointers to character types, and then using those as
different types, might still violate (dubious) aliasing rules.

Disable gcc strict aliasing rules for potentially affected functions,
which, in turn, disables gcc's Type-Based Alias Analysis (TBAA)
optimisations based on those function arguments.

Drop the existing workarounds. Also the (seemingly?) bogus
'maybe-uninitialized' warning on the tcp_tap_handler() > tcp_hash() >
siphash_20b() path goes away with -fno-strict-aliasing on
siphash_20b().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

1 files changed, 0 insertions, 21 deletions

diff --git a/Makefile b/Makefile
index 080c748..667ddfb 100644
--- a/Makefile
+++ b/Makefile
@@ -56,27 +56,6 @@ PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
 	tcp_splice.h udp.h util.h
 HEADERS = $(PASST_HEADERS) seccomp.h
 
-# 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.
-#
-# TODO: With the same combination, in ndp(), gcc optimises away the store of
-# hop_limit in the IPv6 header (temporarily set to the protocol number for
-# convenience, to mimic the ICMPv6 checksum pseudo-header) before the call to
-# csum_unaligned(). Mark csum_unaligned() as "noipa" as a quick work-around,
-# while we figure out if a corresponding gcc issue has already been reported.
-ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion)))
-ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS) $(CPPFLAGS)))
-ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS) $(CPPFLAGS)))
-	FLAGS += -DTCP_HASH_NOINLINE
-	FLAGS += -DSIPHASH_20B_NOINLINE
-	FLAGS += -DCSUM_UNALIGNED_NO_IPA
-endif
-endif
-endif
-
 C := \#include <linux/tcp.h>\nstruct tcp_info x = { .tcpi_snd_wnd = 0 };
 ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
 	FLAGS += -DHAS_SND_WND