[Profiler] Cleanup generic method frame encoding and namespaceless types (#8961)

## Summary of changes
Ensure that no '.' is added when there is no namespace and provide full
type name in generics

## Reason for change
Weird `<.<Foo>>` or `.Bar` type names in frames

## Implementation details
- Fix namespaceless case
- Reuse helper to get full type names in generics

## Test coverage
Fix regex and add dedicated tests for better coverage

## Other details
<!-- Fixes #{issue} -->


<!--  ⚠️ Note:

Where possible, please obtain 2 approvals prior to merging. Unless
CODEOWNERS specifies otherwise, for external teams it is typically best
to have one review from a team member, and one review from apm-dotnet.
Trivial changes do not require 2 reviews.

MergeQueue is NOT enabled in this repository. If you have write access
to the repo, the PR has 1-2 approvals (see above), and all of the
required checks have passed, you can use the Squash and Merge button to
merge the PR. If you don't have write access, or you need help, reach
out in the #apm-dotnet channel in Slack.
-->

Author: chrisnas

profiler/src/Demos/Samples.Computer01/GlobalStruct.cs

@@ -0,0 +1,17 @@
+// <copyright file="GlobalStruct.cs" company="Datadog">
+// 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.
+// </copyright>
+
+#pragma warning disable SA1401 // Fields should be private
+
+
+// This value type is deliberately declared outside of any namespace: when it is given as the type
+// argument of a generic method, the |fg: part of the frame must not be prefixed by a '.' standing
+// for the empty namespace.
+internal struct GlobalStruct
+{
+    public int Member;
+}
+
+#pragma warning restore SA1401 // Fields should be private

profiler/src/Demos/Samples.Computer01/MethodsSignature.cs

@@ -7,6 +7,7 @@
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 using System.Threading;
+using System.Threading.Tasks;
 
 #pragma warning disable SA1402 // File may only contain a single type
 #pragma warning disable SA1401 // Fields should be private
@@ -22,7 +23,58 @@ public override void OnProcess()
             // because at least one of each type will be sampled.
             // Allocations beyond 100 KB would be fine except that we could not test
             // the .NET Framework runtime
-            TriggerExceptions();
+            WithResult().GetAwaiter().GetResult();
+        }
+
+        // Same shape as BuggyBits ProductsController.WithResult() / GetSyncOverAsync(): the exception
+        // is thrown from the synchronous segment of an async method, so the compiler-generated state
+        // machines and AsyncMethodBuilderCore.Start<TStateMachine> are on the stack when it is
+        // captured. Each state machine is a type nested in MethodsSignature: its name must keep both
+        // the namespace and the enclosing type.
+        [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
+        private async Task<int> WithResult()
+        {
+            return await GetSyncOverAsync();
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
+        private async Task<int> GetSyncOverAsync()
+        {
+            // nothing is awaited before this call: the whole chain below runs in the synchronous
+            // segment of the state machine, i.e. under AsyncMethodBuilderCore.Start<...>
+            RunStateMachine();
+
+            await Task.Yield();
+            return 42;
+        }
+
+        // The state machines generated by the compiler for the two methods above are structs in release
+        // builds but classes in debug builds. In the latter case, the shared canonical instantiation of
+        // Start<TStateMachine> is used, so the exact state machine type is not available to the profiler.
+        // Driving our own state machine, always a struct, gives the same frames in both configurations.
+        [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
+        private void RunStateMachine()
+        {
+            var builder = AsyncTaskMethodBuilder.Create();
+            var stateMachine = new SyncOverAsyncStateMachine { Owner = this };
+
+            // no need to complete the builder: nothing awaits its task and the exceptions thrown by
+            // the chain below are caught before they could escape MoveNext()
+            builder.Start(ref stateMachine);
+        }
+
+        private struct SyncOverAsyncStateMachine : IAsyncStateMachine
+        {
+            public MethodsSignature Owner;
+
+            public void MoveNext()
+            {
+                Owner.TriggerExceptions();
+            }
+
+            public void SetStateMachine(IAsyncStateMachine stateMachine)
+            {
+            }
         }
 
         // call methods with different signatures
@@ -132,6 +184,13 @@ private void ThrowGenericMethod1<T>(T element)
             }
             else
             if (element is MyStruct)
+            {
+                GlobalStruct gs;
+                gs.Member = 42;
+                ThrowGenericMethod1(gs);
+            }
+            else
+            if (element is GlobalStruct)
             {
                 ThrowGenericMethod1("this");
             }

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

@@ -609,12 +609,13 @@ std::tuple<ULONG, std::string, std::string, mdTypeDef> FrameStore::GetMethodName
         }
         else // normal namespace.type case
         {
+            // a type declared outside of any namespace has no namespace: don't prefix it with a '.'
             if (!ns.empty())
             {
-                builder << ns;
+                builder << ns << ".";
             }
 
-            builder << "." << typeName;
+            builder << typeName;
         }
 
         if (i < genericParametersCount - 1)
@@ -1117,30 +1118,17 @@ std::pair<std::string, std::string> FrameStore::GetManagedTypeName(ICorProfilerI
         return std::make_pair("", "T");
     }
 
-    IMetaDataImport2* pMetadata;
-    hr = pInfo->GetModuleMetaData(moduleId, ofRead, IID_IMetaDataImport2, reinterpret_cast<IUnknown**>(&pMetadata));
+    ComPtr<IMetaDataImport2> pMetadata;
+    hr = pInfo->GetModuleMetaData(moduleId, ofRead, IID_IMetaDataImport2, reinterpret_cast<IUnknown**>(pMetadata.GetAddressOf()));
     if (FAILED(hr))
     {
         return std::make_pair("", "T");
     }
 
-    std::string typeName = GetTypeNameFromMetadata(pMetadata, mdTypeToken);
-    pMetadata->Release();
-    if (typeName.empty())
-    {
-        return std::make_pair("", "T");
-    }
-
-    // look for the namespace
-    auto const pos = typeName.find_last_of('.');
-    if (pos == std::string::npos)
-    {
-        // no namespace
-        return std::make_pair("", std::move(typeName));
-    }
-
-    // need to split to get the namespace and type name
-    return std::make_pair(typeName.substr(0, pos), typeName.substr(pos + 1));
+    // the namespace and the enclosing types are not part of the metadata name of a nested type
+    // (such as the state machine generated for an async method): GetTypeWithNamespace() rebuilds
+    // them and, for a type that is not nested, splits the namespace from the type name
+    return GetTypeWithNamespace(pMetadata.Get(), mdTypeToken);
 }
 
 // use Peter Sollich way in ClrProfiler to parse the binary signature

