diff --git a/bazel/BUILD b/bazel/BUILD index bffbac213c079..d19bf94a7ead9 100644 --- a/bazel/BUILD +++ b/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"}, diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index d85068486fc07..6e773c52f5a83 100644 --- a/bazel/envoy_build_system.bzl +++ b/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"], diff --git a/bazel/external/apache_thrift.BUILD b/bazel/external/apache_thrift.BUILD index b8fb5e480e7de..210c36cdf6c33 100644 --- a/bazel/external/apache_thrift.BUILD +++ b/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"], ) diff --git a/bazel/external/http-parser.BUILD b/bazel/external/http-parser.BUILD index 852dd715474a7..523d94fbf4316 100644 --- a/bazel/external/http-parser.BUILD +++ b/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"], diff --git a/bazel/external/libcircllhist.BUILD b/bazel/external/libcircllhist.BUILD index b70bf38740642..a77269ef60b02 100644 --- a/bazel/external/libcircllhist.BUILD +++ b/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"], ) diff --git a/bazel/external/libprotobuf_mutator.BUILD b/bazel/external/libprotobuf_mutator.BUILD index 9a76a48358a7e..12fd8b49b51f6 100644 --- a/bazel/external/libprotobuf_mutator.BUILD +++ b/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"], ) diff --git a/bazel/external/twitter_common_rpc.BUILD b/bazel/external/twitter_common_rpc.BUILD index 37046ec69bc89..8e8622ebb4db6 100644 --- a/bazel/external/twitter_common_rpc.BUILD +++ b/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", ], ) diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 169862b255827..69052823f5b3c 100644 --- a/source/common/common/BUILD +++ b/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", ], ) diff --git a/source/common/common/block_memory_hash_set.h b/source/common/common/block_memory_hash_set.h index 0689d24ecc381..fb9f2ada92699 100644 --- a/source/common/common/block_memory_hash_set.h +++ b/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 BlockMemoryHashSet : public Logger::Loggable 0) && ((alignment & (alignment - 1)) == 0), ""); - return (size + alignment - 1) & ~(alignment - 1); - } + static uint64_t align(uint64_t size) { return Memory::align(size); } /** * Computes the byte offset of a cell into cells_. This is not @@ -339,7 +333,7 @@ template class BlockMemoryHashSet : public Logger::Loggable(cells_) + cellOffset(cell_index, stats_options_); - RELEASE_ASSERT((reinterpret_cast(ptr) & (calculateAlignment() - 1)) == 0, ""); + RELEASE_ASSERT((reinterpret_cast(ptr) & (alignment - 1)) == 0, ""); return *reinterpret_cast(ptr); } diff --git a/source/common/memory/BUILD b/source/common/memory/BUILD index 30e2487125960..dd57035b38a12 100644 --- a/source/common/memory/BUILD +++ b/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"], ) diff --git a/source/common/memory/align.h b/source/common/memory/align.h new file mode 100644 index 0000000000000..051031c3b990f --- /dev/null +++ b/source/common/memory/align.h @@ -0,0 +1,18 @@ +#pragma once + +#include +#include + +namespace Envoy { +namespace Memory { + +template 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 diff --git a/source/common/memory/debug.cc b/source/common/memory/debug.cc new file mode 100644 index 0000000000000..fec394bc6a192 --- /dev/null +++ b/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 +#include // don't use Envoy ASSERT as it may allocate memory. +#include +#include + +#include "common/memory/align.h" + +static std::atomic 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(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(size); + bytes_allocated += aligned_size; + uint32_t* marker = static_cast(::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(ptr) - Overhead; + uint32_t* marker = reinterpret_cast(alloced_ptr); + assert(LiveMarker1 == marker[0]); + uint32_t size = marker[1]; + uint64_t aligned_size = Envoy::Memory::align(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 diff --git a/source/common/memory/debug.h b/source/common/memory/debug.h new file mode 100644 index 0000000000000..a0f5e7a954b00 --- /dev/null +++ b/source/common/memory/debug.h @@ -0,0 +1,21 @@ +#pragma once + +#include + +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 diff --git a/source/common/memory/stats.cc b/source/common/memory/stats.cc index 3b77e428ba295..e0aa6f0f5d48c 100644 --- a/source/common/memory/stats.cc +++ b/source/common/memory/stats.cc @@ -2,13 +2,17 @@ #include -#ifdef TCMALLOC +#include "common/memory/debug.h" +#ifdef TCMALLOC #include "gperftools/malloc_extension.h" +#endif namespace Envoy { namespace Memory { +#ifdef TCMALLOC + uint64_t Stats::totalCurrentlyAllocated() { size_t value = 0; MallocExtension::instance()->GetNumericProperty("generic.current_allocated_bytes", &value); @@ -40,21 +44,22 @@ uint64_t Stats::totalPageHeapUnmapped() { return value; } -} // namespace Memory -} // namespace Envoy - #else -namespace Envoy { -namespace Memory { +uint64_t Stats::totalCurrentlyAllocated() { +#if defined(ENVOY_MEMORY_DEBUG_ENABLED) + return Debug::bytesUsed(); +#else + return 0; +#endif +} -uint64_t Stats::totalCurrentlyAllocated() { return 0; } uint64_t Stats::totalThreadCacheBytes() { return 0; } uint64_t Stats::totalCurrentlyReserved() { return 0; } uint64_t Stats::totalPageHeapUnmapped() { return 0; } uint64_t Stats::totalPageHeapFree() { return 0; } +#endif // TCMALLOC + } // namespace Memory } // namespace Envoy - -#endif // #ifdef TCMALLOC diff --git a/source/exe/BUILD b/source/exe/BUILD index cfe3b17143543..1ea12af8ed329 100644 --- a/source/exe/BUILD +++ b/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"), ) diff --git a/source/exe/main.cc b/source/exe/main.cc index fae6d4b6585ef..fceacc9bd7115 100644 --- a/source/exe/main.cc +++ b/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. diff --git a/test/BUILD b/test/BUILD index d1e28211c9e40..0569be410857c 100644 --- a/test/BUILD +++ b/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", diff --git a/test/common/memory/BUILD b/test/common/memory/BUILD new file mode 100644 index 0000000000000..58955705266fd --- /dev/null +++ b/test/common/memory/BUILD @@ -0,0 +1,17 @@ +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:debug_lib", + ], +) diff --git a/test/common/memory/debug_test.cc b/test/common/memory/debug_test.cc new file mode 100644 index 0000000000000..cbdc1f6847a6b --- /dev/null +++ b/test/common/memory/debug_test.cc @@ -0,0 +1,37 @@ +#include "common/memory/debug.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 = Debug::bytesUsed(); + auto ptr = std::make_unique(); + uint64_t after = Debug::bytesUsed(); + EXPECT_EQ(sizeof(MyStruct), after - before); +} + +TEST(MemoryDebug, ScribbleOnNew) { + auto ptr = std::make_unique(); + for (int i = 0; i < ArraySize; ++i) { + EXPECT_EQ(0xfeedfacefeedface, ptr->words_[i]); + } +} + +TEST(MemoryDebug, ZeroByteAlloc) { auto ptr = std::make_unique(0); } + +#endif // ENVOY_MEMORY_DEBUG_ENABLED + +} // namespace Memory +} // namespace Envoy diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 2abece37c900e..5a14ba0e9e3f4 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -55,7 +55,10 @@ envoy_cc_test_library( external_deps = [ "abseil_strings", ], - deps = ["//source/common/memory:stats_lib"], + deps = [ + "//source/common/memory:debug_lib", + "//source/common/memory:stats_lib", + ], ) envoy_cc_test( diff --git a/test/common/stats/stat_test_utility.cc b/test/common/stats/stat_test_utility.cc index cbd9553168f37..b8729f561ae58 100644 --- a/test/common/stats/stat_test_utility.cc +++ b/test/common/stats/stat_test_utility.cc @@ -1,5 +1,6 @@ #include "test/common/stats/stat_test_utility.h" +#include "common/memory/debug.h" #include "common/memory/stats.h" namespace Envoy { @@ -9,16 +10,18 @@ namespace TestUtil { bool hasDeterministicMallocStats() { // We can only test absolute memory usage if the malloc library is a known // quantity. This decision is centralized here. As the preferred malloc - // 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 + // library for Envoy is TCMALLOC that's what we test for here. We also get + // deterministic (though slightly diffferent) byte-counts from memory + // debugging so we allow that as well. If we switch to a different malloc + // library than we'd have to re-evaluate all the thresholds in the tests + // referencing hasDeterministicMallocStats(). +#if !defined(TCMALLOC) && !defined(ENVOY_MEMORY_DEBUG_ENABLED) + return false; +#else const size_t start_mem = Memory::Stats::totalCurrentlyAllocated(); std::unique_ptr data(new char[10000]); const size_t end_mem = Memory::Stats::totalCurrentlyAllocated(); return end_mem - start_mem >= 10000; // actually 10240 -#else - return false; #endif } diff --git a/test/main.cc b/test/main.cc index efca6669890c0..9f4f00c16003f 100644 --- a/test/main.cc +++ b/test/main.cc @@ -1,4 +1,6 @@ // NOLINT(namespace-envoy) +#include "common/memory/debug.h" + #include "test/test_common/environment.h" #include "test/test_runner.h" @@ -11,6 +13,7 @@ // The main entry point (and the rest of this file) should have no logic in it, // this allows overriding by site specific versions of main.cc. int main(int argc, char** argv) { + Envoy::Memory::Debug mem_debug; #ifndef __APPLE__ absl::InitializeSymbolizer(argv[0]); #endif diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc index e9f1c8d692ffd..16365d9d1c756 100644 --- a/test/test_common/environment.cc +++ b/test/test_common/environment.cc @@ -124,7 +124,7 @@ absl::optional 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(); } diff --git a/tools/bazel.rc b/tools/bazel.rc index deff5674b2ca7..91990bf8c8183 100644 --- a/tools/bazel.rc +++ b/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