[Dynamic Instrumentation] DEBUG-2249 Line and method probes explorati…

…on tests (#5914)

## Summary of changes
This PR add line porbe exploration tests.

## Reason for change
To verify that we aren't breaking any code with our instrumentation.

## Implementation details
We instrument each method with a method probe and each method line with
a line probe.

## Test coverage
Several open source projects (e.g. protobuf.net).

---------

Co-authored-by: Matan Green <matangrn@gmail.com>

tracer/build/_build/BuildVariables.cs

@@ -1,17 +1,26 @@
 using System.Collections.Generic;
+using System.IO;
 using Nuke.Common;
-using Logger = Serilog.Log;
 
 partial class Build
 {
-    public void AddDebuggerEnvironmentVariables(Dictionary<string, string> envVars)
+    public void AddDebuggerEnvironmentVariables(Dictionary<string, string> envVars, ExplorationTestDescription description, TargetFramework framework)
     {
         AddTracerEnvironmentVariables(envVars);
 
-        if (!DisableDynamicInstrumentationProduct)
+        if (DisableDynamicInstrumentationProduct)
         {
-            envVars.Add("DD_DYNAMIC_INSTRUMENTATION_ENABLED", "1");
-            envVars.Add("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL", "1");
+            return;
+        }
+
+        envVars.Add("DD_DYNAMIC_INSTRUMENTATION_ENABLED", "1");
+        envVars.Add("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL", "1");
+
+        if (description.LineProbesEnabled)
+        {
+            envVars.Add("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL_LINES", "1");
+            var testRootPath = description.GetTestTargetPath(ExplorationTestsDirectory, framework, BuildConfiguration);
+            envVars.Add("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL_LINES_PATH", Path.Combine(testRootPath, LineProbesFileName));
         }
 
         envVars.Add("COMPlus_DbgEnableMiniDump", "1");
@@ -22,6 +31,10 @@ public void AddDebuggerEnvironmentVariables(Dictionary<string, string> envVars)
         {
             envVars.Add("DD_INTERNAL_FAULT_TOLERANT_INSTRUMENTATION_ENABLED", "true");
         }
+
+        // Uncomment to get rejit verify files
+        // envVars.Add("DD_WRITE_INSTRUMENTATION_TO_DISK", "1");
+        // envVars.Add("CopyingOriginalModulesEnabled", "1");
     }
 
     public void AddContinuousProfilerEnvironmentVariables(Dictionary<string, string> envVars)

tracer/src/Datadog.Trace/Debugger/LiveDebugger.cs

@@ -9,7 +9,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Datadog.Trace.Agent.DiscoveryService;
@@ -26,8 +25,6 @@
 using Datadog.Trace.DogStatsd;
 using Datadog.Trace.Logging;
 using Datadog.Trace.RemoteConfigurationManagement;
-using Datadog.Trace.Util;
-using Datadog.Trace.Vendors.dnlib.DotNet;
 using Datadog.Trace.Vendors.StatsdClient;
 using ProbeInfo = Datadog.Trace.Debugger.Expressions.ProbeInfo;
 

tracer/src/Datadog.Trace/PDBs/DatadogMetadataReader.cs

@@ -53,6 +53,9 @@ private DatadogMetadataReader(PEReader peReader, MetadataReader metadataReader,
             IsPdbExist = PdbReader != null || DnlibPdbReader != null;
         }
 
+        /// <summary>
+        /// Gets the pdb path if exists, otherwise the assembly location
+        /// </summary>
         internal string? PdbFullPath { get; }
 
         internal MetadataReader MetadataReader { get; }
@@ -76,7 +79,7 @@ private DatadogMetadataReader(PEReader peReader, MetadataReader metadataReader,
             if (peReader.TryOpenAssociatedPortablePdb(assembly.Location, File.OpenRead, out var metadataReaderProvider, out var pdbPath))
             {
                 pdbReader = metadataReaderProvider!.GetMetadataReader(MetadataReaderOptions.Default, MetadataStringDecoder.DefaultUTF8);
-                return new DatadogMetadataReader(peReader, metadataReader, pdbReader, pdbPath, null, null);
+                return new DatadogMetadataReader(peReader, metadataReader, pdbReader, pdbPath ?? assembly.Location, null, null);
             }
 
             if (!TryFindPdbFile(assembly.Location, out var pdbFullPath))
@@ -366,18 +369,30 @@ private MethodDefinitionHandle GetMoveNextMethod(MethodDefinition methodDef)
 
             if (PdbReader != null)
             {
+                var normalizeFilePath = GetNormalizedPath(filePath);
+                if (string.IsNullOrEmpty(normalizeFilePath))
+                {
+                    return null;
+                }
+
                 const int methodDefTablePrefix = 0x06000000;
                 foreach (MethodDefinitionHandle methodDefinitionHandle in MetadataReader.MethodDefinitions)
                 {
                     MethodDebugInformation methodDebugInformation = PdbReader.GetMethodDebugInformation(methodDefinitionHandle);
 
                     foreach (VendoredMicrosoftCode.System.Reflection.Metadata.SequencePoint sequencePoint in methodDebugInformation.GetSequencePoints())
                     {
-                        if (sequencePoint.IsHidden || GetDocumentName(sequencePoint.Document) != filePath)
+                        if (sequencePoint.IsHidden)
                         {
                             continue;
                         }
 
+                        var normalizeDocumentPath = GetNormalizedPath(GetDocumentName(sequencePoint.Document));
+                        if (!string.Equals(normalizeDocumentPath, normalizeFilePath, StringComparison.OrdinalIgnoreCase))
+                        {
+                            break;
+                        }
+
                         // Check if the line and column match
                         if (sequencePoint.StartLine <= line && sequencePoint.EndLine >= line &&
                             (column.HasValue == false || (sequencePoint.StartColumn <= column && sequencePoint.EndColumn >= column)))
@@ -390,6 +405,11 @@ private MethodDefinitionHandle GetMoveNextMethod(MethodDefinition methodDef)
             }
 
             return null;
+
+            string? GetNormalizedPath(string? path)
+            {
+                return path == null ? null : Path.GetFullPath(path).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+            }
         }
 
         internal IList<string>? GetDocuments()

tracer/src/Datadog.Tracer.Native/debugger_environment_variables.h

@@ -13,6 +13,8 @@ namespace environment
     // Determine whether to enter "instrument all" mode where the Debugger instrumentation
     // is applied to every jit compiled method. Only useful for testing purposes. Default is false.
     const WSTRING internal_instrument_all_enabled = WStr("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL");
+    const WSTRING internal_instrument_all_lines_enabled = WStr("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL_LINES");
+    const WSTRING internal_instrument_all_lines_path = WStr("DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL_LINES_PATH");
 
     // Determines if the Dynamic Instrumentation (aka live debugger) is enabled.
     const WSTRING debugger_enabled = WStr("DD_DYNAMIC_INSTRUMENTATION_ENABLED");

tracer/src/Datadog.Tracer.Native/debugger_environment_variables_util.cpp

@@ -26,4 +26,9 @@ bool IsDebuggerInstrumentAllEnabled()
     CheckIfTrue(GetEnvironmentValue(environment::internal_instrument_all_enabled));
 }
 
+bool IsDebuggerInstrumentAllLinesEnabled()
+{
+    CheckIfTrue(GetEnvironmentValue(environment::internal_instrument_all_lines_enabled));
+}
+
 } // namespace debugger

tracer/src/Datadog.Tracer.Native/debugger_environment_variables_util.h

@@ -11,6 +11,7 @@ namespace debugger
 bool IsDebuggerEnabled();
 bool IsExceptionReplayEnabled();
 bool IsDebuggerInstrumentAllEnabled();
+bool IsDebuggerInstrumentAllLinesEnabled();
 
 } // namespace debugger
 

tracer/src/Datadog.Tracer.Native/debugger_probes_instrumentation_requester.cpp

@@ -10,7 +10,12 @@
 #include "debugger_rejit_handler_module_method.h"
 #include "debugger_rejit_preprocessor.h"
 #include "fault_tolerant_method_duplicator.h"
+#include "fault_tolerant_tracker.h"
+#include "iast/iast_util.h"
 #include "logger.h"
+
+#include <fstream>
+#include <map>
 #include <random>
 
 namespace debugger
@@ -97,25 +102,74 @@ WSTRING DebuggerProbesInstrumentationRequester::GenerateRandomProbeId()
     return converted;
 }
 
