aboutgitcodebugslistschat
path: root/tcp.c
diff options
context:
space:
mode:
authorStefano Brivio <sbrivio@redhat.com>2023-02-27 01:57:36 +0100
committerStefano Brivio <sbrivio@redhat.com>2023-02-27 18:54:13 +0100
commita48c5c2abf8aba39c32cf6845d51b5ca05d33361 (patch)
treec364fb948bc6ae447862755ad590e518c1b58157 /tcp.c
parentda46fdac36058e97e5a3131e2c51d162f4c619e9 (diff)
downloadpasst-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.tar
passt-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.tar.gz
passt-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.tar.bz2
passt-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.tar.lz
passt-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.tar.xz
passt-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.tar.zst
passt-a48c5c2abf8aba39c32cf6845d51b5ca05d33361.zip
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>
Diffstat (limited to 'tcp.c')
-rw-r--r--tcp.c6
1 files changed, 0 insertions, 6 deletions
diff --git a/tcp.c b/tcp.c
index 21c319d..cbd537e 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1182,12 +1182,6 @@ static int tcp_hash_match(const struct tcp_tap_conn *conn,
*
* Return: hash value, already modulo size of the hash table
*/
-#if TCP_HASH_NOINLINE
-__attribute__((__noinline__)) /* See comment in Makefile */
-#endif
-__attribute__((optimize("O0"))) /* TODO: with -O2 and -flto on gcc 12.2,
- * siphash_20b() doesn't see 'addr', why?
- */
static unsigned int tcp_hash(const struct ctx *c, const union inany_addr *addr,
in_port_t tap_port, in_port_t sock_port)
{