From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 2 May 2024 15:16:10 +0000 (+0200)
Subject: dnsdist: Detect memory leaks during regression tests in CI
X-Git-Tag: rec-5.1.0-alpha1~7^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5766364dd3c2543ff04190b070bb5b86b3601958;p=thirdparty%2Fpdns.git

dnsdist: Detect memory leaks during regression tests in CI
---

diff --git a/.github/workflows/build-and-test-all.yml b/.github/workflows/build-and-test-all.yml
index c30fb4a06a..b686ed4b29 100644
--- a/.github/workflows/build-and-test-all.yml
+++ b/.github/workflows/build-and-test-all.yml
@@ -656,6 +656,7 @@ jobs:
         UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1:suppressions=${{ env.REPO_HOME }}/build-scripts/UBSan.supp"
         # Disabling (intercept_send=0) the custom send wrappers for ASAN and TSAN because they cause the tools to report a race that doesn't exist on actual implementations of send(), see https://github.com/google/sanitizers/issues/1498
         ASAN_OPTIONS: intercept_send=0
+        LSAN_OPTIONS: "suppressions=${{ env.REPO_HOME }}/pdns/dnsdistdist/dnsdist-lsan.supp"
         TSAN_OPTIONS: "halt_on_error=1:intercept_send=0:suppressions=${{ env.REPO_HOME }}/pdns/dnsdistdist/dnsdist-tsan.supp"
         # IncludeDir tests are disabled because of a weird interaction between TSAN and these tests which ever only happens on GH actions
         SKIP_INCLUDEDIR_TESTS: yes
diff --git a/m4/pdns_enable_sanitizers.m4 b/m4/pdns_enable_sanitizers.m4
index 6773e1a991..6243dce06b 100644
--- a/m4/pdns_enable_sanitizers.m4
+++ b/m4/pdns_enable_sanitizers.m4
@@ -78,6 +78,10 @@ AC_DEFUN([PDNS_ENABLE_ASAN], [
             [#include <sanitizer/common_interface_defs.h>]
           )]
         )
+        AC_CHECK_HEADERS([sanitizer/lsan_interface.h],
+          AC_DEFINE([HAVE_LEAK_SANITIZER_INTERFACE], [1], [Define if LSAN interface is available.]),
+         []
+        )
       ],
       [AC_MSG_ERROR([Cannot enable AddressSanitizer])]
     )
@@ -121,6 +125,10 @@ AC_DEFUN([PDNS_ENABLE_LSAN], [
       [SANITIZER_FLAGS="-fsanitize=leak $SANITIZER_FLAGS"],
       [AC_MSG_ERROR([Cannot enable LeakSanitizer])]
     )
+    AC_CHECK_HEADERS([sanitizer/lsan_interface.h],
+      AC_DEFINE([HAVE_LEAK_SANITIZER_INTERFACE], [1], [Define if LSAN interface is available.]),
+      []
+    )
   ])
   AC_SUBST([SANITIZER_FLAGS])
 ])
@@ -164,4 +172,3 @@ AC_DEFUN([PDNS_ENABLE_MSAN], [
   ])
   AC_SUBST([SANITIZER_FLAGS])
 ])
-
diff --git a/pdns/dnsdistdist/dnsdist-lsan.supp b/pdns/dnsdistdist/dnsdist-lsan.supp
new file mode 100644
index 0000000000..a04f7122a8
--- /dev/null
+++ b/pdns/dnsdistdist/dnsdist-lsan.supp
@@ -0,0 +1,2 @@
+# h2o
+leak:create_socket
diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index 80ee94299d..1911e34180 100644
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -2753,7 +2753,22 @@ static void usage()
   cout << "-V,--version          Show dnsdist version information and exit\n";
 }
 
-#ifdef COVERAGE
+/* g++ defines __SANITIZE_THREAD__
+   clang++ supports the nice __has_feature(thread_sanitizer),
+   let's merge them */
+#if defined(__has_feature)
+#if __has_feature(thread_sanitizer)
+#define __SANITIZE_THREAD__ 1
+#endif
+#if __has_feature(address_sanitizer)
+#define __SANITIZE_ADDRESS__ 1
+#if defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE)
+#include <sanitizer/lsan_interface.h>
+#endif
+#endif
+#endif
+
+#if defined(COVERAGE) || (defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE))
 static void cleanupLuaObjects()
 {
   /* when our coverage mode is enabled, we need to make sure
@@ -2770,24 +2785,16 @@ static void cleanupLuaObjects()
   clearWebHandlers();
   dnsdist::lua::hooks::clearMaintenanceHooks();
 }
+#endif /* defined(COVERAGE) || (defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE)) */
 
+#if defined(COVERAGE)
 static void sigTermHandler(int)
 {
   cleanupLuaObjects();
   pdns::coverage::dumpCoverageData();
   _exit(EXIT_SUCCESS);
 }
-#else /* COVERAGE */
-
-/* g++ defines __SANITIZE_THREAD__
-   clang++ supports the nice __has_feature(thread_sanitizer),
-   let's merge them */
-#if defined(__has_feature)
-#if __has_feature(thread_sanitizer)
-#define __SANITIZE_THREAD__ 1
-#endif
-#endif
-
+#else
 static void sigTermHandler([[maybe_unused]] int sig)
 {
 #if !defined(__SANITIZE_THREAD__)
@@ -2802,7 +2809,12 @@ static void sigTermHandler([[maybe_unused]] int sig)
   }
   std::cout << "Exiting on user request" << std::endl;
 #endif /* __SANITIZE_THREAD__ */
-
+#if defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE)
+  auto lock = g_lua.lock();
+  cleanupLuaObjects();
+  *lock = LuaContext();
+  __lsan_do_leak_check();
+#endif /* __SANITIZE_ADDRESS__ && HAVE_LEAK_SANITIZER_INTERFACE */
   _exit(EXIT_SUCCESS);
 }
 #endif /* COVERAGE */
diff --git a/regression-tests.dnsdist/dnsdisttests.py b/regression-tests.dnsdist/dnsdisttests.py
index 3f53bd451a..889d40a54f 100644
--- a/regression-tests.dnsdist/dnsdisttests.py
+++ b/regression-tests.dnsdist/dnsdisttests.py
@@ -215,6 +215,8 @@ class DNSDistTest(AssertEqualDNSMessageMixin, unittest.TestCase):
                 print("kill...", p, file=sys.stderr)
                 p.kill()
                 p.wait()
+            if p.returncode != 0:
+              raise AssertionError('Process exited with return code %d' % (p.returncode))
         except OSError as e:
             # There is a race-condition with the poll() and
             # kill() statements, when the process is dead on the