+auto& DebuggerProbesInstrumentationRequester::GetExplorationTestLineProbes(const WSTRING& filename)
+{
+    std::call_once(explorationLinesInitFlag, [this, fn = filename] {
+        DebuggerProbesInstrumentationRequester::InitializeExplorationTestLineProbes(fn);
+    });
+    return explorationLineProbes;
+}
+
+void DebuggerProbesInstrumentationRequester::InitializeExplorationTestLineProbes(const WSTRING& filename)
+{
+#ifdef _WIN32
+    std::ifstream file(filename);
+#else
+    std::ifstream file(ToString(filename).c_str());
+#endif
+
+    if (!file.is_open())
+    {
+        Logger::Error("InitializeExplorationTestLineProbes: Unable to open file: ", filename);
+        return;
+    }
+
+    std::string line;
+    while (std::getline(file, line))
+    {
+        std::istringstream lineStream(line);
+        std::string guid;
+        std::string methodTokenStr;
+        std::string bytecodeOffsetStr;
+
+        // Split the line by commas
+        if (std::getline(lineStream, guid, ',') && std::getline(lineStream, methodTokenStr, ',') &&
+            std::getline(lineStream, bytecodeOffsetStr, ','))
+        {
+            auto methodToken = static_cast<mdToken>(std::stoul(methodTokenStr));
+            int bytecodeOffset = std::stoi(bytecodeOffsetStr);
+
+            auto key = std::make_pair(guid, methodToken);
+            explorationLineProbes[key].push_back(bytecodeOffset);
+        }
+    }
+
+    file.close();
+}
+
 /**
  * \brief For Testing-Purposes. Requests ReJIT for the given method if certain checks are met. Relevant when the
  * environment variable `DD_INTERNAL_DEBUGGER_INSTRUMENT_ALL` is set to true. \param module_info the ModuleInfo of the
  * module entering into instrumentation-all. \param module_id the ModuleID of the module entering into
  * instrumentation-all. \param function_token the mdToken of the method entering into instrumentation-all.
  */
