From 9d933ef9191417b4b7d29eaa3c3a571f814acc8e Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Tue, 10 Dec 2019 17:05:21 +1000 Subject: [PATCH 1/2] prevector: avoid misaligned member accesses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `#pragma pack(1)` prevents aligning the struct and its members to their required alignment. This can result in code that performs non-aligned reads and writes to integers and pointers, which is problematic on some architectures. It also triggers UBsan — see https://github.com/bitcoin/bitcoin/pull/17156#issuecomment-543123631 and #17510. --- src/prevector.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/prevector.h b/src/prevector.h index 4fb07688ffb..f2a0c3e05b5 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -15,7 +15,6 @@ #include #include -#pragma pack(push, 1) /** Implements a drop-in replacement for std::vector which stores up to N * elements directly (without heap allocation). The types Size and Diff are * used to store element counts, and can be any unsigned + signed type. @@ -147,14 +146,20 @@ public: }; private: - size_type _size = 0; +#pragma pack(push, 1) union direct_or_indirect { char direct[sizeof(T) * N]; struct { - size_type capacity; char* indirect; + size_type capacity; }; - } _union = {}; + }; +#pragma pack(pop) + alignas(char*) direct_or_indirect _union = {}; + size_type _size = 0; + + static_assert(alignof(char*) % alignof(size_type) == 0 && sizeof(char*) % alignof(size_type) == 0, "size_type cannot have more restrictive alignment requirement than pointer"); + static_assert(alignof(char*) % alignof(T) == 0, "value_type T cannot have more restrictive alignment requirement than pointer"); T* direct_ptr(difference_type pos) { return reinterpret_cast(_union.direct) + pos; } const T* direct_ptr(difference_type pos) const { return reinterpret_cast(_union.direct) + pos; } @@ -523,6 +528,5 @@ public: return item_ptr(0); } }; -#pragma pack(pop) #endif // BITCOIN_PREVECTOR_H From 5f26855f109af53a336d5f98ed0ae584e7a31f84 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 20 Nov 2019 10:20:39 +0100 Subject: [PATCH 2/2] test: Remove ubsan alignment suppressions This can be done now that prevector is properly aligned. --- test/sanitizer_suppressions/ubsan | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index f5de358bcb9..8284a6916d6 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -1,7 +1,5 @@ # -fsanitize=undefined suppressions # ================================= -alignment:move.h -alignment:prevector.h float-divide-by-zero:policy/fees.cpp float-divide-by-zero:validation.cpp float-divide-by-zero:wallet/wallet.cpp