allocator shim: Add windows _msize interception

Currently, the Windows allocator shims do not hook _msize() causing
sporadic crashes when GWP-ASan is enabled. This occurs because sqlite3
uses _msize() and the native implementation does not properly handle
allocations not returned by the native allocator.

Furthermore, the current (unused) implementation of
WinHeapGetSizeEstimate() seems to be an artifact of the original
implementation from crrev.com/2163783003. It incorrectly increases the
size estimate returned by the native allocator, which can cause
exceptions in sqlite3 (there are routines that will read the _msize of
an allocation and assume the returned size forms a safe bound.) I
changed this routine to just return the allocation size returned by the
native allocator.

Bug: 909720
Change-Id: Ie8975053992161cdc3e447f75733345f0a142978
Reviewed-on: https://chromium-review.googlesource.com/c/1354219
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612051}

base/allocator/allocator_shim_override_ucrt_symbols_win.h

@@ -65,6 +65,11 @@ __declspec(restrict) void* calloc(size_t n, size_t size) {
   return ShimCalloc(n, size, nullptr);
 }
 
+// _msize() is the Windows equivalent of malloc_size().
+size_t _msize(void* memblock) {
+  return ShimGetSizeEstimate(memblock, nullptr);
+}
+
 // The symbols
 //   * __acrt_heap
 //   * __acrt_initialize_heap
@@ -89,12 +94,4 @@ intptr_t _get_heap_handle(void) {
   return reinterpret_cast<intptr_t>(__acrt_heap);
 }
 
-// The default dispatch translation unit has to define also the following
-// symbols (unless they are ultimately routed to the system symbols):
-//   void malloc_stats(void);
-//   int mallopt(int, int);
-//   struct mallinfo mallinfo(void);
-//   size_t malloc_size(void*);
-//   size_t malloc_usable_size(const void*);
-
 }  // extern "C"

base/allocator/allocator_shim_unittest.cc

@@ -54,6 +54,11 @@ namespace {
 using testing::MockFunction;
 using testing::_;
 
+// Special sentinel values used for testing GetSizeEstimate() interception.
+const char kTestSizeEstimateData[] = "test_value";
+constexpr void* kTestSizeEstimateAddress = (void*)kTestSizeEstimateData;
+constexpr size_t kTestSizeEstimate = 1234;
+
 class AllocatorShimTest : public testing::Test {
  public:
   static const size_t kMaxSizeTracked = 2 * base::kSystemPageSize;
@@ -131,6 +136,9 @@ class AllocatorShimTest : public testing::Test {
   static size_t MockGetSizeEstimate(const AllocatorDispatch* self,
                                     void* address,
                                     void* context) {
+    // Special testing values for GetSizeEstimate() interception.
+    if (address == kTestSizeEstimateAddress)
+      return kTestSizeEstimate;
     return self->next->get_size_estimate_function(self->next, address, context);
   }
 
@@ -460,6 +468,24 @@ TEST_F(AllocatorShimTest, NewHandlerConcurrency) {
 TEST_F(AllocatorShimTest, ShimReplacesCRTHeapWhenEnabled) {
   ASSERT_NE(::GetProcessHeap(), reinterpret_cast<HANDLE>(_get_heap_handle()));
 }
+
+TEST_F(AllocatorShimTest, ShimReplacesMsizeWhenEnabled) {
+  InsertAllocatorDispatch(&g_mock_dispatch);
+  EXPECT_EQ(_msize(kTestSizeEstimateAddress), kTestSizeEstimate);
+  RemoveAllocatorDispatchForTesting(&g_mock_dispatch);
+}
+
+TEST_F(AllocatorShimTest, ShimDoesntChangeMsizeWhenEnabled) {
+  void* alloc = malloc(16);
+  size_t sz = _msize(alloc);
+  EXPECT_GE(sz, 16U);
+
+  InsertAllocatorDispatch(&g_mock_dispatch);
+  EXPECT_EQ(_msize(alloc), sz);
+  RemoveAllocatorDispatchForTesting(&g_mock_dispatch);
+
+  free(alloc);
+}
 #endif  // defined(OS_WIN) && BUILDFLAG(USE_ALLOCATOR_SHIM)
 
 }  // namespace

base/allocator/winheap_stubs_win.cc

@@ -58,21 +58,7 @@ size_t WinHeapGetSizeEstimate(void* ptr) {
   if (!ptr)
     return 0;
 
-  // Get the user size of the allocation.
-  size_t size = HeapSize(get_heap_handle(), 0, ptr);
-
-  // Account for the 8-byte HEAP_HEADER preceding the block.
-  size += 8;
-
-// Round up to the nearest allocation granularity, which is 8 for
-// 32 bit machines, and 16 for 64 bit machines.
-#if defined(ARCH_CPU_64_BITS)
-  const size_t kAllocationGranularity = 16;
-#else
-  const size_t kAllocationGranularity = 8;
-#endif
-
-  return (size + kAllocationGranularity - 1) & ~(kAllocationGranularity - 1);
+  return HeapSize(get_heap_handle(), 0, ptr);
 }
 
 // Call the new handler, if one has been set.