Modify allocator_shim to support _aligned_alloc(), _aligned_free().

base/allocator/win_allocator.cc

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This is a simple allocator based on the windows heap.
 
 extern "C" {
 
 HANDLE win_heap;
 
@@ skipping 29 matching lines @@
     win_heap_free(ptr);
     return NULL;
   }
   return HeapReAlloc(win_heap, 0, ptr, size);
 }
 
 size_t win_heap_msize(void* ptr) {
   return HeapSize(win_heap, 0, ptr);
 }
 
+void* win_heap_memalign(size_t alignment, size_t size) {
+  // Reserve enough space to ensure we can align and set aligned_ptr[-1] to the

[jar (doing other things), 2012/07/30 23:04:22]

I think you may want to be cautious about args here.  I don't know who is
calling us, but we'll get in a lot of trouble if we call malloc with bogus
values.

Perhaps:

CHECK_GT(alignment, 0);  // perhaps, > 4 even???
CHECK_EQ(alignment & alignment - 1, 0);
CHECK_LT(alignment, 1024 * 1024 * 1024);

We also want to worry about wrapping.  Maybe:

size_t oversize = size + (alignment - 1) + sizeof(void*);
CHECK_LT(size, oversize);
CHECK_LT(alignment, oversize);

You could probably simplify some of those checks, but it is probably clearer
(and not performance critical) to leave them as explicit checks.

[DaleCurtis, 2012/07/31 00:44:22]

Can you elaborate on the trouble we could face? From the MSDN docs on HeapAlloc,
it sounds like at worst we'll get a NULL ptr back:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa366597

I.e., no worse than a bogus call to win_heap_malloc(). Added some DCHECKs()
though for sanity. I've put most of these checks in allocator_shim though since
_aligned_malloc() guarantees its arguments are validated.

[jar (doing other things), 2012/07/31 03:18:51]

I didn't see where the args were being validated.  As a result, I was concerned
to be sure we do it here.

Example: Call for something that is 0.5GB aligned.
Ask for a total size 3.5GB.

We'll call malloc and ask for a total size of roughly 4 bytes, and well get an
allocation success <oops>.

We'll return to the caller a success, and the caller can then access memory
anywhere in the 3.5GB region that follows.

That is the class of problem I'd rather not see happen.

On 2012/07/31 00:44:22, DaleCurtis wrote:

[DaleCurtis, 2012/07/31 03:39:48]

Gotcha, I thought there might be some weird HeapAlloc behavior I wasn't aware
of. The new patch set adds an overflow check.

Most users will probably be using base::AlignedAlloc which does input validation
as well:
http://git.chromium.org/gitweb/?p=chromium.git;a=blob;f=base/memory/aligned_m...

+  // original allocation for use with win_heap_memalign_free() later.
+  void* ptr = win_heap_malloc(size + (alignment - 1) + sizeof(void*));
+
+  char* aligned_ptr = static_cast<char*>(ptr) + sizeof(void*);
+  aligned_ptr +=
+      alignment - reinterpret_cast<uintptr_t>(aligned_ptr) & (alignment - 1);
+
+  reinterpret_cast<void**>(aligned_ptr)[-1] = ptr;
+  return aligned_ptr;
+}
+
+void win_heap_memalign_free(void* ptr) {
+  win_heap_free(static_cast<void**>(ptr)[-1]);
+}
+
 }  // extern "C"