[Crashtracking] Filter the ptrace_create hook (#6337)

## Summary of changes When marking a crash as suspicious or not, ignore the entrypoint inserted by the ptrace_create hook. ## Reason for change The entrypoint is inserted into every new thread. Because of that, pretty much every crash was marked as suspicious. ## Implementation details Because the crashtracker doesn't have access to symbols, we had to publicly expose the inserted entrypoint. To filter it more easily, it has been renamed to `dd_pthread_entry`. ## Test coverage Added a new test case that triggers a crash in a new thread. The issue wasn't discovered because the existing test was crashing in the main thread, so it didn't have the injected entrypoint. --------- Co-authored-by: Gregory LEOCADIE <[email protected]>
DataDog · Nov 22, 2024 · 3e4fbeb · 3e4fbeb
1 parent 6cb0268
commit 3e4fbeb
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 11 deletions.
diff --git a/profiler/src/ProfilerEngine/Datadog.Linux.ApiWrapper/functions_to_wrap.c b/profiler/src/ProfilerEngine/Datadog.Linux.ApiWrapper/functions_to_wrap.c
@@ -547,8 +547,8 @@ struct pthread_wrapped_arg
     void* orig_arg;
 };
 
-__attribute__((visibility("hidden")))
-static void* entry2(void* arg)
+// This symbol must be public for crashtracking filtering mechanism
+void* dd_pthread_entry(void* arg)
 {
     struct pthread_wrapped_arg* new_arg = (struct pthread_wrapped_arg*)arg;
     void* result = new_arg->func(new_arg->orig_arg);
@@ -573,7 +573,7 @@ int pthread_create(pthread_t* restrict res, const pthread_attr_t* restrict attrp
     new_arg->func = entry;
     new_arg->orig_arg = arg;
 
-    int result = __real_pthread_create(res, attrp, entry2, new_arg);
+    int result = __real_pthread_create(res, attrp, dd_pthread_entry, new_arg);
 
     ((char*)&functions_entered_counter)[ENTERED_PTHREAD_CREATE]--;
 

diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/CrashReportingLinux.cpp
@@ -248,7 +248,7 @@ std::vector<StackFrame> CrashReportingLinux::GetThreadFrames(int32_t tid, Resolv
         {
             const auto moduleFilename = modulePath.stem().string();
 
-            if (moduleFilename.rfind("Datadog", 0) == 0
+            if ((moduleFilename.rfind("Datadog", 0) == 0 && stackFrame.method != "dd_pthread_entry")
                 || moduleFilename == "libdatadog"
                 || moduleFilename == "datadog"
                 || moduleFilename == "libddwaf"

diff --git a/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs b/tracer/test/Datadog.Trace.Tools.dd_dotnet.ArtifactTests/CreatedumpTests.cs
@@ -379,7 +379,7 @@ public async Task CheckThreadName()
         using var reportFile = new TemporaryFile();
 
         using var helper = await StartConsoleWithArgs(
-                               "crash-thread",
+                               "crash-thread-datadog",
                                enableProfiler: true,
                                [LdPreloadConfig, CrashReportConfig(reportFile)]);
 
@@ -445,16 +445,20 @@ bool IsCrashReport(object payload)
         agent.WaitForLatestTelemetry(IsCrashReport).Should().NotBeNull();
     }
 
-    [SkippableFact]
-    public async Task IgnoreNonDatadogCrashes()
+    [SkippableTheory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task IgnoreNonDatadogCrashes(bool mainThread)
     {
         SkipOn.Platform(SkipOn.PlatformValue.MacOs);
         SkipOn.PlatformAndArchitecture(SkipOn.PlatformValue.Windows, SkipOn.ArchitectureValue.X86);
 
         using var reportFile = new TemporaryFile();
 
+        var arg = mainThread ? "crash" : "crash-thread";
+
         using var helper = await StartConsoleWithArgs(
-                               "crash",
+                               arg,
                                enableProfiler: true,
                                [LdPreloadConfig, CrashReportConfig(reportFile)]);
 

diff --git a/tracer/test/test-applications/integrations/Samples.Console/Program.cs b/tracer/test/test-applications/integrations/Samples.Console/Program.cs
@@ -35,12 +35,16 @@ void DoCrash()
                     progress.Report(exception);
                 }
 
-                if (args[0] == "crash-thread")
+                if (args[0].StartsWith("crash-thread"))
                 {
                     var thread = new Thread(
                         () =>
                         {
-                            SetCurrentThreadName("DD_thread");
+                            if (args[0] == "crash-thread-datadog")
+                            {
+                                SetCurrentThreadName("DD_thread");
+                            }
+
                             DoCrash();
                         });
 
@@ -198,7 +202,7 @@ private static unsafe IntPtr CreateCrashReport(int pid)
 
             var folder = Path.GetDirectoryName(Environment.GetEnvironmentVariable(profilerPathEnvironmentVariable));
             var profilerPath = Path.Combine(folder, "Datadog.Profiler.Native" + (Environment.OSVersion.Platform == PlatformID.Win32NT ? ".dll" : ".so"));
-            
+
             var arguments = new object[] { profilerPath, null };
             tryLoad.Invoke(null, arguments);