Skip to content

Commit 85b7457

Browse files
Tamir Dubersteinfacebook-github-bot
Tamir Duberstein
authored andcommitted
Use posix_memalign on all POSIX platforms
Summary: The feature detection seems to be incorrect, resulting in fallback to the manual implementation, which in turn returns untagged pointers under hwasan. Replace the feature detection with a check on `_POSIX_VERSION` and use `posix_memalign` which is 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 85b7457

File tree

2 files changed

+14
-18
lines changed

2 files changed

+14
-18
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: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -183,36 +183,31 @@ using ssize_t = ptrdiff_t;
183183
* alignment.
184184
*/
185185
#if defined(_MSC_VER)
186-
#include <malloc.h>
186+
#include <cstdlib> // For _aligned_malloc and _aligned_free
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__)
191-
#include <stdlib.h> // For posix_memalign and free
192-
inline void* et_apple_aligned_alloc(size_t alignment, size_t size) {
190+
#else
191+
#include <unistd.h> // For _POSIX_VERSION
192+
#if defined(_POSIX_VERSION) && (_POSIX_VERSION >= 200112L)
193+
#include <cstdlib> // For posix_memalign and free
194+
inline void* et_posix_aligned_alloc(size_t alignment, size_t size) {
193195
void* ptr = nullptr;
194196
// The address of the allocated memory must be a multiple of sizeof(void*).
195197
if (alignment < sizeof(void*)) {
196198
alignment = sizeof(void*);
197199
}
198-
if (posix_memalign(
199-
&ptr, alignment, (size + alignment - 1) & ~(alignment - 1)) != 0) {
200+
if (posix_memalign(&ptr, alignment, size) != 0) {
200201
return nullptr;
201202
}
202203
return ptr;
203204
}
204205
#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))
206+
et_posix_aligned_alloc((alignment), (size))
212207
#define ET_ALIGNED_FREE(ptr) free(ptr)
213208
#else
214-
// If the platform doesn't support aligned_alloc, fallback to malloc.
215-
#include <stdint.h>
209+
// If the platform doesn't support posix_memalign, fallback to malloc.
210+
#include <cstdint>
216211
#include <cstdlib>
217212
inline void* et_aligned_malloc(size_t alignment, size_t size) {
218213
// Place to store the offset to the original pointer.
@@ -263,7 +258,7 @@ inline void et_aligned_free(void* ptr) {
263258

264259
#define ET_ALIGNED_ALLOC(alignment, size) et_aligned_malloc((alignment), (size))
265260
#define ET_ALIGNED_FREE(ptr) et_aligned_free(ptr)
266-
261+
#endif
267262
#endif
268263

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

0 commit comments

Comments
 (0)