Skip to content

Commit 7d8f2f1

Browse files
Tamir Dubersteinfacebook-github-bot
Tamir Duberstein
authored andcommitted
Use posix_memalign on all non-CRT platforms
Summary: The feature detection seems to be incorrect, resulting in fallback to the manual implementation, which in turn returns untagged pointers under hwasan. Remove this complexity in favor of always using `posix_memalign` which has broad support and available on quite old platforms as well. Remove rounding shenanigans while I'm here; the documentation on ET_ALIGNED_ALLOC already stipulates that the caller is responsible for such rounding. Differential Revision: D75806635
1 parent 1bc36c7 commit 7d8f2f1

File tree

2 files changed

+7
-67
lines changed

2 files changed

+7
-67
lines changed

extension/data_loader/file_descriptor_data_loader.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,10 @@ Result<FreeableBuffer> FileDescriptorDataLoader::load(
164164
if (aligned_buffer == nullptr) {
165165
ET_LOG(
166166
Error,
167-
"Reading from %s at offset %zu: ET_ALIGNED_ALLOC(%zd) failed",
167+
"Reading from %s at offset %zu: ET_ALIGNED_ALLOC(%zd, %zd) failed",
168168
file_descriptor_uri_,
169169
offset,
170+
alignment_,
170171
size);
171172
return Error::MemoryAllocationFailed;
172173
}

runtime/platform/compiler.h

Lines changed: 5 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -185,85 +185,24 @@ using ssize_t = ptrdiff_t;
185185
#if defined(_MSC_VER)
186186
#include <malloc.h>
187187
#define ET_ALIGNED_ALLOC(alignment, size) \
188-
_aligned_malloc(((size + alignment - 1) & ~(alignment - 1)), (alignment))
188+
_aligned_malloc((size), (alignment))
189189
#define ET_ALIGNED_FREE(ptr) _aligned_free(ptr)
190-
#elif defined(__APPLE__)
190+
#else
191191
#include <stdlib.h> // For posix_memalign and free
192-
inline void* et_apple_aligned_alloc(size_t alignment, size_t size) {
192+
inline void* et_posix_aligned_alloc(size_t alignment, size_t size) {
193193
void* ptr = nullptr;
194194
// The address of the allocated memory must be a multiple of sizeof(void*).
195195
if (alignment < sizeof(void*)) {
196196
alignment = sizeof(void*);
197197
}
198-
if (posix_memalign(
199-
&ptr, alignment, (size + alignment - 1) & ~(alignment - 1)) != 0) {
198+
if (posix_memalign(&ptr, alignment, size) != 0) {
200199
return nullptr;
201200
}
202201
return ptr;
203202
}
204203
#define ET_ALIGNED_ALLOC(alignment, size) \
205-
et_apple_aligned_alloc((alignment), (size))
206-
#define ET_ALIGNED_FREE(ptr) free(ptr)
207-
#elif __has_builtin(__builtin_aligned_alloc) || defined(_ISOC11_SOURCE)
208-
// Linux and posix systems that support aligned_alloc and are >= C++17.
209-
#include <cstdlib>
210-
#define ET_ALIGNED_ALLOC(alignment, size) \
211-
::aligned_alloc(alignment, (size + alignment - 1) & ~(alignment - 1))
204+
et_posix_aligned_alloc((alignment), (size))
212205
#define ET_ALIGNED_FREE(ptr) free(ptr)
213-
#else
214-
// If the platform doesn't support aligned_alloc, fallback to malloc.
215-
#include <stdint.h>
216-
#include <cstdlib>
217-
inline void* et_aligned_malloc(size_t alignment, size_t size) {
218-
// Place to store the offset to the original pointer.
219-
size_t offset_size = sizeof(uint16_t);
220-
221-
// Malloc extra space for offset + alignment.
222-
size_t alloc_size = size + offset_size + alignment - 1;
223-
void* ptr = std::malloc(alloc_size);
224-
225-
if (ptr == nullptr) {
226-
// Malloc failed.
227-
return nullptr;
228-
}
229-
230-
uintptr_t addr = reinterpret_cast<uintptr_t>(ptr);
231-
// Align the address past addr + offset_size bytes.
232-
// This provides space to store the offset before the aligned pointer.
233-
addr = addr + offset_size;
234-
uintptr_t aligned_ptr = (addr + alignment - 1) & ~(alignment - 1);
235-
236-
// Check that alignment didn't overflow the buffer.
237-
if (reinterpret_cast<uintptr_t>(aligned_ptr) + size >
238-
reinterpret_cast<uintptr_t>(ptr) + alloc_size) {
239-
std::free(ptr);
240-
return nullptr;
241-
}
242-
243-
// Store the offset to the original pointer.
244-
// Used to free the original allocated buffer.
245-
*(reinterpret_cast<uint16_t*>(aligned_ptr) - 1) =
246-
(uint16_t)(reinterpret_cast<uintptr_t>(aligned_ptr) -
247-
reinterpret_cast<uintptr_t>(ptr));
248-
249-
return reinterpret_cast<uint16_t*>(aligned_ptr);
250-
}
251-
252-
inline void et_aligned_free(void* ptr) {
253-
if (ptr == nullptr) {
254-
return;
255-
}
256-
257-
// Get the original pointer using the offset.
258-
uint16_t* original_ptr = reinterpret_cast<uint16_t*>(
259-
reinterpret_cast<uintptr_t>(ptr) -
260-
*(reinterpret_cast<uint16_t*>(ptr) - 1));
261-
std::free(original_ptr);
262-
}
263-
264-
#define ET_ALIGNED_ALLOC(alignment, size) et_aligned_malloc((alignment), (size))
265-
#define ET_ALIGNED_FREE(ptr) et_aligned_free(ptr)
266-
267206
#endif
268207

269208
// DEPRECATED: Use the non-underscore-prefixed versions instead.

0 commit comments

Comments
 (0)