From eafd235d1b4314f7d9f7cf748e12aba4fa88dbaf Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Fri, 8 Jul 2022 10:06:42 -0700 Subject: [PATCH] Rename leak detection build options. Renames options to be formatted as: gpgmm_enable__leak_checks. --- .github/workflows/win_clang_rel_x64.yaml | 2 +- CMakeLists.txt | 23 +++++++++++++------ build_overrides/gpgmm_features.gni | 12 ++++++---- docs/DESIGN.md | 2 +- src/gpgmm/common/BUILD.gn | 12 ++++++---- src/gpgmm/d3d12/DebugResourceAllocatorD3D12.h | 2 +- src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp | 14 +++++------ src/gpgmm/utils/WindowsPlatformDebug.cpp | 2 ++ 8 files changed, 44 insertions(+), 25 deletions(-) diff --git a/.github/workflows/win_clang_rel_x64.yaml b/.github/workflows/win_clang_rel_x64.yaml index 076a19fb8..64c8e2fe3 100644 --- a/.github/workflows/win_clang_rel_x64.yaml +++ b/.github/workflows/win_clang_rel_x64.yaml @@ -161,7 +161,7 @@ jobs: set "PATH=%CD%\..\depot_tools;%PATH%" set "DEPOT_TOOLS_WIN_TOOLCHAIN=0" cd test - gn gen out\Fuzzer --args="is_debug=false use_libfuzzer=true is_asan=true gpgmm_enable_assert_on_warning=true gpgmm_enable_device_checks=true" + gn gen out\Fuzzer --args="is_debug=false use_libfuzzer=true is_asan=true gpgmm_enable_assert_on_warning=true gpgmm_enable_device_leak_checks=true" - name: Build fuzzer for main branch (with patch) shell: cmd diff --git a/CMakeLists.txt b/CMakeLists.txt index e741d6c8a..66b183a4c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -108,10 +108,11 @@ set_if_not_defined(GPGMM_VK_TOOLS_DIR "${GPGMM_VK_DEPS_DIR}/vulkan-tools/src" "D option_if_not_defined(GPGMM_ALWAYS_ASSERT "Enable assertions on all build types" OFF) option_if_not_defined(GPGMM_FORCE_TRACING "Enables event tracing even in release builds" OFF) -option_if_not_defined(GPGMM_ENABLE_DEVICE_CHECKS "Enables checking of device leaks" OFF) -option_if_not_defined(GPGMM_ENABLE_ALLOCATOR_CHECKS "Enables checking of allocator leaks" OFF) +option_if_not_defined(GPGMM_ENABLE_DEVICE_LEAK_CHECKS "Enables checking of device leaks" OFF) +option_if_not_defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS "Enables checking of allocator leaks" OFF) option_if_not_defined(GPGMM_ENABLE_ASSERT_ON_WARNING "Enables ASSERT on severity functionality" OFF) option_if_not_defined(GPGMM_DISABLE_SIZE_CACHE "Enables warming of caches with common resource sizes" OFF) +option_if_not_defined(GPGMM_ENABLE_MEMORY_LEAK_CHECKS "Enables memory leak detection." OFF) # The Vulkan loader is an optional dependency. if (GPGMM_ENABLE_VK) @@ -165,15 +166,15 @@ if(NOT GPGMM_FORCE_TRACING) ) endif() -if(GPGMM_ENABLE_DEVICE_CHECKS) - target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_ENABLE_DEVICE_CHECKS") +if(GPGMM_ENABLE_DEVICE_LEAK_CHECKS) + target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_ENABLE_DEVICE_LEAK_CHECKS") endif() -if(GPGMM_ENABLE_ALLOCATOR_CHECKS) - target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_ENABLE_ALLOCATOR_CHECKS") +if(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) + target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS") else() target_compile_definitions(gpgmm_common_config INTERFACE - $<$:GPGMM_ENABLE_ALLOCATOR_CHECKS> + $<$:GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS> ) endif() @@ -189,6 +190,14 @@ if(GPGMM_DISABLE_SIZE_CACHE) target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_DISABLE_SIZE_CACHE") endif() +if(GPGMM_ENABLE_MEMORY_LEAK_CHECKS) + target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_ENABLE_MEMORY_LEAK_CHECKS") +else() + target_compile_definitions(gpgmm_common_config INTERFACE + $<$:GPGMM_ENABLE_MEMORY_LEAK_CHECKS> + ) +endif() + if(WIN32) target_compile_definitions(gpgmm_common_config INTERFACE "NOMINMAX" "WIN32_LEAN_AND_MEAN") endif() diff --git a/build_overrides/gpgmm_features.gni b/build_overrides/gpgmm_features.gni index 95e86ffcf..f0e433dc4 100644 --- a/build_overrides/gpgmm_features.gni +++ b/build_overrides/gpgmm_features.gni @@ -45,8 +45,8 @@ declare_args() { gpgmm_shared_library = false # Enables checking of allocator leaks. - # Sets -dGPGMM_ENABLE_ALLOCATOR_CHECKS - gpgmm_enable_allocator_checks = is_debug + # Sets -dGPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS + gpgmm_enable_allocator_leak_checks = is_debug # Enables ASSERT on severity functionality. # Sets -dGPGMM_ENABLE_ASSERT_ON_WARNING @@ -57,8 +57,12 @@ declare_args() { gpgmm_disable_size_cache = false # Enables checking of device leaks. - # Sets -dGPGMM_ENABLE_DEVICE_CHECKS - gpgmm_enable_device_checks = false + # Sets -dGPGMM_ENABLE_DEVICE_LEAK_CHECKS + gpgmm_enable_device_leak_checks = false + + # Enables checking of memory leaks. + # Sets -dGPGMM_ENABLE_MEMORY_LEAK_CHECKS + gpgmm_enable_memory_leak_checks = is_debug # Configures Vulkan functions by statically importing them (ie. importing the Vulkan loader symbols). gpgmm_vk_static_functions = true diff --git a/docs/DESIGN.md b/docs/DESIGN.md index f29fb093e..652644f5d 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -168,7 +168,7 @@ Instead, GPGMM *prevents* out-of-memory events by maximizing re-use through opti ### Robust leak detection -GPGMM detects and reports leaks at the allocation-level (vs Memory Object). Should resources persist on the device (externally created), for example, between allocators, they will NOT be falsely reported as leaks, even when imported into GPGMM. This is enabled by `GPGMM_ENABLE_ALLOCATOR_CHECKS`. +GPGMM detects and reports leaks at the allocation-level (vs Memory Object). Should resources persist on the device (externally created), for example, between allocators, they will NOT be falsely reported as leaks, even when imported into GPGMM. This is enabled by `GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS`. ### **Data definition, schema design and Persistence requirements** diff --git a/src/gpgmm/common/BUILD.gn b/src/gpgmm/common/BUILD.gn index 687929fda..e74298c79 100644 --- a/src/gpgmm/common/BUILD.gn +++ b/src/gpgmm/common/BUILD.gn @@ -65,8 +65,8 @@ config("gpgmm_common_config") { defines += [ "GPGMM_DISABLE_TRACING" ] } - if (gpgmm_enable_allocator_checks) { - defines += [ "GPGMM_ENABLE_ALLOCATOR_CHECKS" ] + if (gpgmm_enable_allocator_leak_checks) { + defines += [ "GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS" ] } if (gpgmm_enable_assert_on_warning) { @@ -77,8 +77,12 @@ config("gpgmm_common_config") { defines += [ "GPGMM_DISABLE_SIZE_CACHE" ] } - if (gpgmm_enable_device_checks) { - defines += [ "GPGMM_ENABLE_DEVICE_CHECKS" ] + if (gpgmm_enable_device_leak_checks) { + defines += [ "GPGMM_ENABLE_DEVICE_LEAK_CHECKS" ] + } + + if (gpgmm_enable_memory_leak_checks) { + defines += [ "GPGMM_ENABLE_MEMORY_LEAK_CHECKS" ] } # Only internal build targets can use this config, this means only targets in diff --git a/src/gpgmm/d3d12/DebugResourceAllocatorD3D12.h b/src/gpgmm/d3d12/DebugResourceAllocatorD3D12.h index fbd0585d6..6adcb6291 100644 --- a/src/gpgmm/d3d12/DebugResourceAllocatorD3D12.h +++ b/src/gpgmm/d3d12/DebugResourceAllocatorD3D12.h @@ -27,7 +27,7 @@ namespace gpgmm::d3d12 { A "live" allocation means the allocation was created (allocated) but not released (de-allocated). - Use `gpgmm_enable_allocator_checks = true` to always report for leaks. + Use `gpgmm_enable_allocator_leak_checks = true` to always report for leaks. */ class DebugResourceAllocator final : public MemoryAllocator { public: diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp index 8aa3208e8..9bf9cf52b 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp @@ -357,14 +357,14 @@ namespace gpgmm::d3d12 { SetLogMessageLevel(GetLogSeverity(newDescriptor.MinLogLevel)); } -#if defined(GPGMM_ENABLE_DEVICE_CHECKS) +#if defined(GPGMM_ENABLE_DEVICE_LEAK_CHECKS) ComPtr leakMessageQueue; if (SUCCEEDED(newDescriptor.Device.As(&leakMessageQueue))) { D3D12_INFO_QUEUE_FILTER emptyFilter{}; ReturnIfFailed(leakMessageQueue->PushRetrievalFilter(&emptyFilter)); } else { - gpgmm::WarningLog() - << "Debug layer must be installed and enabled to use GPGMM_ENABLE_DEVICE_CHECKS."; + gpgmm::WarningLog() << "Debug layer must be installed and enabled to use " + "GPGMM_ENABLE_DEVICE_LEAK_CHECKS."; } #endif @@ -398,7 +398,7 @@ namespace gpgmm::d3d12 { mUseDetailedTimingEvents(descriptor.RecordOptions.UseDetailedTimingEvents) { GPGMM_TRACE_EVENT_OBJECT_NEW(this); -#if defined(GPGMM_ENABLE_ALLOCATOR_CHECKS) +#if defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) mDebugAllocator = std::make_unique(); #endif @@ -604,11 +604,11 @@ namespace gpgmm::d3d12 { mResourceAllocatorOfType = {}; mResourceHeapAllocatorOfType = {}; -#if defined(GPGMM_ENABLE_ALLOCATOR_CHECKS) +#if defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) mDebugAllocator->ReportLiveAllocations(); #endif -#if defined(GPGMM_ENABLE_DEVICE_CHECKS) +#if defined(GPGMM_ENABLE_DEVICE_LEAK_CHECKS) ReportLiveDeviceObjects(mDevice); #endif mResidencyManager = nullptr; @@ -692,7 +692,7 @@ namespace gpgmm::d3d12 { // Insert a new (debug) allocator layer into the allocation so it can report details used // during leak checks. Since we don't want to use it unless we are debugging, we hide it // behind a macro. -#if defined(GPGMM_ENABLE_ALLOCATOR_CHECKS) +#if defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) mDebugAllocator->AddLiveAllocation(*ppResourceAllocationOut); #endif diff --git a/src/gpgmm/utils/WindowsPlatformDebug.cpp b/src/gpgmm/utils/WindowsPlatformDebug.cpp index 63e417c8f..2e1b194fc 100644 --- a/src/gpgmm/utils/WindowsPlatformDebug.cpp +++ b/src/gpgmm/utils/WindowsPlatformDebug.cpp @@ -58,7 +58,9 @@ namespace gpgmm { // Perform automatic leak checking at program exit through a call to _CrtDumpMemoryLeaks // and generate an error report if the application failed to free all the memory it // allocated. +#ifdef GPGMM_ENABLE_MEMORY_LEAK_CHECKS _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); +#endif } private: