Skip to content

Commit

Permalink
[ADT] Fix alignment check in unique_function constructor (#99403)
Browse files Browse the repository at this point in the history
Right now the check fails for any state-capturing lambda since this
expression - `alignof(decltype(StorageUnion.InlineStorage))` - returns 1
for the alignment value and not 4/8 as expected
([MSVC|Clang|GCC](https://godbolt.org/z/eTEdq4xjM)). So this check fails
for pretty much any state-capturing callable we try to store into a
`unique_function` and we take the out-of-line storage path:

\llvm-project\llvm\include\llvm\ADT\FunctionExtras.h,
`UniqueFunctionBase` constructor (line ~266):

```
if (sizeof(CallableT) > InlineStorageSize ||
    alignof(CallableT) > alignof(decltype(StorageUnion.InlineStorage))) {
  // ...
}
```

The fix is simply to use an explicit const variable to store the
alignment value.

There is no easy way to unit-test the fix since inline storage is
considered to be an implementation detail so we shouldn't assume how the
lambda ends up being stored.
  • Loading branch information
kerambyte authored Aug 19, 2024
1 parent 92a8ec7 commit 22b4496
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
6 changes: 4 additions & 2 deletions llvm/include/llvm/ADT/FunctionExtras.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ using EnableIfCallable = std::enable_if_t<std::disjunction<
template <typename ReturnT, typename... ParamTs> class UniqueFunctionBase {
protected:
static constexpr size_t InlineStorageSize = sizeof(void *) * 3;
static constexpr size_t InlineStorageAlign = alignof(void *);

template <typename T, class = void>
struct IsSizeLessThanThresholdT : std::false_type {};
Expand Down Expand Up @@ -161,7 +162,8 @@ template <typename ReturnT, typename... ParamTs> class UniqueFunctionBase {
// provide three pointers worth of storage here.
// This is mutable as an inlined `const unique_function<void() const>` may
// still modify its own mutable members.
alignas(void *) mutable std::byte InlineStorage[InlineStorageSize];
alignas(InlineStorageAlign) mutable std::byte
InlineStorage[InlineStorageSize];
} StorageUnion;

// A compressed pointer to either our dispatching callback or our table of
Expand Down Expand Up @@ -262,7 +264,7 @@ template <typename ReturnT, typename... ParamTs> class UniqueFunctionBase {
bool IsInlineStorage = true;
void *CallableAddr = getInlineStorage();
if (sizeof(CallableT) > InlineStorageSize ||
alignof(CallableT) > alignof(decltype(StorageUnion.InlineStorage))) {
alignof(CallableT) > InlineStorageAlign) {
IsInlineStorage = false;
// Allocate out-of-line storage. FIXME: Use an explicit alignment
// parameter in C++17 mode.
Expand Down
19 changes: 19 additions & 0 deletions llvm/unittests/ADT/FunctionExtrasTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,4 +310,23 @@ class Incomplete {};
Incomplete incompleteFunction() { return {}; }
const Incomplete incompleteFunctionConst() { return {}; }

// Check that we can store a pointer-sized payload inline in the unique_function.
TEST(UniqueFunctionTest, InlineStorageWorks) {
// We do assume a couple of implementation details of the unique_function here:
// - It can store certain small-enough payload inline
// - Inline storage size is at least >= sizeof(void*)
void *ptr;
unique_function<void(void *)> UniqueFunctionWithInlineStorage{
[ptr](void *self) {
auto mid = reinterpret_cast<uintptr_t>(&ptr);
auto beg = reinterpret_cast<uintptr_t>(self);
auto end = reinterpret_cast<uintptr_t>(self) +
sizeof(unique_function<void(void *)>);
// Make sure the address of the captured pointer lies somewhere within
// the unique_function object.
EXPECT_TRUE(mid >= beg && mid < end);
}};
UniqueFunctionWithInlineStorage(&UniqueFunctionWithInlineStorage);
}

} // anonymous namespace

0 comments on commit 22b4496

Please sign in to comment.