Commit eb4de739 authored by Carlos Caballero's avatar Carlos Caballero Committed by Commit Bot

[promises] Make sure alignas value is compatible with heap allocations

Allocators must return memory that is aligned at least as strictly as std::max_align_t. So make
sure we do not ask for more, otherwise we risk getting misaligned memory in heap allocations.

For more context, see discussion in https://crrev.com/c/1648176/18/base/task/promise/dependent_list.h#61

Change-Id: I91d69844e9a83899f3a9453f0e060a9779b40028
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810960Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Reviewed-by: default avatarAlex Clarke <alexclarke@chromium.org>
Commit-Queue: Carlos Caballero <carlscab@google.com>
Cr-Commit-Position: refs/heads/master@{#714395}
parent a62cd235
......@@ -6,10 +6,12 @@
#define BASE_TASK_PROMISE_DEPENDENT_LIST_H_
#include <atomic>
#include <cstddef>
#include <cstdint>
#include <type_traits>
#include "base/base_export.h"
#include "base/compiler_specific.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/scoped_refptr.h"
......@@ -57,8 +59,6 @@ class BASE_EXPORT DependentList {
FAIL_PROMISE_CANCELED,
};
// Align Node on an 8-byte boundary to ensure the first 3 bits are 0 and can
// be used to store additional state (see static_asserts below).
class BASE_EXPORT ALIGNAS(8) Node {
public:
Node();
......@@ -245,10 +245,24 @@ class BASE_EXPORT DependentList {
NextPowerOfTwo(static_cast<uintptr_t>(State::kLastValue)) - 1;
static constexpr uintptr_t kAllowInsertsBitMask = kStateMask + 1;
static constexpr uintptr_t kHeadMask = ~(kAllowInsertsBitMask | kStateMask);
static constexpr uintptr_t kRequiredNodeAlignment = ~kHeadMask + 1;
// It is really important that Node instances get aligned so that we can store
// state in the lower bits of pointers.
//
// Allocators are required to return memory aligned at least as strictly as
// std::max_align_t but not more, so we can not ask for a bigger alignment
// here otherwise we risk not getting proper alignment from heap allocations.
// Also, to make sure that stack allocations also get properly aligned (used
// currently in tests) we need the ALIGNAS(8) attribute, which is ignored by
// allocators. Thus we need the two following static_asserts.
static_assert(
std::alignment_of<Node>() > kAllowInsertsBitMask,
std::alignment_of<Node>() >= kRequiredNodeAlignment,
"Will not be able to hold the Node* and all the state in a uintptr_t");
static_assert(
std::alignment_of<std::max_align_t>() >= std::alignment_of<Node>(),
"malloc (et al.) will not return memory propery aligned to be able to "
"hold the Node* and all the state in a uintptr_t");
static State ExtractState(uintptr_t data) {
return static_cast<State>(data & kStateMask);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment