From 580fc3eefabae2ea4ef1264f31d931d80a93e6cb Mon Sep 17 00:00:00 2001 From: Cyrus Jackson Date: Thu, 12 Jan 2023 13:03:25 +0530 Subject: [PATCH] Use vld native set for IgnoreFunctionsSet (#22) * Use native Set for ignore functions * Address Review and add gTest for the uts * fix braces Co-authored-by: Cyrus Jackson --- .../ignore_functions_test.cpp | 106 +++++++++++++++--- src/tests/ignore_functions_test/vld.ini | 2 +- src/vld.cpp | 16 ++- src/vldint.h | 21 ++-- 4 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/tests/ignore_functions_test/ignore_functions_test.cpp b/src/tests/ignore_functions_test/ignore_functions_test.cpp index 076d4b33..7929d0c0 100644 --- a/src/tests/ignore_functions_test/ignore_functions_test.cpp +++ b/src/tests/ignore_functions_test/ignore_functions_test.cpp @@ -3,35 +3,105 @@ #include "stdafx.h" #include "vld.h" -#include - +#include + +#include + +class TestIgnoreFunctions : public ::testing::Test +{ + virtual void SetUp() + { + VLDMarkAllLeaksAsReported(); + } + virtual void TearDown() + { + // Check that callstack resolved without unresolved functions (required symbols for all dll's) + EXPECT_EQ(0, VLDResolveCallstacks()); + } +}; -std::string GetOSVersion() { +std::string GetOSVersion() +{ std::string osVersion = "Windows"; return osVersion; } -std::string SomeOtherString() { +std::string SomeOtherString() +{ std::string osVersion = "Windows"; return osVersion; } - -int main(int argc, char** argv) + +std::string abcdefg() { + std::string osVersion = "Windows"; + return osVersion; +} + +std::string testOtherString() +{ + std::string osVersion = "Windows"; + return osVersion; +} + +std::string NotInTheList() +{ + std::string osVersion = "NotListed"; + return osVersion; +} + +TEST_F(TestIgnoreFunctions, IgnoreFunctionsSuccess) +{ + int leaks = static_cast(VLDGetLeaksCount()); + ASSERT_EQ(0, leaks); + + // All of these strings should be ignored. static std::string const osVer = GetOSVersion(); static std::string const someOtherString = SomeOtherString(); - + static std::string const str3 = abcdefg(); + static std::string const str4 = testOtherString(); + + leaks = static_cast(VLDGetLeaksCount()); + ASSERT_EQ(0, leaks); +} + +TEST_F(TestIgnoreFunctions, IgnoreFunctionsReportsNonListedLeaks) +{ int leaks = static_cast(VLDGetLeaksCount()); - if (0 != leaks) - { - (void)printf("!!! FAILED - Leaks detected: %i\n", leaks); - VLDReportLeaks(); - } - else - { - (void)printf("PASSED\n"); - } - + ASSERT_EQ(0, leaks); + + // All of these strings should be ignored. + static std::string const osVer = GetOSVersion(); + static std::string const someOtherString = SomeOtherString(); + static std::string const str3 = abcdefg(); + + //This should be detected as leak + static std::string const str4 = NotInTheList(); + + leaks = static_cast(VLDGetLeaksCount()); + ASSERT_EQ(1, leaks); +} + +TEST_F(TestIgnoreFunctions, IgnoreFunctionsReportsStaticStringLeaks) +{ + int leaks = static_cast(VLDGetLeaksCount()); + ASSERT_EQ(0, leaks); + + // All of these strings should be ignored. + static std::string const someOtherString = SomeOtherString(); + static std::string const str3 = abcdefg(); + + //This should be detected as leak + static std::string const osVer = "LeakString"; + static std::string const str4 = NotInTheList(); + + leaks = static_cast(VLDGetLeaksCount()); + ASSERT_EQ(2, leaks); +} - return leaks; +int main(int argc, char** argv) +{ + ::testing::InitGoogleTest(&argc, argv); + VLDMarkAllLeaksAsReported(); + return RUN_ALL_TESTS(); } diff --git a/src/tests/ignore_functions_test/vld.ini b/src/tests/ignore_functions_test/vld.ini index df498fcd..d12b8dcb 100644 --- a/src/tests/ignore_functions_test/vld.ini +++ b/src/tests/ignore_functions_test/vld.ini @@ -181,4 +181,4 @@ SkipCrtStartupLeaks = yes ; Valid Values: Functions names as strings separated by comma. Strings are case sensitive. ; Default: None. ; -IgnoreFunctionsList = GetOSVersion,SomeOtherString \ No newline at end of file +IgnoreFunctionsList = GetOSVersion,SomeOtherString,abcdefg,SomeOtherString,otherString,testOtherString \ No newline at end of file diff --git a/src/vld.cpp b/src/vld.cpp index 2d12feb8..83c98591 100644 --- a/src/vld.cpp +++ b/src/vld.cpp @@ -426,7 +426,8 @@ VisualLeakDetector::VisualLeakDetector () m_selfTestLine = 0; m_tlsIndex = TlsAlloc(); m_tlsLock.Initialize(); - m_tlsMap = new TlsMap; + m_tlsMap = new TlsMap; + m_ignoreFunctions = new IgnoreFunctionsSet(); if (m_options & VLD_OPT_SELF_TEST) { // Self-test mode has been enabled. Intentionally leak a small amount of @@ -458,8 +459,9 @@ VisualLeakDetector::VisualLeakDetector () LPCWSTR pwc; wchar_t* buffer; pwc = wcstok_s(m_ignoreFunctionsList, L",", &buffer); - while (pwc != NULL) { - m_ignoreFunctions.insert(pwc); + while (pwc != NULL) { + functioninfo_t funtioninfo = { pwc }; + m_ignoreFunctions->insert(funtioninfo); pwc = wcstok_s(NULL, L",", &buffer); } } @@ -683,7 +685,8 @@ VisualLeakDetector::~VisualLeakDetector () } delete m_heapMap; } - delete m_loadedModules; + delete m_loadedModules; + delete m_ignoreFunctions; { // Free internally allocated resources used for thread local storage. @@ -2376,8 +2379,9 @@ bool VisualLeakDetector::isModuleExcluded(UINT_PTR address) bool VisualLeakDetector::isFunctionIgnored(LPCWSTR functionName) -{ - return g_vld.m_ignoreFunctions.find(functionName) != g_vld.m_ignoreFunctions.end(); +{ + functioninfo_t functioninfo = { functionName }; + return g_vld.m_ignoreFunctions->find(functioninfo) != g_vld.m_ignoreFunctions->end(); } SIZE_T VisualLeakDetector::GetLeaksCount() diff --git a/src/vldint.h b/src/vldint.h index c5a535aa..a4a8e45a 100644 --- a/src/vldint.h +++ b/src/vldint.h @@ -158,13 +158,18 @@ typedef Set ModuleSet; typedef Set ReportHookSet; - -struct LPCWSTRComparator { - bool operator()(LPCWSTR a, LPCWSTR b) const { - return lstrcmpW(a, b) > 0; - } -}; -typedef std::set IgnoreFunctionsSet; +struct functioninfo_t { + BOOL operator < (const struct functioninfo_t& other) const { + + if (lstrcmpW(name, other.name) < 0) { + return TRUE; + } else { + return FALSE; + } + } + LPCWSTR name; +}; +typedef Set IgnoreFunctionsSet; // Thread local storage structure. Every thread in the process gets its own copy // of this structure. Thread specific information, such as the current leak @@ -387,7 +392,7 @@ class VisualLeakDetector : public IMalloc CriticalSection m_modulesLock; // Protects accesses to the "loaded modules" ModuleSet. CriticalSection m_optionsLock; // Serializes access to the heap and block maps. UINT32 m_options; // Configuration options. - IgnoreFunctionsSet m_ignoreFunctions; // Contains information about all the functions that needs to be ignored. + IgnoreFunctionsSet *m_ignoreFunctions; // Contains information about all the functions that needs to be ignored. static patchentry_t m_kernelbasePatch []; static patchentry_t m_kernel32Patch [];