From 0ad5f4344238b548e2240a405d418f7af9290623 Mon Sep 17 00:00:00 2001 From: Rich Ercolani <214141+rincebrain@users.noreply.github.com> Date: Fri, 24 Mar 2023 13:29:19 -0400 Subject: [PATCH] Drop lying to the compiler in the fletcher4 code This is probably the uncontroversial part of #13631, which fixes a real problem people are having. There's still things to improve in our code after this is merged, but it should stop the breakage that people have reported, where we lie about a type always being aligned and then pass in stack objects with no alignment requirement and hope for the best. Of course, our SIMD code was written with unaligned accesses, so it doesn't care if we drop this...but some auto-vectorized code that gcc emits sure does, since we told it it can assume they're aligned. Reviewed-by: Brian Behlendorf Reviewed-by: Tino Reichardt Reviewed-by: Richard Yao Signed-off-by: Rich Ercolani Closes #14649 --- include/zfs_fletcher.h | 24 ++++------------------ module/zcommon/zfs_fletcher.c | 4 ---- module/zcommon/zfs_fletcher_aarch64_neon.c | 2 -- module/zcommon/zfs_fletcher_avx512.c | 2 -- module/zcommon/zfs_fletcher_intel.c | 2 -- module/zcommon/zfs_fletcher_sse.c | 2 -- module/zcommon/zfs_fletcher_superscalar.c | 4 ---- module/zcommon/zfs_fletcher_superscalar4.c | 4 ---- 8 files changed, 4 insertions(+), 40 deletions(-) diff --git a/include/zfs_fletcher.h b/include/zfs_fletcher.h index e913a0bd7..ca1a09292 100644 --- a/include/zfs_fletcher.h +++ b/include/zfs_fletcher.h @@ -76,19 +76,19 @@ typedef struct zfs_fletcher_superscalar { } zfs_fletcher_superscalar_t; typedef struct zfs_fletcher_sse { - uint64_t v[2] __attribute__((aligned(16))); + uint64_t v[2]; } zfs_fletcher_sse_t; typedef struct zfs_fletcher_avx { - uint64_t v[4] __attribute__((aligned(32))); + uint64_t v[4]; } zfs_fletcher_avx_t; typedef struct zfs_fletcher_avx512 { - uint64_t v[8] __attribute__((aligned(64))); + uint64_t v[8]; } zfs_fletcher_avx512_t; typedef struct zfs_fletcher_aarch64_neon { - uint64_t v[2] __attribute__((aligned(16))); + uint64_t v[2]; } zfs_fletcher_aarch64_neon_t; @@ -161,20 +161,4 @@ _ZFS_FLETCHER_H const fletcher_4_ops_t fletcher_4_aarch64_neon_ops; } #endif -#if defined(ZFS_UBSAN_ENABLED) -#if defined(__has_attribute) -#if __has_attribute(no_sanitize_undefined) -#define ZFS_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize_undefined)) -#elif __has_attribute(no_sanitize) -#define ZFS_NO_SANITIZE_UNDEFINED __attribute__((no_sanitize("undefined"))) -#else -#error "Compiler has to support attribute " - "`no_sanitize_undefined` or `no_sanitize(\"undefined\")`" - "when compiling with UBSan enabled" -#endif /* __has_attribute(no_sanitize_undefined) */ -#endif /* defined(__has_attribute) */ -#else -#define ZFS_NO_SANITIZE_UNDEFINED -#endif /* defined(ZFS_UBSAN_ENABLED) */ - #endif /* _ZFS_FLETCHER_H */ diff --git a/module/zcommon/zfs_fletcher.c b/module/zcommon/zfs_fletcher.c index eae854f3d..1d9b1cffc 100644 --- a/module/zcommon/zfs_fletcher.c +++ b/module/zcommon/zfs_fletcher.c @@ -301,21 +301,18 @@ fletcher_2_byteswap(const void *buf, uint64_t size, (void) fletcher_2_incremental_byteswap((void *) buf, size, zcp); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_scalar_init(fletcher_4_ctx_t *ctx) { ZIO_SET_CHECKSUM(&ctx->scalar, 0, 0, 0, 0); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_scalar_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { memcpy(zcp, &ctx->scalar, sizeof (zio_cksum_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_scalar_native(fletcher_4_ctx_t *ctx, const void *buf, uint64_t size) @@ -339,7 +336,6 @@ fletcher_4_scalar_native(fletcher_4_ctx_t *ctx, const void *buf, ZIO_SET_CHECKSUM(&ctx->scalar, a, b, c, d); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_scalar_byteswap(fletcher_4_ctx_t *ctx, const void *buf, uint64_t size) diff --git a/module/zcommon/zfs_fletcher_aarch64_neon.c b/module/zcommon/zfs_fletcher_aarch64_neon.c index cd5fe545a..26f2115c4 100644 --- a/module/zcommon/zfs_fletcher_aarch64_neon.c +++ b/module/zcommon/zfs_fletcher_aarch64_neon.c @@ -48,14 +48,12 @@ #include #include -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_aarch64_neon_init(fletcher_4_ctx_t *ctx) { memset(ctx->aarch64_neon, 0, 4 * sizeof (zfs_fletcher_aarch64_neon_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_aarch64_neon_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { diff --git a/module/zcommon/zfs_fletcher_avx512.c b/module/zcommon/zfs_fletcher_avx512.c index 81182ead2..95fc2b151 100644 --- a/module/zcommon/zfs_fletcher_avx512.c +++ b/module/zcommon/zfs_fletcher_avx512.c @@ -35,14 +35,12 @@ #define __asm __asm__ __volatile__ #endif -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_avx512f_init(fletcher_4_ctx_t *ctx) { memset(ctx->avx512, 0, 4 * sizeof (zfs_fletcher_avx512_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_avx512f_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { diff --git a/module/zcommon/zfs_fletcher_intel.c b/module/zcommon/zfs_fletcher_intel.c index 6108bda7a..34590a155 100644 --- a/module/zcommon/zfs_fletcher_intel.c +++ b/module/zcommon/zfs_fletcher_intel.c @@ -47,14 +47,12 @@ #include #include -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_avx2_init(fletcher_4_ctx_t *ctx) { memset(ctx->avx, 0, 4 * sizeof (zfs_fletcher_avx_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_avx2_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { diff --git a/module/zcommon/zfs_fletcher_sse.c b/module/zcommon/zfs_fletcher_sse.c index 096472c9a..8ab9b9acb 100644 --- a/module/zcommon/zfs_fletcher_sse.c +++ b/module/zcommon/zfs_fletcher_sse.c @@ -49,14 +49,12 @@ #include #include -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_sse2_init(fletcher_4_ctx_t *ctx) { memset(ctx->sse, 0, 4 * sizeof (zfs_fletcher_sse_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_sse2_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { diff --git a/module/zcommon/zfs_fletcher_superscalar.c b/module/zcommon/zfs_fletcher_superscalar.c index 8b5b72a7b..2a80816ff 100644 --- a/module/zcommon/zfs_fletcher_superscalar.c +++ b/module/zcommon/zfs_fletcher_superscalar.c @@ -47,14 +47,12 @@ #include #include -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar_init(fletcher_4_ctx_t *ctx) { memset(ctx->superscalar, 0, 4 * sizeof (zfs_fletcher_superscalar_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { @@ -70,7 +68,6 @@ fletcher_4_superscalar_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) ZIO_SET_CHECKSUM(zcp, A, B, C, D); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar_native(fletcher_4_ctx_t *ctx, const void *buf, uint64_t size) @@ -110,7 +107,6 @@ fletcher_4_superscalar_native(fletcher_4_ctx_t *ctx, ctx->superscalar[3].v[1] = d2; } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar_byteswap(fletcher_4_ctx_t *ctx, const void *buf, uint64_t size) diff --git a/module/zcommon/zfs_fletcher_superscalar4.c b/module/zcommon/zfs_fletcher_superscalar4.c index bef387933..0b52bb63d 100644 --- a/module/zcommon/zfs_fletcher_superscalar4.c +++ b/module/zcommon/zfs_fletcher_superscalar4.c @@ -47,14 +47,12 @@ #include #include -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar4_init(fletcher_4_ctx_t *ctx) { memset(ctx->superscalar, 0, 4 * sizeof (zfs_fletcher_superscalar_t)); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar4_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) { @@ -84,7 +82,6 @@ fletcher_4_superscalar4_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp) ZIO_SET_CHECKSUM(zcp, A, B, C, D); } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar4_native(fletcher_4_ctx_t *ctx, const void *buf, uint64_t size) @@ -150,7 +147,6 @@ fletcher_4_superscalar4_native(fletcher_4_ctx_t *ctx, ctx->superscalar[3].v[3] = d4; } -ZFS_NO_SANITIZE_UNDEFINED static void fletcher_4_superscalar4_byteswap(fletcher_4_ctx_t *ctx, const void *buf, uint64_t size)