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"],

source/common/memory/BUILD

@@ -8,6 +8,14 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "mem_debug_lib",
+    srcs = ["mem_debug.cc"],
+    hdrs = ["mem_debug.h"],
+    tcmalloc_dep = 1,
+    deps = ["//source/common/common:assert_lib"],
+)
+
 envoy_cc_library(
     name = "stats_lib",
     srcs = ["stats.cc"],

source/common/memory/mem_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 (kLiveMarker or kDeadMarker1). 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
+
+#define INSTALL_HOOKS
+
+// 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)) with
+// tcmallocHook(const void* ptr, size_t size). I tried const_casting ptr
+// and scribbling over it, but this results in 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 free hook. See
+// gperftools/malloc_hook.h for details.
+
+#if !defined(TCMALLOC) && !defined(ENVOY_DISABLE_MEMDEBUG)
+
+#include <cstdlib>
+
+#include "common/common/assert.h"
+
+namespace {
+
+constexpr int32_t kLiveMarker = 0xfeedface;       // first 4 bytes after alloc
+constexpr int32_t kDeadMarker1 = 0xabacabff;      // first 4 bytes after free
+constexpr int32_t kDeadMarker2 = 0xdeadbeef;      // overwrites the 'size' field on free
+constexpr size_t kOverhead = 2 * sizeof(int32_t); // number of extra bytes to alloc
+
+void scribble(void* ptr, size_t size, int32_t scribble_word) {
+  int32_t num_ints = size / sizeof(int32_t);
+  int32_t* p = static_cast<int32_t*>(ptr);
+  for (int i = 0; i < num_ints; ++i, ++p) {
+    *p = scribble_word;
+  }
+}
+
+size_t roundedSize(size_t size) {
+  if (size == 0) {
+    size = kOverhead;
+  } else if ((size % kOverhead) != 0) {
+    size = size + kOverhead - (size % kOverhead);
+  }
+  return size;
+}
+
+void* debugMalloc(size_t size) {
+  size_t rounded = roundedSize(size);
+  int32_t* marker = static_cast<int32_t*>(malloc(rounded + kOverhead));
+  ASSERT(marker != NULL);
+  marker[0] = kLiveMarker;
+  marker[1] = size;
+  int32_t* ret = marker + 2;
+  scribble(ret, rounded, kLiveMarker);
+  return reinterpret_cast<char*>(marker) + kOverhead;
+}
+
+void debugFree(void* ptr) {
+  if (ptr != NULL) {
+    char* alloced_ptr = static_cast<char*>(ptr) - kOverhead;
+    int32_t* marker = reinterpret_cast<int32_t*>(alloced_ptr);
+    scribble(ptr, roundedSize(marker[1]), kDeadMarker2);
+    ASSERT(kLiveMarker == marker[0]);
+    marker[0] = kDeadMarker1;
+    marker[1] = kDeadMarker2;
+    free(marker);
+  }
+}
+
+} // namespace
+
+// C++ operator new/delete overrides, in all 8 combinations:
+//    (new vs delete) * (const std::nothrow_t& vs not) * ([] vs not)
+// On MacOS __THROW appears to be missing so hide those in an ifdef.
+#ifndef __THROW
+#define __THROW
+#endif
+
+void* operator new(size_t size) { return debugMalloc(size); }
+void operator delete(void* ptr)_GLIBCXX_USE_NOEXCEPT { debugFree(ptr); }
+void operator delete(void* ptr, size_t)_GLIBCXX_USE_NOEXCEPT { debugFree(ptr); }
+
+void* operator new[](size_t size) { return debugMalloc(size); }
+void operator delete[](void* ptr) _GLIBCXX_USE_NOEXCEPT { debugFree(ptr); }
+void operator delete[](void* ptr, size_t) _GLIBCXX_USE_NOEXCEPT { debugFree(ptr); }
+
+#endif // !TCMALLOC && !ENVOY_DISABLE_MEMDEBUG
+#endif // !NDEBUG
+
+// We provide the entry-point to be called to force-load the memory debugger
+// regardless of compilation mode.
+namespace Envoy {
+
+void MemDebugLoader() {}
+
+} // namespace Envoy

source/common/memory/mem_debug.h

@@ -0,0 +1,10 @@
+#pragma once
+
+namespace Envoy {
+
+// Called to force-load the memory debugging module, which (when tcmalloc is
+// disabled) overrides operator new/delete. See comments in the .cc file for
+// more details.
+void MemDebugLoader();
+
+} // namespace Envoy

source/exe/BUILD

@@ -69,6 +69,7 @@ envoy_cc_library(
         "//source/common/api:os_sys_calls_lib",
         "//source/common/common:compiler_requirements_lib",
         "//source/common/common:perf_annotation_lib",
+        "//source/common/memory:mem_debug_lib",
         "//source/server:hot_restart_lib",
         "//source/server:hot_restart_nop_lib",
         "//source/server:proto_descriptors_lib",

source/exe/main_common.cc

@@ -6,6 +6,7 @@
 #include "common/common/compiler_requirements.h"
 #include "common/common/perf_annotation.h"
 #include "common/event/libevent.h"
+#include "common/memory/mem_debug.h"
 #include "common/network/utility.h"
 #include "common/stats/thread_local_store.h"
 
@@ -46,6 +47,7 @@ MainCommonBase::MainCommonBase(OptionsImpl& options, Event::TimeSystem& time_sys
                                Thread::ThreadFactory& thread_factory)
     : options_(options), component_factory_(component_factory), thread_factory_(thread_factory) {
   ares_library_init(ARES_LIB_INIT_ALL);
+  Envoy::MemDebugLoader();
   Event::Libevent::Global::initialize();
   RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors(), "");
 

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:mem_debug_lib",
         "//test/mocks/access_log:access_log_mocks",
         "//test/test_common:environment_lib",
         "//test/test_common:global_lib",

test/main.cc

@@ -1,4 +1,6 @@
 // NOLINT(namespace-envoy)
+#include "common/memory/mem_debug.h"
+
 #include "test/test_common/environment.h"
 #include "test/test_runner.h"
 
@@ -32,5 +34,6 @@ int main(int argc, char** argv) {
   // v4 and v6 addresses is desired. This feature is in progress and will be rolled out to all tests
   // in upcoming PRs.
   Envoy::TestEnvironment::setEnvVar("ENVOY_IP_TEST_VERSIONS", "all", 0);
+  Envoy::MemDebugLoader();
   return Envoy::TestRunner::RunTests(argc, argv);
 }

test/test_common/environment.cc

@@ -124,7 +124,7 @@ absl::optional<std::string> TestEnvironment::getOptionalEnvVar(const std::string
 
 std::string TestEnvironment::getCheckedEnvVar(const std::string& var) {
   auto optional = getOptionalEnvVar(var);
-  RELEASE_ASSERT(optional.has_value(), "");
+  RELEASE_ASSERT(optional.has_value(), var);
   return optional.value();
 }
 

tools/bazel.rc

@@ -35,6 +35,7 @@ build:clang-asan --linkopt -fuse-ld=lld
 
 # Clang 5.0 TSAN
 build:clang-tsan --define ENVOY_CONFIG_TSAN=1
+build:clang-tsan --define ENVOY_MEMDEBUG_DISABLE=1
 build:clang-tsan --copt -fsanitize=thread
 build:clang-tsan --linkopt -fsanitize=thread
 build:clang-tsan --define tcmalloc=disabled