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 <[email protected]>
Azure · Jan 12, 2023 · 580fc3e · 580fc3e
1 parent 23770aa
commit 580fc3e
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 33 deletions.
diff --git 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 <string>
-
+#include <string>
+
+#include <gtest/gtest.h>
+
+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<int>(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<int>(VLDGetLeaksCount());
+    ASSERT_EQ(0, leaks);
+}
+
+TEST_F(TestIgnoreFunctions, IgnoreFunctionsReportsNonListedLeaks)
+{
     int leaks = static_cast<int>(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<int>(VLDGetLeaksCount());
+    ASSERT_EQ(1, leaks);
+}
+
+TEST_F(TestIgnoreFunctions, IgnoreFunctionsReportsStaticStringLeaks)
+{
+    int leaks = static_cast<int>(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<int>(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
@@ -181,4 +181,4 @@ SkipCrtStartupLeaks = yes
 ;   Valid Values: Functions names as strings separated by comma. Strings are case sensitive.
 ;   Default: None.
 ;
-IgnoreFunctionsList = GetOSVersion,SomeOtherString
+IgnoreFunctionsList = GetOSVersion,SomeOtherString,abcdefg,SomeOtherString,otherString,testOtherString
diff --git 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
@@ -158,13 +158,18 @@ typedef Set<moduleinfo_t> ModuleSet;
 
 typedef Set<VLD_REPORT_HOOK> ReportHookSet;
 
-
-struct LPCWSTRComparator {
-    bool operator()(LPCWSTR a, LPCWSTR b) const {
-        return lstrcmpW(a, b) > 0;
-    }
-};
-typedef std::set<LPCWSTR, LPCWSTRComparator> 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<functioninfo_t> 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 [];