-void DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded(const ModuleID& module_id,
-                                                                          const mdToken& function_token)
+void DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded(const ModuleID& module_id, mdToken& function_token)
 {
-    if (!IsDebuggerInstrumentAllEnabled())
+    try
     {
-        return;
-    }
+        if (!IsDebuggerInstrumentAllEnabled())
+        {
+            return;
+        }
 
-    const auto& module_info = GetModuleInfo(m_corProfiler->info_, module_id);
-    const auto assembly_name = module_info.assembly.name;
+        const auto& module_info = GetModuleInfo(m_corProfiler->info_, module_id);
+        const auto assembly_name = module_info.assembly.name;
+
+        if (IsCoreLibOr3rdParty(assembly_name))
+        {
+            return;
+        }
 
-    if (!IsCoreLibOr3rdParty(assembly_name))
-    {
         ComPtr<IUnknown> metadataInterfaces;
         auto hr = m_corProfiler->info_->GetModuleMetaData(module_id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                           metadataInterfaces.GetAddressOf());
@@ -125,7 +179,7 @@ void DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded(const
         auto assemblyImport = metadataInterfaces.As<IMetaDataAssemblyImport>(IID_IMetaDataAssemblyImport);
         auto assemblyEmit = metadataInterfaces.As<IMetaDataAssemblyEmit>(IID_IMetaDataAssemblyEmit);
 
-        Logger::Debug("Temporaly allocating the ModuleMetadata for injection. ModuleId=", module_id,
+        Logger::Debug("Temporally allocating the ModuleMetadata for injection. ModuleId=", module_id,
                       " ModuleName=", module_info.assembly.name);
 
         std::unique_ptr<ModuleMetadata> module_metadata = std::make_unique<ModuleMetadata>(
@@ -143,9 +197,10 @@ void DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded(const
         hr = caller.method_signature.TryParse();
         if (FAILED(hr))
         {
-            Logger::Warn(
-                " * DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded: The method signature: ",
-                caller.method_signature.str(), " cannot be parsed.");
+            Logger::Warn("    * DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded: [MethodDef=",
+                         shared::TokenStr(&function_token), ", Type=", caller.type.name, ", Method=", caller.name, "]",
+                         ": could not parse method signature.");
+            Logger::Debug("    Method signature is: ", caller.method_signature.str());
             return;
         }
 
@@ -180,9 +235,82 @@ void DebuggerProbesInstrumentationRequester::PerformInstrumentAllIfNeeded(const
             /* is_exact_signature_match */ false));
 
         const auto numReJITs = m_debugger_rejit_preprocessor->RequestRejitForLoadedModules(
-            std::vector{module_id}, std::vector{methodProbe},
-            /* enqueueInSameThread */ true);
-        Logger::Debug("Instrument-All: Total number of ReJIT Requested: ", numReJITs);
+            std::vector{module_id}, std::vector{methodProbe}, /* enqueueInSameThread */ true);
+
+        Logger::Debug("Instrument-All: ReJIT Requested for: ", methodProbe->target_method.method_name,
+                      ". ProbeId:", methodProbe->probeId, ". Numbers of ReJits: ", numReJITs);
+
+        if (!IsDebuggerInstrumentAllLinesEnabled())
+        {
+            return;
+        }
+
+        const auto lineProbesPath = GetEnvironmentValue(environment::internal_instrument_all_lines_path);
+        const shared::WSTRING& probeFilePath = WStr("line_exploration_file_path");
+
+        auto lineProbesMap = GetExplorationTestLineProbes(lineProbesPath);
+
+        WCHAR moduleName[MAX_PACKAGE_NAME];
+        ULONG nSize;
+        GUID mvid;
+        hr = metadataImport->GetScopeProps(moduleName, MAX_PACKAGE_NAME, &nSize, &mvid);
+
+        if (FAILED(hr))
+        {
+            Logger::Warn("Can't read module scope props for module ID: ", module_id);
+        }
+        else
+        {
+            auto original_function_token = function_token;
+
+            if (fault_tolerant::FaultTolerantTracker::Instance()->IsInstrumentedMethod(module_id, function_token))
+            {
+                function_token =
+                    fault_tolerant::FaultTolerantTracker::Instance()->GetKickoffMethodFromInstrumentedMethod(
+                        module_id, function_token);
+            }
+
+            auto lowerMvid = iast::ToLower(ToString(mvid));
+            lowerMvid = lowerMvid.substr(1, lowerMvid.length() - 2);
+            auto key = std::make_pair(lowerMvid, function_token);
+            auto it = lineProbesMap.find(key);
+            if (it == lineProbesMap.end())
+            {
+                Logger::Warn("No line probes found in method: ", caller.type.name, ".", caller.name);
+            }
+            else
+            {
+                std::vector<std::shared_ptr<LineProbeDefinition>> lineProbeDefinitions;
+                auto bytecodeOffsets = it->second;
+                for (const auto& offset : bytecodeOffsets)
+                {
+                    const auto& lineProbe = std::make_shared<LineProbeDefinition>(LineProbeDefinition(
+                        GenerateRandomProbeId(), offset, offset, mvid, original_function_token, probeFilePath));
+                    lineProbeDefinitions.push_back(lineProbe);
+                }
+
+                std::promise<std::vector<MethodIdentifier>> promise;
+                std::future<std::vector<MethodIdentifier>> future = promise.get_future();
+                m_debugger_rejit_preprocessor->EnqueuePreprocessLineProbes(std::vector{module_id}, lineProbeDefinitions,
+                                                                           &promise);
+                const auto& lineProbeRequests = future.get();
+
+                // RequestRejit
+                auto requestRejitPromise = std::make_shared<std::promise<void>>();
+                std::future<void> requestRejitFuture = requestRejitPromise->get_future();
+                std::vector<MethodIdentifier> requests(lineProbeRequests.size());
+                std::copy(lineProbeRequests.begin(), lineProbeRequests.end(), requests.begin());
+                m_debugger_rejit_preprocessor->EnqueueRequestRejit(requests, requestRejitPromise);
+                // wait and get the value from the future<void>
+                requestRejitFuture.get();
+                Logger::Debug("Instrument-All: ReJIT Requested for: ", methodProbe->target_method.method_name,
+                              ". Numbers of line probes: ", lineProbeDefinitions.size());
+            }
+        }
+    }
+    catch (...)
+    {
+        Logger::Error("PerformInstrumentAllIfNeeded: fail for module: ", module_id, ". method: ", function_token);
     }
 }
 
@@ -192,7 +320,7 @@ DebuggerProbesInstrumentationRequester::DebuggerProbesInstrumentationRequester(
     std::shared_ptr<fault_tolerant::FaultTolerantMethodDuplicator> fault_tolerant_method_duplicator) :
     m_corProfiler(corProfiler),
     m_debugger_rejit_preprocessor(
-    std::make_unique<DebuggerRejitPreprocessor>(corProfiler, rejit_handler, work_offloader)),
+        std::make_unique<DebuggerRejitPreprocessor>(corProfiler, rejit_handler, work_offloader)),
     m_rejit_handler(rejit_handler),
     m_work_offloader(work_offloader),
     m_fault_tolerant_method_duplicator(fault_tolerant_method_duplicator)
@@ -424,8 +552,7 @@ void DebuggerProbesInstrumentationRequester::AddMethodProbes(debugger::DebuggerM
 
     auto promise = std::make_shared<std::promise<std::vector<MethodIdentifier>>>();
     std::future<std::vector<MethodIdentifier>> future = promise->get_future();
-    m_debugger_rejit_preprocessor->EnqueuePreprocessRejitRequests(modules.Ref(), methodProbeDefinitions,
-                                                                  promise);
+    m_debugger_rejit_preprocessor->EnqueuePreprocessRejitRequests(modules.Ref(), methodProbeDefinitions, promise);
 
     const auto& methodProbeRequests = future.get();
 
@@ -471,8 +598,8 @@ void DebuggerProbesInstrumentationRequester::AddLineProbes(debugger::DebuggerLin
 
             const shared::WSTRING& probeId = shared::WSTRING(current.probeId);
             const shared::WSTRING& probeFilePath = shared::WSTRING(current.probeFilePath);
-            const auto& lineProbe = std::make_shared<LineProbeDefinition>(LineProbeDefinition(probeId, current.bytecodeOffset, current.lineNumber,
-                                                        current.mvid, current.methodId, probeFilePath));
+            const auto& lineProbe = std::make_shared<LineProbeDefinition>(LineProbeDefinition(
+                probeId, current.bytecodeOffset, current.lineNumber, current.mvid, current.methodId, probeFilePath));
 
             lineProbeDefinitions.push_back(lineProbe);
         }