memory: Add memory-debug scribbling when tcmalloc is disabled and not compiled for optimization

bazel/BUILD

@@ -89,6 +89,16 @@ config_setting(
     values = {"define": "ENVOY_CONFIG_COVERAGE=1"},
 )
 
+config_setting(
+    name = "tsan_build",
+    values = {"define": "ENVOY_CONFIG_TSAN=1"},
+)
+
+config_setting(
+    name = "asan_build",
+    values = {"define": "ENVOY_CONFIG_ASAN=1"},
+)
+
 config_setting(
     name = "disable_tcmalloc",
     values = {"define": "tcmalloc=disabled"},

bazel/envoy_build_system.bzl

@@ -67,6 +67,12 @@ def envoy_copts(repository, test = False):
            }) + select({
                repository + "//bazel:disable_tcmalloc": ["-DABSL_MALLOC_HOOK_MMAP_DISABLE"],
                "//conditions:default": ["-DTCMALLOC"],
+           }) + select({
+               repository + "//bazel:tsan_build": ["-DENVOY_DISABLE_MEMDEBUG=1"],
+               "//conditions:default": [],
+           }) + select({
+               repository + "//bazel:asan_build": ["-DENVOY_DISABLE_MEMDEBUG=1"],
+               "//conditions:default": [],
            }) + select({
                repository + "//bazel:disable_signal_trace": [],
                "//conditions:default": ["-DENVOY_HANDLE_SIGNALS"],

bazel/external/apache_thrift.BUILD

@@ -10,7 +10,11 @@ genrule(
     srcs = src_files,
     outs = [f.replace("src/", "thrift/") for f in src_files],
     cmd = "\n".join(
-        ["mkdir -p $$(dirname $(location %s)) && cp $(location %s) $(location :%s)" % (f, f, f.replace("src/", "thrift/")) for f in src_files],
+        ["mkdir -p $$(dirname $(location %s)) && cp $(location %s) $(location :%s)" % (
+            f,
+            f,
+            f.replace("src/", "thrift/"),
+        ) for f in src_files],
     ),
     visibility = ["//visibility:private"],
 )

bazel/external/http-parser.BUILD

@@ -2,7 +2,10 @@ licenses(["notice"])  # Apache 2
 
 cc_library(
     name = "http_parser",
-    srcs = ["http_parser.c", "http_parser.h"],
+    srcs = [
+        "http_parser.c",
+        "http_parser.h",
+    ],
     hdrs = ["http_parser.h"],
     includes = ["."],
     visibility = ["//visibility:public"],

bazel/external/libcircllhist.BUILD

@@ -6,10 +6,10 @@ cc_library(
     hdrs = [
         "src/circllhist.h",
     ],
-    includes = ["src"],
-    visibility = ["//visibility:public"],
     copts = select({
         "@envoy//bazel:windows_x86_64": ["-DWIN32"],
         "//conditions:default": [],
     }),
+    includes = ["src"],
+    visibility = ["//visibility:public"],
 )

bazel/external/libprotobuf_mutator.BUILD

@@ -2,10 +2,17 @@ licenses(["notice"])  # Apache 2
 
 cc_library(
     name = "libprotobuf_mutator",
-    srcs = glob(["src/**/*.cc", "src/**/*.h", "port/protobuf.h"], exclude = ["**/*_test.cc"]),
+    srcs = glob(
+        [
+            "src/**/*.cc",
+            "src/**/*.h",
+            "port/protobuf.h",
+        ],
+        exclude = ["**/*_test.cc"],
+    ),
     hdrs = ["src/libfuzzer/libfuzzer_macro.h"],
-    includes = ["."],
     include_prefix = "libprotobuf_mutator",
-    deps = ["//external:protobuf"],
+    includes = ["."],
     visibility = ["//visibility:public"],
+    deps = ["//external:protobuf"],
 )

bazel/external/twitter_common_rpc.BUILD

@@ -7,7 +7,7 @@ py_library(
     ]),
     visibility = ["//visibility:public"],
     deps = [
-        "@com_github_twitter_common_lang//:twitter_common_lang",
         "@com_github_twitter_common_finagle_thrift//:twitter_common_finagle_thrift",
+        "@com_github_twitter_common_lang//:twitter_common_lang",
     ],
 )

source/common/common/BUILD

@@ -282,6 +282,7 @@ envoy_cc_library(
     deps = [
         ":assert_lib",
         ":logger_lib",
+        "//source/common/memory:align_lib",
         "//source/common/stats:stats_options_lib",
     ],
 )

source/common/common/block_memory_hash_set.h

@@ -10,6 +10,7 @@
 #include "common/common/assert.h"
 #include "common/common/fmt.h"
 #include "common/common/logger.h"
+#include "common/memory/align.h"
 
 #include "absl/strings/string_view.h"
 
@@ -312,13 +313,7 @@ template <class Value> class BlockMemoryHashSet : public Logger::Loggable<Logger
     return std::max(alignof(Cell), std::max(alignof(uint32_t), alignof(Control)));
   }
 
