Allocator shims working on VS2015.

base/allocator/allocator_shim_win.cc

Index: base/allocator/allocator_shim_win.cc
diff --git a/base/allocator/allocator_shim_win.cc b/base/allocator/allocator_shim_win.cc
index b65544f5dd89ad00db76f36f894347a8506e76df..b867d0559790b89afe06f862e5bc44815336f052 100644
--- a/base/allocator/allocator_shim_win.cc
+++ b/base/allocator/allocator_shim_win.cc
@@ -2,29 +2,28 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// The allocator shim is only enabled in Release Static builds.
+// This #if is needed as gyp can't have different compile
+// targets between Debug and Release.
+// TODO(wfh): Remove this once gyp is dead.
+#if defined(ALLOCATOR_SHIM)
+
 #include <limits.h>
 #include <malloc.h>
 #include <new.h>
 #include <windows.h>
 #include <stddef.h>
 
+#include "allocator_shim_win.h"
+
 // This shim make it possible to perform additional checks on allocations
 // before passing them to the Heap functions.
 
-// Heap functions are stripped from libcmt.lib using the prep_libc.py
-// for each object file stripped, we re-implement them here to allow us to
-// perform additional checks:
+// Override heap functions to perform additional checks:
 // 1. Enforcing the maximum size that can be allocated to 2Gb.
-// 2. Calling new_handler if malloc fails.
+// 2. Calling new_handler if malloc fails
 
-extern "C" {
-// We set this to 1 because part of the CRT uses a check of _crtheap != 0
-// to test whether the CRT has been initialized.  Once we've ripped out
-// the allocators from libcmt, we need to provide this definition so that
-// the rest of the CRT is still usable.
-// heapinit.c
-void* _crtheap = reinterpret_cast<void*>(1);
-}
+// See definitions of original functions in ucrt\ucrt_malloc.h

[brucedawson, 2016/03/28 20:38:18]

Do you mean ucrt\corecrt_malloc.h? From c:\Program Files (x86)\Windows
Kits\10\Include?

Do we have the source as well? Would be good to give exact locations for the
header and source files this is based on.

Also, are you overriding all or just some functions from that file? When I've
done this previously I found that you had to override every single function or
else risk link errors if one of the other ones was referenced.

[Will Harris, 2016/03/29 18:23:51]

Yes I updated the comment with the correct header path

I updated all the functions below with the ones they are replacing.

