Skip to content

Commit f7fcce4

Browse files
Tamir Dubersteinfacebook-github-bot
Tamir Duberstein
authored andcommitted
Fix std::aligned_alloc feature detection. (pytorch#11293)
Summary: The current 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 `__cplusplus`. While I'm here, use smart pointers to manage memory where it is simple to do so and remove allocation size rounding where it isn't required. Differential Revision: D75806635
1 parent d7699d6 commit f7fcce4

File tree

3 files changed

+27
-23
lines changed

3 files changed

+27
-23
lines changed

extension/data_loader/file_data_loader.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <cstddef>
1414
#include <cstring>
1515
#include <limits>
16+
#include <memory>
1617

1718
#include <executorch/runtime/platform/compat_unistd.h>
1819
#include <fcntl.h>
@@ -149,7 +150,9 @@ Result<FreeableBuffer> FileDataLoader::load(
149150
}
150151

151152
// Allocate memory for the FreeableBuffer.
152-
void* aligned_buffer = ET_ALIGNED_ALLOC(alignment_, size);
153+
std::unique_ptr<void, void (*)(void*)> aligned_buffer(
154+
ET_ALIGNED_ALLOC(alignment_, size),
155+
[](void* ptr) { ET_ALIGNED_FREE(ptr); });
153156
if (aligned_buffer == nullptr) {
154157
ET_LOG(
155158
Error,
@@ -161,14 +164,14 @@ Result<FreeableBuffer> FileDataLoader::load(
161164
return Error::MemoryAllocationFailed;
162165
}
163166

164-
auto err = load_into(offset, size, segment_info, aligned_buffer);
167+
auto err = load_into(offset, size, segment_info, aligned_buffer.get());
165168
if (err != Error::Ok) {
166-
ET_ALIGNED_FREE(aligned_buffer);
167169
return err;
168170
}
169171

170172
// Pass the aligned_buffer pointer as context to FreeSegment.
171-
return FreeableBuffer(aligned_buffer, size, FreeSegment, aligned_buffer);
173+
void* ptr = aligned_buffer.release();
174+
return FreeableBuffer(ptr, size, FreeSegment, ptr);
172175
}
173176

174177
Result<size_t> FileDataLoader::size() const {

extension/data_loader/file_descriptor_data_loader.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <cstddef>
1414
#include <cstring>
1515
#include <limits>
16+
#include <memory>
1617

1718
#include <fcntl.h>
1819
#include <sys/stat.h>
@@ -160,24 +161,27 @@ Result<FreeableBuffer> FileDescriptorDataLoader::load(
160161
}
161162

162163
// Allocate memory for the FreeableBuffer.
163-
void* aligned_buffer = ET_ALIGNED_ALLOC(alignment_, size);
164+
std::unique_ptr<void, void (*)(void*)> aligned_buffer(
165+
ET_ALIGNED_ALLOC(alignment_, size),
166+
[](void* ptr) { ET_ALIGNED_FREE(ptr); });
164167
if (aligned_buffer == nullptr) {
165168
ET_LOG(
166169
Error,
167-
"Reading from %s at offset %zu: ET_ALIGNED_ALLOC(%zd) failed",
170+
"Reading from %s at offset %zu: ET_ALIGNED_ALLOC(%zd, %zd) failed",
168171
file_descriptor_uri_,
169172
offset,
173+
alignment_,
170174
size);
171175
return Error::MemoryAllocationFailed;
172176
}
173177

174-
auto err = load_into(offset, size, segment_info, aligned_buffer);
178+
auto err = load_into(offset, size, segment_info, aligned_buffer.get());
175179
if (err != Error::Ok) {
176-
ET_ALIGNED_FREE(aligned_buffer);
177180
return err;
178181
}
179182

180-
return FreeableBuffer(aligned_buffer, size, FreeSegment, aligned_buffer);
183+
void* ptr = aligned_buffer.release();
184+
return FreeableBuffer(ptr, size, FreeSegment, ptr);
181185
}
182186

183187
Result<size_t> FileDescriptorDataLoader::size() const {

runtime/platform/compiler.h

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -182,38 +182,36 @@ using ssize_t = ptrdiff_t;
182182
* Note: alignment must be a power of 2 and size must be an integral multiple of
183183
* alignment.
184184
*/
185+
#include <cstdlib>
185186
#if defined(_MSC_VER)
186-
#include <malloc.h>
187-
#define ET_ALIGNED_ALLOC(alignment, size) \
188-
_aligned_malloc(((size + alignment - 1) & ~(alignment - 1)), (alignment))
187+
#define ET_ALIGNED_ALLOC(alignment, size) _aligned_malloc((size), (alignment))
189188
#define ET_ALIGNED_FREE(ptr) _aligned_free(ptr)
190189
#elif defined(__APPLE__)
191-
#include <stdlib.h> // For posix_memalign and free
192190
inline void* et_apple_aligned_alloc(size_t alignment, size_t size) {
193191
void* ptr = nullptr;
194192
// The address of the allocated memory must be a multiple of sizeof(void*).
195193
if (alignment < sizeof(void*)) {
196194
alignment = sizeof(void*);
197195
}
198-
if (posix_memalign(
199-
&ptr, alignment, (size + alignment - 1) & ~(alignment - 1)) != 0) {
196+
if (posix_memalign(&ptr, alignment, size) != 0) {
200197
return nullptr;
201198
}
202199
return ptr;
203200
}
204201
#define ET_ALIGNED_ALLOC(alignment, size) \
205202
et_apple_aligned_alloc((alignment), (size))
206203
#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>
204+
#elif __cplusplus >= 201703L
210205
#define ET_ALIGNED_ALLOC(alignment, size) \
211-
::aligned_alloc(alignment, (size + alignment - 1) & ~(alignment - 1))
212-
#define ET_ALIGNED_FREE(ptr) free(ptr)
206+
std::aligned_alloc(alignment, (size + alignment - 1) & ~(alignment - 1))
207+
#define ET_ALIGNED_FREE(ptr) std::free(ptr)
213208
#else
209+
#if defined(__has_feature) && __has_feature(hwaddress_sanitizer)
210+
#error native aligned allocation is not available on this platform, and the fallback is not compatible with HWASAN.
211+
#endif
212+
214213
// If the platform doesn't support aligned_alloc, fallback to malloc.
215-
#include <stdint.h>
216-
#include <cstdlib>
214+
#include <cstdint>
217215
inline void* et_aligned_malloc(size_t alignment, size_t size) {
218216
// Place to store the offset to the original pointer.
219217
size_t offset_size = sizeof(uint16_t);
@@ -263,7 +261,6 @@ inline void et_aligned_free(void* ptr) {
263261

264262
#define ET_ALIGNED_ALLOC(alignment, size) et_aligned_malloc((alignment), (size))
265263
#define ET_ALIGNED_FREE(ptr) et_aligned_free(ptr)
266-
267264
#endif
268265

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

0 commit comments

Comments
 (0)