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"},
@@ -119,6 +129,11 @@ config_setting(
     values = {"define": "exported_symbols=enabled"},
 )
 
+config_setting(
+    name = "debug_memory",
+    values = {"define": "debug_memory=enabled"},
+)
+
 config_setting(
     name = "enable_perf_annotation",
     values = {"define": "perf_annotation=enabled"},

bazel/envoy_build_system.bzl

@@ -67,6 +67,15 @@ def envoy_copts(repository, test = False):
            }) + select({
                repository + "//bazel:disable_tcmalloc": ["-DABSL_MALLOC_HOOK_MMAP_DISABLE"],
                "//conditions:default": ["-DTCMALLOC"],
+           }) + select({
+               repository + "//bazel:debug_memory": ["-DENVOY_MEMORY_DEBUG_ENABLED=1"],
+               "//conditions:default": [],
+           }) + select({
+               repository + "//bazel:tsan_build": ["-DENVOY_TSAN_BUILD=1"],
+               "//conditions:default": [],
+           }) + select({
+               repository + "//bazel:asan_build": ["-DENVOY_ASAN_BUILD=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"
 
@@ -308,17 +309,10 @@ template <class Value> class BlockMemoryHashSet : public Logger::Loggable<Logger
   };
 
   // It seems like this is an obvious constexpr, but it won't compile as one.
-  static uint64_t calculateAlignment() {
-    return std::max(alignof(Cell), std::max(alignof(uint32_t), alignof(Control)));
-  }
+  static constexpr uint64_t alignment =
+      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<alignment>(size); }
 
   /**
    * Computes the byte offset of a cell into cells_. This is not
@@ -339,7 +333,7 @@ template <class Value> class BlockMemoryHashSet : public Logger::Loggable<Logger
   Cell& getCell(uint32_t cell_index) {
     // Because the key-size is parameteriziable, an array-lookup on sizeof(Cell) does not work.
     char* ptr = reinterpret_cast<char*>(cells_) + cellOffset(cell_index, stats_options_);
-    RELEASE_ASSERT((reinterpret_cast<uint64_t>(ptr) & (calculateAlignment() - 1)) == 0, "");
+    RELEASE_ASSERT((reinterpret_cast<uint64_t>(ptr) & (alignment - 1)) == 0, "");
     return *reinterpret_cast<Cell*>(ptr);
   }
 

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,18 @@
+#pragma once
+
+#include <cstdint>
+#include <type_traits>
+
+namespace Envoy {
+namespace Memory {
+
+template <uint64_t alignment> inline uint64_t align(uint64_t size) {
+  // Check that alignment is a power of 2:
+  // http://www.graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2
+  static_assert((alignment > 0) && ((alignment & (alignment - 1)) == 0),
+                "alignment must be a power of 2");
+  return (size + alignment - 1) & ~(alignment - 1);
+}
+
+} // namespace Memory
+} // namespace Envoy

source/common/memory/debug.cc

@@ -0,0 +1,151 @@
+// 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 DeadMarker). 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 <new>
+
+#include "common/memory/align.h"
+
+static std::atomic<uint64_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 ENVOY_MEMORY_DEBUG_ENABLED
+
+// 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 optimized builds to avoid impacting
+// production performance.
+#ifdef NDEBUG
+#error Memory debugging should not be enabled for production builds
+#endif
+
+// 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.
+#ifdef TCMALLOC
+#error Memory debugging cannot be enabled with tcmalloc.
+#endif
+
+#ifdef ENVOY_TSAN_BUIOD
+#error Memory debugging cannot be enabled with tsan.
+#endif
+
+#ifdef ENVOY_ASAN_BUILD
+#error Memory debugging cannot be enabled with asan.
+#endif
+
+namespace {
+
+constexpr uint32_t LiveMarker1 = 0xfeedface;         // first 4 bytes after alloc
+constexpr uint64_t LiveMarker2 = 0xfeedfacefeedface; // pattern written into allocated memory
+constexpr uint64_t DeadMarker = 0xdeadbeefdeadbeef;  // pattern to scribble over memory before 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.
+void scribble(void* ptr, uint64_t aligned_size, uint64_t scribble_word) {
+  assert((aligned_size % Overhead) == 0);
+  uint64_t num_uint64s = aligned_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 aligned_size = Envoy::Memory::align<Overhead>(size);
+  bytes_allocated += aligned_size;
+  uint32_t* marker = static_cast<uint32_t*>(::malloc(aligned_size + Overhead));
+  assert(marker != NULL);
+  marker[0] = LiveMarker1;
+  marker[1] = size;
+  uint32_t* payload = marker + sizeof(Overhead) / sizeof(uint32_t);
+  scribble(payload, aligned_size, LiveMarker2);
+  return payload;
+}
+
+// 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);
+    assert(LiveMarker1 == marker[0]);
+    uint32_t size = marker[1];
+    uint64_t aligned_size = Envoy::Memory::align<Overhead>(size);
+    assert(bytes_allocated >= aligned_size);
+    bytes_allocated -= aligned_size;
+    scribble(marker, aligned_size + sizeof(Overhead), DeadMarker);
+    ::free(marker);
+  }
+}
+
+} // namespace
+
+void* operator new(size_t size) { return debugMalloc(size); }
+void* operator new(size_t size, const std::nothrow_t&) noexcept { return debugMalloc(size); }
+void operator delete(void* ptr) noexcept { debugFree(ptr); }
+void operator delete(void* ptr, size_t) noexcept { debugFree(ptr); }
+void operator delete(void* ptr, std::nothrow_t const&)noexcept { debugFree(ptr); }
+
+void* operator new[](size_t size) { return debugMalloc(size); }
+void* operator new[](size_t size, const std::nothrow_t&) noexcept { return debugMalloc(size); }
+void operator delete[](void* ptr) noexcept { debugFree(ptr); }
+void operator delete[](void* ptr, size_t) noexcept { debugFree(ptr); }
+
+#endif // ENVOY_MEMORY_DEBUG_ENABLED

source/common/memory/debug.h

@@ -0,0 +1,21 @@
+#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();
+};
+
+} // namespace Memory
+} // namespace Envoy