From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Mon, 13 May 2024 13:42:18 +0000 (+0200)
Subject: Tweaks to make LSAN work
X-Git-Tag: rec-5.1.0-beta1~41^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4f4340d2c024b0c84d9e6198974b576e836c7ae2;p=thirdparty%2Fpdns.git

Tweaks to make LSAN work
---

diff --git a/.github/workflows/build-and-test-all.yml b/.github/workflows/build-and-test-all.yml
index 8877a30222..c40f94bb5d 100644
--- a/.github/workflows/build-and-test-all.yml
+++ b/.github/workflows/build-and-test-all.yml
@@ -565,7 +565,8 @@ jobs:
       image: "${{ needs.get-runner-container-image.outputs.id }}:${{ needs.get-runner-container-image.outputs.tag }}"
       env:
         UBSAN_OPTIONS: 'print_stacktrace=1:halt_on_error=1:suppressions=${{ env.REPO_HOME }}/build-scripts/UBSan.supp'
-        ASAN_OPTIONS: detect_leaks=0
+        ASAN_OPTIONS: ""
+        LSAN_OPTIONS: "suppressions=${{ env.REPO_HOME }}/pdns/recursordist/recursor-lsan.supp"
         TSAN_OPTIONS: "halt_on_error=1:suppressions=${{ env.REPO_HOME }}/pdns/recursordist/recursor-tsan.supp"
       options: --sysctl net.ipv6.conf.all.disable_ipv6=0
     steps:
diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 6cacb4e66d..304718f00c 100644
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -67,6 +67,18 @@ thread_local FrameStreamServersInfo t_frameStreamServersInfo;
 thread_local FrameStreamServersInfo t_nodFrameStreamServersInfo;
 #endif /* HAVE_FSTRM */
 
+/* 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
+#endif
+#endif
+
 string g_programname = "pdns_recursor";
 string g_pidfname;
 RecursorControlChannel g_rcc; // only active in the handler thread
@@ -1870,6 +1882,10 @@ static int initForks(Logr::log_t log)
     signal(SIGTERM, termIntHandler);
     signal(SIGINT, termIntHandler);
   }
+#if defined(__SANITIZE_THREAD__) || (defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE))
+  // If san is wanted, we dump the info ourselves
+  signal(SIGTERM, termIntHandler);
+#endif
 
   signal(SIGUSR1, usr1Handler);
   signal(SIGUSR2, usr2Handler);
diff --git a/pdns/recursordist/rec_channel_rec.cc b/pdns/recursordist/rec_channel_rec.cc
index 3de3fbc6fe..1f82eb2f1c 100644
--- a/pdns/recursordist/rec_channel_rec.cc
+++ b/pdns/recursordist/rec_channel_rec.cc
@@ -42,6 +42,21 @@
 
 #include "settings/cxxsettings.hh"
 
+/* 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
+
 std::pair<std::string, std::string> PrefixDashNumberCompare::prefixAndTrailingNum(const std::string& a)
 {
   auto i = a.length();
@@ -1592,8 +1607,18 @@ void registerAllStats()
   }
 }
 
+static auto clearLuaScript()
+{
+  vector<string> empty;
+  empty.emplace_back();
+  return doQueueReloadLuaScript(empty.begin(), empty.end());
+}
+
 void doExitGeneric(bool nicely)
 {
+#if defined(__SANITIZE_THREAD__)
+  _exit(0); // regression test check for exit 0
+#endif
   g_log << Logger::Error << "Exiting on user request" << endl;
   g_rcc.~RecursorControlChannel();
 
@@ -1605,8 +1630,15 @@ void doExitGeneric(bool nicely)
     RecursorControlChannel::stop = true;
   }
   else {
+#if defined(__SANITIZE_ADDRESS__) && defined(HAVE_LEAK_SANITIZER_INTERFACE)
+    clearLuaScript();
     pdns::coverage::dumpCoverageData();
-    _exit(1);
+    __lsan_do_leak_check();
+    _exit(0); // let the regression test distinguish between leaks and no leaks as __lsan_do_leak_check() exits 1 on leaks
+#else
+    pdns::coverage::dumpCoverageData();
+    _exit(1); // for historic reasons we exit 1
+#endif
   }
 }
 
@@ -2334,9 +2366,7 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int socket, cons
     return {0, doTraceRegex(begin == end ? FDWrapper(-1) : getfd(socket), begin, end)};
   }
   if (cmd == "unload-lua-script") {
-    vector<string> empty;
-    empty.emplace_back();
-    return doQueueReloadLuaScript(empty.begin(), empty.end());
+    return clearLuaScript();
   }
   if (cmd == "reload-acls") {
     return reloadACLs();
diff --git a/pdns/recursordist/recursor-lsan.supp b/pdns/recursordist/recursor-lsan.supp
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/pdns/recursordist/recursor-tsan.supp b/pdns/recursordist/recursor-tsan.supp
index ee6d9eaeea..859837022e 100644
--- a/pdns/recursordist/recursor-tsan.supp
+++ b/pdns/recursordist/recursor-tsan.supp
@@ -1,3 +1,5 @@
 # We don't care about stats for now
 race:doStats
 race:g_stats
+# Just calling _exit in th TSAN case
+signal:termIntHandler
diff --git a/regression-tests.recursor-dnssec/recursortests.py b/regression-tests.recursor-dnssec/recursortests.py
index 59762839ca..b61a5f9d2e 100644
--- a/regression-tests.recursor-dnssec/recursortests.py
+++ b/regression-tests.recursor-dnssec/recursortests.py
@@ -759,7 +759,7 @@ distributor-threads={threads}""".format(confdir=confdir,
             return
         try:
             p.terminate()
-            for count in range(10):
+            for count in range(100): # tsan can be slow
                 x = p.poll()
                 if x is not None:
                     break