I am only replacing the ones that need to be replaced and for all the tests to
pass.

 
 namespace base {
 namespace allocator {
@@ -38,33 +37,18 @@ const size_t kWindowsPageSize = 4096;
 const size_t kMaxWindowsAllocation = INT_MAX - kWindowsPageSize;
 int new_mode = 0;
 
-// VS2013 crt uses the process heap as its heap, so we do the same here.
-// See heapinit.c in VS CRT sources.
-bool win_heap_init() {
-  // Set the _crtheap global here.  THis allows us to offload most of the
-  // memory management to the CRT, except the functions we need to shim.
-  _crtheap = GetProcessHeap();
-  if (_crtheap == NULL)
-    return false;
-
-  ULONG enable_lfh = 2;
-  // NOTE: Setting LFH may fail.  Vista already has it enabled.
-  //       And under the debugger, it won't use LFH.  So we
-  //       ignore any errors.
-  HeapSetInformation(_crtheap, HeapCompatibilityInformation, &enable_lfh,
-                     sizeof(enable_lfh));
-
-  return true;
+inline HANDLE get_heap_handle() {
+  return reinterpret_cast<HANDLE>(_get_heap_handle());
 }
 
 void* win_heap_malloc(size_t size) {
   if (size < kMaxWindowsAllocation)
-    return HeapAlloc(_crtheap, 0, size);
-  return NULL;
+    return HeapAlloc(get_heap_handle(), 0, size);
+  return nullptr;
 }
 
 void win_heap_free(void* size) {
-  HeapFree(_crtheap, 0, size);
+  HeapFree(get_heap_handle(), 0, size);
 }
 
 void* win_heap_realloc(void* ptr, size_t size) {
@@ -72,15 +56,11 @@ void* win_heap_realloc(void* ptr, size_t size) {
     return win_heap_malloc(size);
   if (!size) {
     win_heap_free(ptr);
-    return NULL;
+    return nullptr;
   }
   if (size < kMaxWindowsAllocation)
-    return HeapReAlloc(_crtheap, 0, ptr, size);
-  return NULL;
-}
-
-void win_heap_term() {
-  _crtheap = NULL;
+    return HeapReAlloc(get_heap_handle(), 0, ptr, size);
+  return nullptr;
 }
 
 // Call the new handler, if one has been set.
@@ -93,11 +73,10 @@ inline bool call_new_handler(bool nothrow, size_t size) {
     return false;
   // Since exceptions are disabled, we don't really know if new_handler
   // failed.  Assume it will abort if it fails.
-  return nh(size);
+  return nh(size) ? true : false;
 #else
 #error "Exceptions in allocator shim are not supported!"
 #endif  // defined(_HAS_EXCEPTIONS) && !_HAS_EXCEPTIONS
-  return false;
 }
 
 // Implement a C++ style allocation, which always calls the new_handler
@@ -116,35 +95,7 @@ inline void* generic_cpp_alloc(size_t size, bool nothrow) {
 
 }  // namespace
 
-// new.cpp
-void* operator new(size_t size) {
-  return generic_cpp_alloc(size, false);

[Primiano Tucci (use gerrit), 2016/03/29 18:46:06]

Out of curiosity, why are you not re-defining these? Is it because you rely on
them calling malloc/free under the hoods? Wouldn't it be safer to still redefine
them?

[Will Harris, 2016/03/29 18:56:20]

yes they call down into malloc. It seems MS put more effort in with the uCRT to
support replacing malloc cleanly - see comment on lines 6/7 of
ucrt\heap\malloc_base.cpp for evidence of this.

-}
-
-// delete.cpp
-void operator delete(void* p) throw() {
-  free(p);
-}
-
-// new2.cpp
-void* operator new[](size_t size) {
-  return generic_cpp_alloc(size, false);
-}
-
-// delete2.cpp
-void operator delete[](void* p) throw() {
-  free(p);
-}
-
-// newopnt.cpp
-void* operator new(size_t size, const std::nothrow_t& nt) {
-  return generic_cpp_alloc(size, true);
-}
-
-// newaopnt.cpp
-void* operator new[](size_t size, const std::nothrow_t& nt) {
-  return generic_cpp_alloc(size, true);
-}
+extern "C" {
 
 // This function behaves similarly to MSVC's _set_new_mode.
 // If flag is 0 (default), calls to malloc will behave normally.
@@ -152,7 +103,10 @@ void* operator new[](size_t size, const std::nothrow_t& nt) {
 // and the std_new_handler will be invoked on failure.
 // Returns the previous mode.
 // new_mode.cpp
-int _set_new_mode(int flag) throw() {
+int _set_new_mode(int flag) {
+  // The MS CRT calls this function early on in startup, so this serves as a low
+  // overhead proof that the allocator shim is in place for this process.
+  base::allocator::g_is_win_shim_layer_initialized = true;
   int old_mode = new_mode;
   new_mode = flag;
   return old_mode;
@@ -163,9 +117,8 @@ int _query_new_mode() {
   return new_mode;
 }
 
-extern "C" {
 // malloc.c
-void* malloc(size_t size) {
+__declspec(restrict) __declspec(allocator) void* malloc(size_t size) {
   void* ptr;
   for (;;) {
     ptr = win_heap_malloc(size);
@@ -188,7 +141,8 @@ void free(void* p) {
 }
 
 // realloc.c
-void* realloc(void* ptr, size_t size) {
+__declspec(restrict) __declspec(allocator) void* realloc(void* ptr,
+                                                         size_t size) {
   // Webkit is brittle for allocators that return NULL for malloc(0).  The
   // realloc(0, 0) code path does not guarantee a non-NULL return, so be sure
   // to call malloc for this case.
@@ -210,120 +164,21 @@ void* realloc(void* ptr, size_t size) {
   return new_ptr;
 }
 
-// heapinit.c
-intptr_t _get_heap_handle() {
-  return reinterpret_cast<intptr_t>(_crtheap);
-}
-
-// heapinit.c
-int _heap_init() {
-  base::allocator::g_is_win_shim_layer_initialized = true;
-  return win_heap_init() ? 1 : 0;
-}
-
-// heapinit.c
-void _heap_term() {
-  win_heap_term();
-}
-
 // calloc.c
-void* calloc(size_t n, size_t elem_size) {
+__declspec(restrict) __declspec(allocator) void* calloc(size_t n,
+                                                        size_t elem_size) {
   // Overflow check.
   const size_t size = n * elem_size;
   if (elem_size != 0 && size / elem_size != n)
-    return NULL;
+    return nullptr;
 
   void* result = malloc(size);
-  if (result != NULL) {
+  if (result) {
     memset(result, 0, size);

[Primiano Tucci (use gerrit), 2016/03/29 18:46:07]

I wonder if implementing this as WinHeapAlloc(HEAP_ZERO_MEMORY) would be any
faster for larger allocations.
On linux calloc(large_size) is usually short-circuited on a mmap(), which gives
the guarantee that the memory is zero-initialized (typically the kernel has a
pool of alredy laundered pages)  avoiding the memset

[brucedawson, 2016/03/29 18:51:25]

I don't think we can use HeapAlloc(HEAP_ZERO_MEMORY) because then we have an
alloc/free mismatch. Or at least we'd have to coordinate with the DrMemory team
to avoid having this reported as a mismatch.

That is, while the CRT heap does use the Windows heap under the hood in release
builds, DrMemory still reports mismatches if you allocate with one family and
free with the other.

[Will Harris, 2016/03/29 18:56:20]

I'm leaving the inners of the functions the same for this CL, but if you have
suggestions they can be addressed in future CL.

From my testing, the working set doesn't actually get a 2Gb allocation anyways,
so I think windows is doing some optimization here anyway...

[Primiano Tucci (use gerrit), 2016/03/29 19:30:03]

Looks like brucedawson@ answered here and it seems non trivial.

 
Yup, the same happens on linux. The optimization lies in the fact that when you
know that the memory is zero-initialized anywyas you can skip the memset.
Whatever, on the other side I don't have any evidence on how much do we rely on
calloc(large-size) on fast-paths and even if it was the case this is not slower
than what it used to be on MSVS2013 shim

   }
   return result;
 }
 
-// recalloc.c
-void* _recalloc(void* p, size_t n, size_t elem_size) {
-  if (!p)
-    return calloc(n, elem_size);
-
-  // This API is a bit odd.
-  // Note: recalloc only guarantees zeroed memory when p is NULL.
-  //   Generally, calls to malloc() have padding.  So a request
-  //   to malloc N bytes actually malloc's N+x bytes.  Later, if
-  //   that buffer is passed to recalloc, we don't know what N
-  //   was anymore.  We only know what N+x is.  As such, there is
-  //   no way to know what to zero out.
-  const size_t size = n * elem_size;
-  if (elem_size != 0 && size / elem_size != n)
-    return NULL;
-  return realloc(p, size);
-}
-
-// calloc_impl.c
-void* _calloc_impl(size_t n, size_t size) {
-  return calloc(n, size);
-}
-
-#ifndef NDEBUG
-#undef malloc
-#undef free
-#undef calloc
-
-static int error_handler(int reportType) {
-  switch (reportType) {
-    case 0:  // _CRT_WARN
-      __debugbreak();
-      return 0;
-
-    case 1:  // _CRT_ERROR
-      __debugbreak();
-      return 0;
-
-    case 2:  // _CRT_ASSERT
-      __debugbreak();
-      return 0;
-  }
-  char* p = NULL;
-  *p = '\0';
-  return 0;
-}
-
-int _CrtDbgReport(int reportType,
-                  const char*,
-                  int,
-                  const char*,
-                  const char*,
-                  ...) {
-  return error_handler(reportType);
-}
-
-int _CrtDbgReportW(int reportType,
-                   const wchar_t*,
-                   int,
-                   const wchar_t*,
-                   const wchar_t*,
-                   ...) {
-  return error_handler(reportType);
-}
-
-int _CrtSetReportMode(int, int) {
-  return 0;
-}
-
-void* _malloc_dbg(size_t size, int, const char*, int) {
-  return malloc(size);
-}
-
-void* _realloc_dbg(void* ptr, size_t size, int, const char*, int) {
-  return realloc(ptr, size);
-}
-
-void _free_dbg(void* ptr, int) {
-  free(ptr);
-}
-
-void* _calloc_dbg(size_t n, size_t size, int, const char*, int) {
-  return calloc(n, size);
-}
-#endif  // NDEBUG
-
 }  // extern C
+
+#endif  // defined(ALLOCATOR_SHIM)