Fix native and managed callstacks merge

Author: gleocadie

profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.cpp

@@ -355,17 +355,19 @@ int32_t CrashReporting::ExportImpl(ddog_Endpoint* endpoint)
 std::vector<StackFrame> CrashReporting::MergeFrames(const std::vector<StackFrame>& nativeFrames, const std::vector<StackFrame>& managedFrames)
 {
     std::vector<StackFrame> result;
+    // it's safe here to not use nativeFrames.size() + managedFrames.size()
+    // because the managed frames should be a subset of the native frames
     result.reserve(std::max(nativeFrames.size(), managedFrames.size()));
 
     size_t i = 0, j = 0;
     while (i < nativeFrames.size() && j < managedFrames.size())
     {
-        if (nativeFrames[i].sp < managedFrames[j].sp)
+        if (nativeFrames[i].sp > managedFrames[j].sp)
         {
             result.push_back(nativeFrames[i]);
             ++i;
         }
-        else if (managedFrames[j].sp < nativeFrames[i].sp)
+        else if (managedFrames[j].sp > nativeFrames[i].sp)
         {
             result.push_back(managedFrames[j]);
             ++j;

profiler/src/ProfilerEngine/Datadog.Profiler.Native/CrashReporting.h

@@ -143,6 +143,9 @@ class CrashReporting : public ICrashReporting
     virtual std::vector<StackFrame> GetThreadFrames(int32_t tid, ResolveManagedCallstack resolveManagedCallstack, void* context) = 0;
     virtual std::string GetSignalInfo(int32_t signal) = 0;
 
+#ifdef DD_TEST
+    friend class CrashReportingTest_CheckMergedCallstack_Test;
+#endif
     static std::vector<StackFrame> MergeFrames(const std::vector<StackFrame>& nativeFrames, const std::vector<StackFrame>& managedFrames);
 private:
     int32_t ExportImpl(ddog_Endpoint* endpoint);

profiler/test/Datadog.Profiler.Native.Tests/CrashReportingTest.cpp

@@ -1,10 +1,20 @@
 // Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc.
 
+#include "gtest/gtest.h"
+
+#include "unknwn.h"
+
+const IID IID_IUnknown = {0x00000000,
+    0x0000,
+    0x0000,
+    {0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46}};
+    
+#include "CrashReporting.h"
+
 #ifdef _WIN32
 
 #include "resource.h"
-#include "gtest/gtest.h"
 #include <string_view>
 #include <windows.h>
 #include <vector>
@@ -130,5 +140,38 @@ TEST(CrashReportingTest, ExtractPdbSignaturePE64)
     //                                  |        Pdb signature          |Age|
     ASSERT_STRCASEEQ(buildIdStr.data(), "C465AFCDBDBC58A0100995839A0E4C271");
 }
+#endif
+
 
-#endif
+TEST(CrashReportingTest, CheckMergedCallstack)
+{
+    std::vector<StackFrame> nativeFrames = {
+        {0x7F4DECDF2BC0, 0x7F478000ACE0, "__GI___wait4", 0x7F4DECDF2BC0, 0x7F4DECD1F000, false, ""},
+        {0x7F4DECB882F0, 0x7F478000AD10, "PROCCreateCrashDump(std::vector<char const*, std::allocator<char const*> >&, char*, int, bool)", 0x7F4DECB882F0, 0x7F4DEC514000, false, ""},
+        {0x7F4D778E1A7D, 0x7F476CC3E9D0, "/memfd:doublemapper (deleted)!<unknown>+b84da7d", 0x7F4D778E1A7D, 0x7F4D6C094000, false, ""},
+        {0x7F4D76593D2A, 0x7F476CC3EA10, "/memfd:doublemapper (deleted)!<unknown>+a4ffd2a", 0x7F4D76593D2A, 0x7F4D6C094000, false, ""},
+        {0x7F4D6D7D3924, 0x7F476CC3EA90, "/usr/share/dotnet/shared/Microsoft.NETCore.App/9.0.10/System.Private.CoreLib.dll!<unknown>+5e370b924", 0x7F4D6D7D3924, 0x7F478A0C8000, false, ""},
+    };
+
+    std::vector<StackFrame> managedFrames = {
+        {0x7F4D778E1A7D, 0x7F476CC3E9D0, "System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Threading.Tasks.VoidTaskResult>+AsyncStateMachineBox<Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol+<ProcessRequests>d__238<System.__Canon>>.MoveNext", 0x7F4D778E1A7D, 0x7F4D6C094000, false, ""},
+        {0x7F4D76593D2A, 0x7F476CC3EA10, "System.Private.CoreLib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch", 0x7F4D76593D2A, 0x7F4D6C094000, false, ""},
+        {0x7F4D6D7D3924, 0x7F476CC3EA90, "System.Private.CoreLib.dll!System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart", 0x7F4D6D7D3924, 0x7F478A0C8000, false, ""},
+    };
+
+    auto mergedFrames = CrashReporting::MergeFrames(nativeFrames, managedFrames);
+    
+    std::vector<std::string> expectedFunctions = {
+        "__GI___wait4",
+        "PROCCreateCrashDump(std::vector<char const*, std::allocator<char const*> >&, char*, int, bool)",
+        "System.Private.CoreLib.dll!System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Threading.Tasks.VoidTaskResult>+AsyncStateMachineBox<Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol+<ProcessRequests>d__238<System.__Canon>>.MoveNext",
+        "System.Private.CoreLib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch",
+        "System.Private.CoreLib.dll!System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart",
+    };
+    
+    ASSERT_EQ(mergedFrames.size(), expectedFunctions.size());
+    for (size_t i = 0; i < mergedFrames.size(); i++)
+    {
+        ASSERT_EQ(mergedFrames[i].method, expectedFunctions[i]);
+    }
+}