[Profiler] Cleanup generic method frame encoding and namespaceless types

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)"),