Skip to content

Commit

Permalink
Merge bitcoin#17708: prevector: avoid misaligned member accesses
Browse files Browse the repository at this point in the history
5f26855 test: Remove ubsan alignment suppressions (Wladimir J. van der Laan)
9d933ef prevector: avoid misaligned member accesses (Anthony Towns)

Pull request description:

  Ensure prevector data is appropriately aligned. Earlier discussion in bitcoin#17530.

  **Edit laanwj**: In contrast to bitcoin#17530, it does this without increase in size of any of the coin cache data structures (x86_64, clang)

  | Struct        | (size,align) before           | (size,align) after  |
  | ------------- | ------------- | ------- |
  | Coin | 48, 8        |     48, 8   |
  | CCoinsCacheEntry | 56, 8    |   56, 8  |
  | CScript | 32, 1       |      32, 8  |

ACKs for top commit:
  laanwj:
    ACK 5f26855
  practicalswift:
    ACK 5f26855
  jonatack:
    ACK 5f26855

Tree-SHA512: 98d112d6856f683d5b212410b73f3071d2994f1efb046a2418a35890aa1cf1aa7c96a960fc2e963fa15241e861093c1ea41951cf5b4b5431f88345eb1dd0a98a
  • Loading branch information
laanwj authored and kwvg committed Oct 12, 2021
1 parent f0d143e commit dd0657d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
15 changes: 9 additions & 6 deletions src/prevector.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include <compat.h>

#pragma pack(push, 1)
/** Implements a drop-in replacement for std::vector<T> 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.
Expand Down Expand Up @@ -149,14 +148,20 @@ class prevector {
};

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<T*>(_union.direct) + pos; }
const T* direct_ptr(difference_type pos) const { return reinterpret_cast<const T*>(_union.direct) + pos; }
Expand Down Expand Up @@ -567,6 +572,4 @@ class prevector {
}
};

#pragma pack(pop)

#endif // BITCOIN_PREVECTOR_H
4 changes: 2 additions & 2 deletions test/sanitizer_suppressions/ubsan
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
alignment:move.h
alignment:prevector.h
# -fsanitize=undefined suppressions
# =================================
bool:wallet/wallet.cpp
float-divide-by-zero:policy/fees.cpp
float-divide-by-zero:validation.cpp
Expand Down

0 comments on commit dd0657d

Please sign in to comment.