-  static uint64_t align(uint64_t size) {
-    const uint64_t alignment = calculateAlignment();
-    // Check that alignment is a power of 2:
-    // http://www.graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2
-    RELEASE_ASSERT((alignment > 0) && ((alignment & (alignment - 1)) == 0), "");
-    return (size + alignment - 1) & ~(alignment - 1);
-  }
+  static uint64_t align(uint64_t size) { return Memory::align(size, calculateAlignment()); }
 
   /**
    * Computes the byte offset of a cell into cells_. This is not

source/common/memory/BUILD

@@ -8,9 +8,22 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "align_lib",
+    hdrs = ["align.h"],
+)
+
+envoy_cc_library(
+    name = "debug_lib",
+    srcs = ["debug.cc"],
+    hdrs = ["debug.h"],
+    deps = [":align_lib"],
+)
+
 envoy_cc_library(
     name = "stats_lib",
     srcs = ["stats.cc"],
     hdrs = ["stats.h"],
     tcmalloc_dep = 1,
+    deps = [":debug_lib"],
 )

source/common/memory/align.h

@@ -0,0 +1,17 @@
+#pragma once
+
+#include <cassert>
+#include <cstdint>
+
+namespace Envoy {
+namespace Memory {
+
+inline uint64_t align(uint64_t size, uint64_t alignment) {
+  // Check that alignment is a power of 2:
+  // http://www.graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2
+  assert((alignment > 0) && ((alignment & (alignment - 1)) == 0));
+  return (size + alignment - 1) & ~(alignment - 1);
+}
+
+} // namespace Memory
+} // namespace Envoy

source/common/memory/debug.cc

@@ -0,0 +1,118 @@
+// Very simple memory debugging overrides for operator new/delete, to
+// help us quickly find simple memory violations:
+//   1. Double destruct
+//   2. Read before write (via scribbling)
+//   3. Read after delete (via scribbling)
+//
+// Note that valgrind does all of this much better, but is too slow to run all
+// the time. asan does read-after-delete detection but not read-before-init
+// detection. See
+// https://clang.llvm.org/docs/AddressSanitizer.html#initialization-order-checking
+// for more details.
+
+// Principle of operation: add 8 bytes to every allocation. The first
+// 4 bytes are a marker (LiveMarker1 or DeadMarker1). The next 4
+// bytes are used to store size of the allocation, which helps us
+// know how many bytes to scribble when we free.
+//
+// This code was adapted from mod_pagespeed, and adapted for Envoy
+// style. Original source:
+// https://github.com/apache/incubator-pagespeed-mod/blob/master/pagespeed/kernel/base/mem_debug.cc
+
+// We keep a global count of bytes allocated so that memory-consumption tests
+// work with memory debugging. Put another way, if we disable tcmalloc when
+// compiling for debug, we want the memory-debugging tests to work, otherwise we
+// can't debug them.
+#include "common/memory/debug.h"
+
+#include <atomic>
+#include <cassert> // don't use Envoy ASSERT as it may allocate memory.
+#include <cstdlib>
+
+#include "common/memory/align.h"
+
+static std::atomic<int64_t> bytes_allocated(0);
+
+namespace Envoy {
+namespace Memory {
+
+// We always provide the constructor entry-point to be called to force-load this
+// module, regardless of compilation mode. If the #ifdefs line as required
+// below, then it will also override operator new/delete in various flavors so
+// that we can debug memory issues.
+Debug::Debug() = default;
+
+// We also provide the bytes-loaded counter, though this will return 0 when
+// memory-debugging is not compiled in.
+uint64_t Debug::bytesUsed() { return uint64_t(bytes_allocated); }
+
+} // namespace Memory
+} // namespace Envoy
+
+#ifdef MEMORY_DEBUG_ENABLED
+
+namespace {
+
+constexpr uint32_t LiveMarker1 = 0xfeedface;         // first 4 bytes after alloc
+constexpr uint64_t LiveMarker2 = 0xfeedfacefeedface; // first 4 bytes after alloc
+constexpr uint32_t DeadMarker1 = 0xabacabff;         // first 4 bytes after free
+constexpr uint64_t DeadMarker2 = 0xdeadbeefdeadbeef; // overwrites the 'size' field on free
+constexpr uint64_t Overhead = sizeof(uint64_t);      // number of extra bytes to alloc
+
+// Writes scribble_word over the block of memory starting at ptr and extending
+// size bytes.
+inline void scribble(void* ptr, uint64_t rounded_size, uint64_t scribble_word) {
+  assert((rounded_size % Overhead) == 0);
+  uint64_t num_uint64s = rounded_size / sizeof(uint64_t);
+  uint64_t* p = static_cast<uint64_t*>(ptr);
+  for (uint64_t i = 0; i < num_uint64s; ++i, ++p) {
+    *p = scribble_word;
+  }
+}
+
+// Replacement allocator, which prepends an 8-byte overhead where we write the
+// size, and scribbles over the returned payload so that callers assuming
+// malloced memory is 0 get data that, when interpreted as pointers, will SEGV,
+// and that will be easily seen in the debugger (0xfeedface pattern).
+void* debugMalloc(uint64_t size) {
+  assert(size <= 0xffffffff); // For now we store the original size in a uint32_t.
+  uint64_t rounded = Envoy::Memory::align(size, Overhead);
+  bytes_allocated += rounded;
+  uint32_t* marker = static_cast<uint32_t*>(::malloc(rounded + Overhead));
+  assert(marker != NULL);
+  marker[0] = LiveMarker1;
+  marker[1] = size;
+  uint32_t* ret = marker + 2;
+  scribble(ret, rounded, LiveMarker2);
+  return reinterpret_cast<char*>(marker) + Overhead;
+}
+
+// free() implementation corresponding to debugMalloc(), which pulls out
+// The size from the 8 bytes prior to the payload, so it can know how much
+// 0xdeadbeef to scribble over the freed memory before calling actual free().
+void debugFree(void* ptr) {
+  if (ptr != NULL) {
+    char* alloced_ptr = static_cast<char*>(ptr) - Overhead;
+    uint32_t* marker = reinterpret_cast<uint32_t*>(alloced_ptr);
+    uint32_t size = marker[1];
+    uint64_t rounded = Envoy::Memory::align(size, Overhead);
+    bytes_allocated -= rounded;
+    scribble(ptr, rounded, DeadMarker2);
+    assert(LiveMarker1 == marker[0]);
+    marker[0] = DeadMarker1;
+    marker[1] = DeadMarker2 & 0xffffffff;
+    ::free(marker);
+  }
+}
+
+} // namespace
+
+void* operator new(uint64_t size) { return debugMalloc(size); }
+void operator delete(void* ptr) noexcept { debugFree(ptr); }
+void operator delete(void* ptr, size_t) noexcept { debugFree(ptr); }
+
+void* operator new[](size_t size) { return debugMalloc(size); }
+void operator delete[](void* ptr) noexcept { debugFree(ptr); }
+void operator delete[](void* ptr, size_t) noexcept { debugFree(ptr); }
+
+#endif // MEMORY_DEBUG_ENABLED

source/common/memory/debug.h

@@ -0,0 +1,48 @@
+#pragma once
+
+#include <cstdint>
+
+namespace Envoy {
+namespace Memory {
+
+class Debug {
+public:
+  // Instantiate to force-load the memory debugging module. This is called
+  // whether or not memory-debugging is enabled, which is controlled by ifdefs
+  // in mem_debug.cc.
+  Debug();
+
+  // Returns the number of bytes used -- if memory debugging is enabled.
+  // Otherwise returns 0.
+  static uint64_t bytesUsed();
+};
+
+// Centralized ifdef logic to determine whether this compile has memory
+// debugging. This is exposed in the header file for testing.
+
+// We don't run memory debugging for optimizd builds to avoid impacting
+// production performance.
+#ifndef NDEBUG
+
+// We can't run memory debugging with tcmalloc due to conflicts with
+// overriding operator new/delete. Note tcmalloc allows installation
+// of a malloc hook (MallocHook::AddNewHook(&tcmallocHook)) e.g.
+// tcmallocHook(const void* ptr, size_t size). I tried const_casting ptr
+// and scribbling over it, but this results in a SEGV in grpc and the
+// internals of gtest.
+//
+// And in any case, you can't use the tcmalloc hooks to do free-scribbling
+// as it does not pass in the size to the corresponding free hook. See
+// gperftools/malloc_hook.h for details.
+//
+// We also must disable memory debugging for tsan/asan builds, as they also
+// need to override operator new/delete.
+#if !defined(TCMALLOC) && !defined(ENVOY_DISABLE_MEMDEBUG)
+
+#define MEMORY_DEBUG_ENABLED 1
+
+#endif // !TCMALLOC && !ENVOY_DISABLE_MEMDEBUG
+#endif // !NDEBUG
+
+} // namespace Memory
+} // namespace Envoy

source/common/memory/stats.cc

@@ -45,10 +45,12 @@ uint64_t Stats::totalPageHeapUnmapped() {
 
 #else
 
+#include "common/memory/debug.h"
+
 namespace Envoy {
 namespace Memory {
 
-uint64_t Stats::totalCurrentlyAllocated() { return 0; }
+uint64_t Stats::totalCurrentlyAllocated() { return Debug::bytesUsed(); }
 uint64_t Stats::totalThreadCacheBytes() { return 0; }
 uint64_t Stats::totalCurrentlyReserved() { return 0; }
 uint64_t Stats::totalPageHeapUnmapped() { return 0; }

source/exe/BUILD

@@ -57,6 +57,7 @@ envoy_cc_library(
     ],
     deps = [
         ":envoy_main_common_lib",
+        "//source/common/memory:debug_lib",
     ] + envoy_cc_platform_dep("platform_impl_lib"),
 )
 

source/exe/main.cc

@@ -1,3 +1,5 @@
+#include "common/memory/debug.h"
+
 #include "exe/main_common.h"
 
 #include "absl/debugging/symbolize.h"
@@ -12,6 +14,7 @@
  * after setting up command line options.
  */
 int main(int argc, char** argv) {
+  Envoy::Memory::Debug mem_debug;
 #ifndef __APPLE__
   // absl::Symbolize mostly works without this, but this improves corner case
   // handling, such as running in a chroot jail.

test/BUILD

@@ -27,6 +27,7 @@ envoy_cc_test_library(
         "//source/common/common:logger_lib",
         "//source/common/common:thread_lib",
         "//source/common/event:libevent_lib",
+        "//source/common/memory:debug_lib",
         "//test/mocks/access_log:access_log_mocks",
         "//test/test_common:environment_lib",
         "//test/test_common:global_lib",