Allocator shim skeleton + Linux impl behind a build flag

base/allocator/allocator_shim.h

Index: base/allocator/allocator_shim.h
diff --git a/base/allocator/allocator_shim.h b/base/allocator/allocator_shim.h
new file mode 100644
index 0000000000000000000000000000000000000000..9af44ce08cb27f864be81a1f2a927755db483d04
--- /dev/null
+++ b/base/allocator/allocator_shim.h
@@ -0,0 +1,87 @@
+// Copyright 2016 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.
+
+#ifndef BASE_ALLOCATOR_ALLOCATOR_SHIM_H_
+#define BASE_ALLOCATOR_ALLOCATOR_SHIM_H_
+
+#include <stddef.h>
+
+#include "base/base_export.h"
+
+namespace base {
+namespace allocator {
+
+// Allocator Shim API. Allows to to:
+//  - Configure the behavior of the allocator (what to do on OOM failures).
+//  - Install new hooks (AllocatorDispatch) in the allocator chain.
+
+// When this shim layer is enabled, the route of an allocation is as-follows:
+//
+// [allocator_shim_override_*.h] Intercept malloc() / operator new calls:
+//   The override_* headers define the symbols required to intercept calls to
+//   malloc() and operator new (if not overridden by specific C++ classes).
+//
+// [allocator_shim.cc] Routing allocation calls to the shim:
+//   The headers above route the calls to the internal ShimMalloc(), ShimFree(),
+//   ShimCppNew() etc. methods defined in allocator_shim.cc.
+//   These methods will: (1) forward the allocation call to the front of the
+//   AllocatorDispatch chain. (2) perform security hardenings (e.g., might
+//   call std::new_handler on OOM failure).
+//
+// [allocator_shim_default_dispatch_to_*.cc] The AllocatorDispatch chain:
+//   It is a singly linked list where each element is a struct with function
+//   pointers (|malloc_function|, |free_function|, etc). Normally the chain
+//   consists of a single AllocatorDispatch element, herein called
+//   the "default dispatch", which is statically defined at build time and
+//   ultimately routes the calls to the actual allocator defined by the build
+//   config (tcmalloc, glibc, ...).
+//
+// It is possible to dynamically insert further AllocatorDispatch stages
+// to the front of the chain, for debugging / profiling purposes.
+
+struct AllocatorDispatch {
+  using AllocFn = void*(size_t size, const AllocatorDispatch* self);

[Nico, 2016/03/08 03:08:42]

why make self the last arg?

mention that these functions must all be thread-safe.

are there any performance concerns about the indirection? with the default
setup, it's the same number of indirections as previously, right?

[Primiano Tucci (use gerrit), 2016/03/08 20:54:05]

Is the question:
Q1) why having |self| at all?
Q2) why having |self| instead of |next|?
Q3) why having |self| as last arg and not first?

A1) Because we want to give the dispatch the ability to chain to the next
element in the chain, snoop the result, and proxy it back.
A2) To avoid the useless indirection in the case of the default chain. In that
case |next| is always nullptr, and the default dispatchers don't need to look at
|next| at all, so why bothering dereferencing that for them no reason?
A3) No reason, can move it if you think it's more readable (or there is some
other benefit I am missing).


 
Done.

 
I will find out soon (I did run some trybots two weeks ago, but forgot and lost
the results). Worst case the perf waterfall will tell.
Correct. I don't expect any per hit (last famous words) as, as you say, we
already have these layers of indirection in the current code.

[Nico, 2016/03/09 05:34:25]

Q3 – seems a bit more common (languages with explicit self have it first, and in
c++ the implicit this is also the first arg). But up to you

[Primiano Tucci (use gerrit), 2016/03/09 10:51:59]

Alright, point taken. Moved as 1st argument.

+  using AllocZeroInitializedFn = void*(size_t n,
+                                       size_t size,
+                                       const AllocatorDispatch* self);
+  using AllocAlignedFn = void*(size_t alignment,
+                               size_t size,
+                               const AllocatorDispatch* self);
+  using ReallocFn = void*(void* address,
+                          size_t size,
+                          const AllocatorDispatch* self);
+  using FreeFn = void(void* address, const AllocatorDispatch* self);
+
+  AllocFn* const alloc_function;
+  AllocZeroInitializedFn* const alloc_zero_initialized_function;
+  AllocAlignedFn* const alloc_aligned_function;
+  ReallocFn* const realloc_function;
+  FreeFn* const free_function;
+
+  const AllocatorDispatch* next;
+
+  // |default_dispatch| is statically defined by one (and only one) of the
+  // allocator_shim_default_dispatch_to_*.cc files, depending on the build
+  // configuration.
+  static const AllocatorDispatch default_dispatch;
+};
+
+// When true makes malloc behave like new, w.r.t calling the new_handler if
+// the allocation fails (see set_new_mode() in Windows).
+BASE_EXPORT void SetCallNewHandlerOnMallocFailure(bool value);
+
+// Allocates |size| bytes or returns nullptr. It does NOT call the new_handler,
+// regardless of SetCallNewHandlerOnMallocFailure().
+BASE_EXPORT void* UncheckedAlloc(size_t size);
+
+// Inserts |dispatch| in front of the allocator chain. This method is NOT
+// thread-safe w.r.t concurrent invocations of InsertAllocatorDispatch().
+// The callers have the responsibility of linearizing the changes to the chain
+// (or more likely call these always on the same thread).
+BASE_EXPORT void InsertAllocatorDispatch(AllocatorDispatch* dispatch);

[Nico, 2016/03/08 03:08:42]

removing a dispatch object is not possible?

[Primiano Tucci (use gerrit), 2016/03/08 20:54:05]

I removed it after a discussion that with chrisha:
- none of us has a real use case for the removal. for both of us (future clients
of this API) the decision to install is taken at startup time (either
command-line or finch).
- doing a sane removal (sane: of an arbitrary dispatch) on a singly linked list
will make the code funky, and at that point the malloc() needs a lock, which I'd
really really try to avoid (that would be a candidate for a perf hit).

So, for the tracing case, I perfer adding an if(should_noop) to the dispatch
that will be inserted by --enable-heap-profiling. Rationale: that will make the
heap profiler code slighly slower (still, no locks) but will keep the default
production code lock-free in the malloc fastpath.

The lock-free alternative would be RemoveFirstAllocatorDispatch, which is a bit
of a awkward and potentially dangerous (you have to know for sure the
registration order) API.

Thinking more, I need a RemoveForTesing one, to ensure that the unittests always
clean up after themselves.

+
+}  // namespace allocator
+}  // namespace base
+
+#endif  // BASE_ALLOCATOR_ALLOCATOR_SHIM_H_