profiler/test/Datadog.Profiler.IntegrationTests/Helpers/EnvironmentHelper.cs

@@ -371,7 +371,7 @@ private string GetNativeLoaderPath()
             if (!IsRunningInCi())
             {
                 // native loader output folder
-                var binFolder = Path.Combine(GetSolutionDirectory(), "native-bin", "Datadog.Trace.ClrProfiler.Native", "bin");
+                var binFolder = Path.Combine(GetSolutionDirectory(), "artifacts", "native-bin", "Datadog.Trace.ClrProfiler.Native");
 
                 return GetOS() switch
                 {

profiler/test/Datadog.Profiler.IntegrationTests/Helpers/StackFrame.cs

@@ -28,7 +28,7 @@ public StackFrame(string rawStackFrame)
         public StackFrame(string rawStackFrame, string filename, long line)
         {
             // |lm:Datadog.Demos.ExceptionGenerator |ns:ExceptionGenerator |ct:ExceptionsProfilerTestScenario |fn:Throw1_2
-            var match = Regex.Match(rawStackFrame, @"^\|lm:(?<module>.*) \|ns:(?<namespace>.*) \|ct:(?<type>.*) \|cg:(?<typeAdorn>.*) \|fn:(?<function>.*) \|fg:(?<functionArdorn>.*) \|sg:(?<signature>.*)$");
+            var match = Regex.Match(rawStackFrame, @"^\|lm:(?<module>.*) \|ns:(?<namespace>.*) \|ct:(?<type>.*) \|cg:(?<typeAdorn>.*) \|fn:(?<function>.*) \|fg:(?<functionAdorn>.*) \|sg:(?<signature>.*)$");
 
             if (!match.Success)
             {

profiler/test/Datadog.Profiler.IntegrationTests/Signature/SignatureTest.cs

@@ -23,6 +23,17 @@ public SignatureTest(ITestOutputHelper output)
 
         [TestAppFact("Samples.Computer01", new[] { "net48", "netcoreapp3.1", "net6.0", "net8.0", })] // FIXME: .NET 9 skipping .NET 9 for now
         public void ValidateSignatures(string appName, string framework, string appAssembly)
+        {
+            CheckExceptionsInProfiles(framework, GetExceptionSamples(appName, framework, appAssembly));
+        }
+
+        [TestAppFact("Samples.Computer01", new[] { "net48", "netcoreapp3.1", "net6.0", "net8.0", })] // FIXME: .NET 9 skipping .NET 9 for now
+        public void ValidateAsyncStateMachineSignatures(string appName, string framework, string appAssembly)
+        {
+            CheckAsyncStateMachineFrames(GetExceptionSamples(appName, framework, appAssembly));
+        }
+
+        private (string Type, string Message, long Count, StackTrace Stacktrace)[] GetExceptionSamples(string appName, string framework, string appAssembly)
         {
             var runner = new TestApplicationRunner(appName, framework, appAssembly, _output, commandLine: "--scenario 20");
             EnvironmentHelper.DisableDefaultProfilers(runner);
@@ -33,8 +44,47 @@ public void ValidateSignatures(string appName, string framework, string appAssem
             runner.Run(agent);
             Assert.True(agent.NbCallsOnProfilingEndpoint > 0);
 
-            var exceptionSamples = SamplesHelper.ExtractExceptionSamples(runner.Environment.PprofDir).ToArray();
-            CheckExceptionsInProfiles(framework, exceptionSamples);
+            return SamplesHelper.ExtractExceptionSamples(runner.Environment.PprofDir).ToArray();
+        }
+
+        private static void CheckAsyncStateMachineFrames((string Type, string Message, long Count, StackTrace Stacktrace)[] exceptionSamples)
+        {
+            // A state machine is a type nested in the type declaring the async method: the generic
+            // parameter of Start<TStateMachine> must carry its namespace and its enclosing type, exactly
+            // like the signature already does. The frame is AsyncMethodBuilderCore.Start on .NET Core
+            // and AsyncTaskMethodBuilder.Start on .NET Framework.
+            var frames = exceptionSamples
+                .SelectMany(sample => Enumerable.Range(0, sample.Stacktrace.FramesCount).Select(i => sample.Stacktrace[i]))
+                .Distinct()
+                .ToArray();
+
+            // split code to debug more easily outside of LINQ
+            var startFrames = frames
+                .Where(frame => frame.Function == "Start")
+                .Distinct()
+                .ToArray();
+
+            var startAdornments = startFrames
+                .Select(frame => frame.FunctionAdornment)
+                .Distinct()
+                .ToArray();
+
+#if DEBUG
+            // MethodsSignature drives its own state machine: a struct is never shared between
+            // instantiations so its exact type is always known, whatever the configuration used to
+            // build the sample
+            startAdornments.Should().Contain("<Samples.Computer01.MethodsSignature.SyncOverAsyncStateMachine>");
+
+            // the state machines generated by the compiler are structs in release builds but classes in
+            // debug builds: in the latter case, only the shared canonical instantiation is known (<T0>)
+            startAdornments.Should().Contain("<T0>");
+#else
+            startAdornments.Should().Contain("<Samples.Computer01.MethodsSignature.<GetSyncOverAsync>d__2>");
+            startAdornments.Should().Contain("<Samples.Computer01.MethodsSignature.<WithResult>d__1>");
+#endif
+
+            // regression test to check the "type with empty namespace" bug is fixed
+            frames.Should().OnlyContain(frame => !frame.FunctionAdornment.StartsWith("<."));
         }
 
         private static void CheckExceptionsInProfiles(string framework, (string Type, string Message, long Count, StackTrace Stacktrace)[] exceptionSamples)
@@ -56,6 +106,7 @@ private static void CheckExceptionsInProfiles(string framework, (string Type, st
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:GenericClassForValueTypeTest |cg:<System.Int32, System.Boolean> |fn:ThrowOneGenericFromType |fg: |sg:(TKey value)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod2 |fg:<T0, System.Int32, T2, T3> |sg:(T0 key1, System.Int32 value1, System.Int32 value2, T2 key2, T3 key3, System.Collections.Generic.List<System.Int32> listOfTValue)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<T0> |sg:(T0 element)"),
+                    new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<GlobalStruct> |sg:(GlobalStruct element)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<Samples.Computer01.MyStruct> |sg:(Samples.Computer01.MyStruct element)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<T0> |sg:(T0 element)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<System.Boolean> |sg:(System.Boolean element)"),
@@ -80,6 +131,7 @@ private static void CheckExceptionsInProfiles(string framework, (string Type, st
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:GenericClassForValueTypeTest |cg:<System.Int32, System.Boolean> |fn:ThrowOneGenericFromType |fg: |sg:(TVal value)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod2 |fg:<T0, System.Int32, T2, T3> |sg:(T0 key1, System.Int32 value1, System.Int32 value2, T2 key2, T3 key3, System.Collections.Generic.List<System.Int32> listOfTValue)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<T0> |sg:(T0 element)"),
+                    new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<GlobalStruct> |sg:(GlobalStruct element)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<Samples.Computer01.MyStruct> |sg:(Samples.Computer01.MyStruct element)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<T0> |sg:(T0 element)"),
                     new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:MethodsSignature |cg: |fn:ThrowGenericMethod1 |fg:<System.Boolean> |sg:(System.Boolean element)"),