debug: Link in the debug version of tcmalloc for debug builds.

bazel/BUILD

@@ -95,6 +95,11 @@ config_setting(
     values = {"define": "tcmalloc=disabled"},
 )
 
+config_setting(
+    name = "debug_tcmalloc",
+    values = {"define": "tcmalloc=debug"},
+)
+
 config_setting(
     name = "disable_signal_trace",
     values = {"define": "signal_trace=disabled"},

bazel/README.md

@@ -325,6 +325,9 @@ The following optional features can be enabled on the Bazel build command-line:
 * ASSERT() can be configured to log failures and increment a stat counter in a release build with
   `--define log_debug_assert_in_release=enabled`. The default behavior is to compile debug assertions out of
   release builds so that the condition is not evaluated. This option has no effect in debug builds.
+* tcmalloc can be disabled with `--define tcmalloc=disabled`
+* memory-debugging (scribbling over memory after allocation and before freeing) with
+  `--define tcmalloc=debug`.
 
 ## Disabling extensions
 

bazel/envoy_build_system.bzl

@@ -67,6 +67,9 @@ def envoy_copts(repository, test = False):
            }) + select({
                repository + "//bazel:disable_tcmalloc": ["-DABSL_MALLOC_HOOK_MMAP_DISABLE"],
                "//conditions:default": ["-DTCMALLOC"],
+           }) + select({
+               repository + "//bazel:debug_tcmalloc": ["-DENVOY_MEMORY_DEBUG_ENABLED=1"],
+               "//conditions:default": [],
            }) + select({
                repository + "//bazel:disable_signal_trace": [],
                "//conditions:default": ["-DENVOY_HANDLE_SIGNALS"],
@@ -168,6 +171,7 @@ def envoy_external_dep_path(dep):
 def tcmalloc_external_dep(repository):
     return select({
         repository + "//bazel:disable_tcmalloc": None,
+        repository + "//bazel:debug_tcmalloc": envoy_external_dep_path("tcmalloc_debug"),
         "//conditions:default": envoy_external_dep_path("tcmalloc_and_profiler"),
     })
 
@@ -177,6 +181,7 @@ def tcmalloc_external_dep(repository):
 def tcmalloc_external_deps(repository):
     return select({
         repository + "//bazel:disable_tcmalloc": [],
+        repository + "//bazel:debug_tcmalloc": [envoy_external_dep_path("tcmalloc_debug")],
         "//conditions:default": [envoy_external_dep_path("tcmalloc_and_profiler")],
     })
 

bazel/target_recipes.bzl

@@ -6,6 +6,7 @@ TARGET_RECIPES = {
     "benchmark": "benchmark",
     "event": "libevent",
     "tcmalloc_and_profiler": "gperftools",
+    "tcmalloc_debug": "gperftools",
     "luajit": "luajit",
     "nghttp2": "nghttp2",
     "yaml_cpp": "yaml-cpp",

ci/do_ci.sh

@@ -157,6 +157,7 @@ elif [[ "$1" == "bazel.compile_time_options" ]]; then
     --define google_grpc=disabled \
     --define boringssl=fips \
     --define log_debug_assert_in_release=enabled \
+    --define=tcmalloc=debug \
   "
   setup_clang_toolchain
   # This doesn't go into CI but is available for developer convenience.

ci/prebuilt/BUILD

@@ -66,6 +66,13 @@ cc_library(
     strip_include_prefix = "thirdparty_build/include",
 )
 
+cc_library(
+    name = "tcmalloc_debug",
+    srcs = ["thirdparty_build/lib/libtcmalloc_debug.a"],
+    hdrs = glob(["thirdparty_build/include/gperftools/**/*.h"]),
+    strip_include_prefix = "thirdparty_build/include",
+)
+
 cc_library(
     name = "yaml_cpp",
     srcs = select({

source/common/profiler/profiler.cc

@@ -2,7 +2,7 @@
 
 #include <string>
 
-#ifdef TCMALLOC
+#ifdef PROFILER_AVAILABLE
 
 #include "gperftools/heap-profiler.h"
 #include "gperftools/profiler.h"

source/common/profiler/profiler.h

@@ -2,6 +2,13 @@
 
 #include <string>
 
+// Profiling support is provided in the release tcmalloc, but not in the library
+// that supplies the debug tcmalloc. So all the profiling code must be ifdef'd
+// on PROFILER_AVAILABLE which is dependent on those two settings.
+#if defined(TCMALLOC) && !defined(ENVOY_MEMORY_DEBUG_ENABLED)
+#define PROFILER_AVAILABLE
+#endif
+
 namespace Envoy {
 namespace Profiler {
 

test/common/memory/BUILD

@@ -0,0 +1,15 @@
+licenses(["notice"])  # Apache 2
+
+load(
+    "//bazel:envoy_build_system.bzl",
+    "envoy_cc_test",
+    "envoy_package",
+)
+
+envoy_package()
+
+envoy_cc_test(
+    name = "debug_test",
+    srcs = ["debug_test.cc"],
+    deps = ["//source/common/memory:stats_lib"],
+)

test/common/memory/debug_test.cc

@@ -0,0 +1,51 @@
+#include "common/memory/stats.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Memory {
+
+#ifdef ENVOY_MEMORY_DEBUG_ENABLED
+
+constexpr int ArraySize = 10;
+
+struct MyStruct {
+  MyStruct() : x_(0) {} // words_ is uninitialized; will have whatever allocator left there.
+  uint64_t x_;
+  uint64_t words_[ArraySize];
+};
+
+TEST(MemoryDebug, ByteSize) {
+  uint64_t before = Stats::totalCurrentlyAllocated();
+  auto ptr = std::make_unique<MyStruct>();
+  uint64_t after = Stats::totalCurrentlyAllocated();
+  EXPECT_LE(sizeof(MyStruct), after - before);
+}
+
+TEST(MemoryDebug, ScribbleOnNew) {
+  auto ptr = std::make_unique<MyStruct>();
+  for (int i = 0; i < ArraySize; ++i) {
+    // This is the pattern written by tcmalloc's debug library.
+    EXPECT_EQ(0xabababababababab, ptr->words_[i]);
+  }
+}
+
+TEST(MemoryDebug, ScribbleOnDelete) {
+  uint64_t* words;
+  {
+    auto ptr = std::make_unique<MyStruct>();
+    words = ptr->words_;
+  }
+  for (int i = 0; i < ArraySize; ++i) {
+    // This is the pattern written by tcmalloc's debug library on destruction.
+    // Note: this test cannot be run under valgrind or asan.
+    EXPECT_EQ(0xcdcdcdcdcdcdcdcd, words[i]);
+  }
+}
+
+TEST(MemoryDebug, ZeroByteAlloc) { auto ptr = std::make_unique<uint8_t[]>(0); }
+
+#endif // ENVOY_MEMORY_DEBUG_ENABLED
+
+} // namespace Memory
+} // namespace Envoy

test/common/stats/stat_test_utility.cc

@@ -12,7 +12,7 @@ bool hasDeterministicMallocStats() {
   // library for Envoy is TCMALLOC that's what we test for here. If we switch
   // to a different malloc library than we'd have to re-evaluate all the
   // thresholds in the tests referencing hasDeterministicMallocStats().
-#ifdef TCMALLOC
+#if defined(TCMALLOC) && !defined(ENVOY_MEMORY_DEBUG_ENABLED)
   const size_t start_mem = Memory::Stats::totalCurrentlyAllocated();
   std::unique_ptr<char[]> data(new char[10000]);
   const size_t end_mem = Memory::Stats::totalCurrentlyAllocated();

test/integration/integration_admin_test.cc

@@ -5,6 +5,7 @@
 #include "envoy/http/header_map.h"
 
 #include "common/common/fmt.h"
+#include "common/profiler/profiler.h"
 #include "common/stats/stats_matcher_impl.h"
 
 #include "test/integration/utility.h"
@@ -391,9 +392,6 @@ TEST_P(IntegrationAdminTest, AdminOnDestroyCallbacks) {
   EXPECT_EQ(test_server_->server().statsFlushInterval(), std::chrono::milliseconds(5000));
 }
 
-// Successful call to startProfiler requires tcmalloc.
-#ifdef TCMALLOC
-
 TEST_P(IntegrationAdminTest, AdminCpuProfilerStart) {
   config_helper_.addConfigModifier([&](envoy::config::bootstrap::v2::Bootstrap& bootstrap) -> void {
     auto* admin = bootstrap.mutable_admin();
@@ -404,14 +402,17 @@ TEST_P(IntegrationAdminTest, AdminCpuProfilerStart) {
   BufferingStreamDecoderPtr response = IntegrationUtil::makeSingleRequest(
       lookupPort("admin"), "POST", "/cpuprofiler?enable=y", "", downstreamProtocol(), version_);
   EXPECT_TRUE(response->complete());
+#ifdef PROFILER_AVAILABLE
   EXPECT_STREQ("200", response->headers().Status()->value().c_str());
+#else
+  EXPECT_STREQ("500", response->headers().Status()->value().c_str());
+#endif
 
   response = IntegrationUtil::makeSingleRequest(
       lookupPort("admin"), "POST", "/cpuprofiler?enable=n", "", downstreamProtocol(), version_);
   EXPECT_TRUE(response->complete());
   EXPECT_STREQ("200", response->headers().Status()->value().c_str());
 }
-#endif
 
 class IntegrationAdminIpv4Ipv6Test : public HttpIntegrationTest, public testing::Test {
 public:

test/server/http/admin_test.cc

@@ -622,22 +622,26 @@ class AdminInstanceTest : public testing::TestWithParam<Network::Address::IpVers
 INSTANTIATE_TEST_CASE_P(IpVersions, AdminInstanceTest,
                         testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
                         TestUtility::ipTestParamsToString);
-// Can only get code coverage of AdminImpl::handlerCpuProfiler stopProfiler with
-// a real profiler linked in (successful call to startProfiler). startProfiler
-// requies tcmalloc.
-#ifdef TCMALLOC
 
 TEST_P(AdminInstanceTest, AdminProfiler) {
   Buffer::OwnedImpl data;
   Http::HeaderMapImpl header_map;
+
+  // Can only get code coverage of AdminImpl::handlerCpuProfiler stopProfiler with
+  // a real profiler linked in (successful call to startProfiler).
+#ifdef PROFILER_AVAILABLE
   EXPECT_EQ(Http::Code::OK, postCallback("/cpuprofiler?enable=y", header_map, data));
   EXPECT_TRUE(Profiler::Cpu::profilerEnabled());
+#else
+  EXPECT_EQ(Http::Code::InternalServerError,
+            postCallback("/cpuprofiler?enable=y", header_map, data));
+  EXPECT_FALSE(Profiler::Cpu::profilerEnabled());
+#endif
+
   EXPECT_EQ(Http::Code::OK, postCallback("/cpuprofiler?enable=n", header_map, data));
   EXPECT_FALSE(Profiler::Cpu::profilerEnabled());
 }
 
-#endif
-
 TEST_P(AdminInstanceTest, MutatesErrorWithGet) {
   Buffer::OwnedImpl data;
   Http::HeaderMapImpl header_map;