Skip to content

Commit 6f9764e

Browse files
authored
Replace manual pointer alignment with std::aligned_alloc
Differential Revision: D75553227 Pull Request resolved: #11203
1 parent e78a944 commit 6f9764e

File tree

1 file changed

+10
-56
lines changed

1 file changed

+10
-56
lines changed

extension/data_loader/file_descriptor_data_loader.cpp

Lines changed: 10 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,6 @@ static bool is_power_of_2(size_t value) {
4141
return value > 0 && (value & ~(value - 1)) == value;
4242
}
4343

44-
/**
45-
* Returns the next alignment for a given pointer.
46-
*/
47-
static uint8_t* align_pointer(void* ptr, size_t alignment) {
48-
intptr_t addr = reinterpret_cast<intptr_t>(ptr);
49-
if ((addr & (alignment - 1)) == 0) {
50-
// Already aligned.
51-
return reinterpret_cast<uint8_t*>(ptr);
52-
}
53-
// Bump forward.
54-
addr = (addr | (alignment - 1)) + 1;
55-
return reinterpret_cast<uint8_t*>(addr);
56-
}
5744
} // namespace
5845

5946
FileDescriptorDataLoader::~FileDescriptorDataLoader() {
@@ -139,13 +126,13 @@ namespace {
139126
/**
140127
* FreeableBuffer::FreeFn-compatible callback.
141128
*
142-
* `context` is actually a ptrdiff_t value (not a pointer) that contains the
143-
* offset in bytes between `data` and the actual pointer to free.
129+
* `context` is the original buffer pointer. It is allocated with
130+
* ET_ALIGNED_ALLOC, and must be freed with ET_ALIGNED_FREE.
131+
*
132+
* `data` and `size` are unused.
144133
*/
145134
void FreeSegment(void* context, void* data, ET_UNUSED size_t size) {
146-
ptrdiff_t offset = reinterpret_cast<ptrdiff_t>(context);
147-
ET_DCHECK_MSG(offset >= 0, "Unexpected offset %ld", (long int)offset);
148-
std::free(static_cast<uint8_t*>(data) - offset);
135+
ET_ALIGNED_FREE(context);
149136
}
150137
} // namespace
151138

@@ -173,57 +160,24 @@ Result<FreeableBuffer> FileDescriptorDataLoader::load(
173160
}
174161

175162
// Allocate memory for the FreeableBuffer.
176-
size_t alloc_size = size;
177-
if (alignment_ > alignof(std::max_align_t)) {
178-
// malloc() will align to smaller values, but we must manually align to
179-
// larger values.
180-
alloc_size += alignment_;
181-
}
182-
void* buffer = std::malloc(alloc_size);
183-
if (buffer == nullptr) {
163+
void* aligned_buffer = ET_ALIGNED_ALLOC(alignment_, size);
164+
if (aligned_buffer == nullptr) {
184165
ET_LOG(
185166
Error,
186-
"Reading from %s at offset %zu: malloc(%zd) failed",
167+
"Reading from %s at offset %zu: ET_ALIGNED_ALLOC(%zd) failed",
187168
file_descriptor_uri_,
188169
offset,
189170
size);
190171
return Error::MemoryAllocationFailed;
191172
}
192173

193-
// Align.
194-
void* aligned_buffer = align_pointer(buffer, alignment_);
195-
196-
// Assert that the alignment didn't overflow the buffer.
197-
ET_DCHECK_MSG(
198-
reinterpret_cast<uintptr_t>(aligned_buffer) + size <=
199-
reinterpret_cast<uintptr_t>(buffer) + alloc_size,
200-
"aligned_buffer %p + size %zu > buffer %p + alloc_size %zu",
201-
aligned_buffer,
202-
size,
203-
buffer,
204-
alloc_size);
205-
206174
auto err = load_into(offset, size, segment_info, aligned_buffer);
207175
if (err != Error::Ok) {
208-
// Free `buffer`, which is what malloc() gave us, not `aligned_buffer`.
209-
std::free(buffer);
176+
ET_ALIGNED_FREE(aligned_buffer);
210177
return err;
211178
}
212179

213-
// We can't naively free this pointer, since it may not be what malloc() gave
214-
// us. Pass the offset to the real buffer as context. This is the number of
215-
// bytes that need to be subtracted from the FreeableBuffer::data() pointer to
216-
// find the actual pointer to free.
217-
return FreeableBuffer(
218-
aligned_buffer,
219-
size,
220-
FreeSegment,
221-
/*free_fn_context=*/
222-
reinterpret_cast<void*>(
223-
// Using signed types here because it will produce a signed ptrdiff_t
224-
// value, though for us it will always be non-negative.
225-
reinterpret_cast<intptr_t>(aligned_buffer) -
226-
reinterpret_cast<intptr_t>(buffer)));
180+
return FreeableBuffer(aligned_buffer, size, FreeSegment, aligned_buffer);
227181
}
228182

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

0 commit comments

Comments
 (0)