From c77d3be8afedfa0d01afb97c35cf1550af28da53 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 29 Oct 2020 15:31:55 +0900 Subject: [PATCH 01/14] Add IL Offset and Method Token to stacktrace For easier debugging, if the PDB is not deployed, add the Method Token and IL Offset to the stacktrace. This functionality can be turned on and off by environment setting "COMPlus_ILOffsetToStackTrace" --- .../src/Resources/Strings.resx | 3 +++ .../src/System/Diagnostics/StackTrace.cs | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 807cca91a37ae..90635361dee4d 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -3253,6 +3253,9 @@ The timeout must represent a value between -1 and Int32.MaxValue, inclusive. + + in {0}:token 0x{1}+0x{2} + in {0}:line {1} diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs index 3c6ba961c79d3..f6c20d52e9752 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs @@ -19,6 +19,7 @@ public partial class StackTrace { public const int METHODS_TO_SKIP = 0; + private static readonly bool s_ilOffsetToStackTrace = Environment.GetEnvironmentVariable("COMPlus_ILOffsetToStackTrace") == "1"; private int _numOfFrames; private int _methodsToSkip; @@ -206,6 +207,7 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) string word_At = SR.GetResourceString(nameof(SR.Word_At), defaultString: "at"); // We also want to pass in a default for inFileLineNumber. string inFileLineNum = SR.GetResourceString(nameof(SR.StackTrace_InFileLineNumber), defaultString: "in {0}:line {1}"); + string inFileILOffset = SR.GetResourceString(nameof(SR.StackTrace_InFileILOffset), defaultString: "in {0}:token 0x{1}+0x{2}"); bool fFirstFrame = true; for (int iFrameIndex = 0; iFrameIndex < _numOfFrames; iFrameIndex++) { @@ -323,6 +325,21 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) sb.Append(' '); sb.AppendFormat(CultureInfo.InvariantCulture, inFileLineNum, fileName, sf.GetFileLineNumber()); } + else if (s_ilOffsetToStackTrace) + { + try + { + string methodToken = Convert.ToString(mb.MetadataToken, 16); + string ilOffset = Convert.ToString(sf.GetILOffset(), 16); + string? assemblyName = System.IO.Path.GetFileName(mb.ReflectedType?.Assembly.Location); + if (assemblyName != null) + { + sb.Append(' '); + sb.AppendFormat(CultureInfo.InvariantCulture, inFileILOffset, assemblyName, methodToken, ilOffset); + } + } + catch (Exception) {} + } } // Skip EDI boundary for async From a7f6060aa2e3d4e17f53f03b3be4848d7079e0dc Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Tue, 16 Mar 2021 09:00:31 +0900 Subject: [PATCH 02/14] Use LocalAppContextSwitch and HexConverter Use LocalAppContextSwitch to get environment variable without using static constructor in StackTrace. Also, use HexConverter to get metadata token and il offset without unnecessary complex. --- .../src/System/LocalAppContextSwitches.Common.cs | 5 +++++ .../src/System/Diagnostics/StackTrace.cs | 11 +++++++---- .../src/System/LocalAppContextSwitches.cs | 8 ++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs index bcbc8d56093ac..765ff993d2fe1 100644 --- a/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs +++ b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs @@ -56,6 +56,11 @@ private static bool GetSwitchDefaultValue(string switchName) return true; } + if (switchName == "Switch.System.Diagnostics.StackTrace.ILOffsetToStackTrace") + { + return Environment.GetEnvironmentVariable("COMPlus_ILOffsetToStackTrace") == "1" ? true : false; + } + return false; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs index f6c20d52e9752..5b37dc81b2c48 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs @@ -8,6 +8,7 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Text; +using System.Buffers.Binary; namespace System.Diagnostics { @@ -19,7 +20,6 @@ public partial class StackTrace { public const int METHODS_TO_SKIP = 0; - private static readonly bool s_ilOffsetToStackTrace = Environment.GetEnvironmentVariable("COMPlus_ILOffsetToStackTrace") == "1"; private int _numOfFrames; private int _methodsToSkip; @@ -208,6 +208,7 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) // We also want to pass in a default for inFileLineNumber. string inFileLineNum = SR.GetResourceString(nameof(SR.StackTrace_InFileLineNumber), defaultString: "in {0}:line {1}"); string inFileILOffset = SR.GetResourceString(nameof(SR.StackTrace_InFileILOffset), defaultString: "in {0}:token 0x{1}+0x{2}"); + Span bytes = stackalloc byte[4]; bool fFirstFrame = true; for (int iFrameIndex = 0; iFrameIndex < _numOfFrames; iFrameIndex++) { @@ -325,12 +326,14 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) sb.Append(' '); sb.AppendFormat(CultureInfo.InvariantCulture, inFileLineNum, fileName, sf.GetFileLineNumber()); } - else if (s_ilOffsetToStackTrace) + else if (LocalAppContextSwitches.ILOffsetToStackTrace) { try { - string methodToken = Convert.ToString(mb.MetadataToken, 16); - string ilOffset = Convert.ToString(sf.GetILOffset(), 16); + BinaryPrimitives.WriteInt32BigEndian(bytes, mb.MetadataToken); + string methodToken = HexConverter.ToString(bytes.Slice(0, 4), HexConverter.Casing.Lower).TrimStart('0'); + BinaryPrimitives.WriteInt32BigEndian(bytes, sf.GetILOffset()); + string ilOffset = HexConverter.ToString(bytes.Slice(0, 4), HexConverter.Casing.Lower).TrimStart('0'); string? assemblyName = System.IO.Path.GetFileName(mb.ReflectedType?.Assembly.Location); if (assemblyName != null) { diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs index d669bd4cccc4e..9a50ae5e852fe 100644 --- a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs +++ b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs @@ -47,5 +47,13 @@ public static bool SerializationGuard [MethodImpl(MethodImplOptions.AggressiveInlining)] get => GetCachedSwitchValue("Switch.System.Runtime.Serialization.SerializationGuard", ref s_serializationGuard); } + + private static int s_ilOffsetToStackTrace; + public static bool ILOffsetToStackTrace + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => GetCachedSwitchValue("Switch.System.Diagnostics.StackTrace.ILOffsetToStackTrace", ref s_ilOffsetToStackTrace); + } + } } From 99f96fa9d3ac2cf6624e0980e23529da2e86c87e Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Wed, 24 Mar 2021 09:10:40 +0900 Subject: [PATCH 03/14] Use AppendFormat instead of HexConverter Use AppendFormat() for formatting of the hex number. --- .../src/Resources/Strings.resx | 2 +- .../src/System/Diagnostics/StackTrace.cs | 20 +++++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 90635361dee4d..2b214c82eaf72 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -3254,7 +3254,7 @@ The timeout must represent a value between -1 and Int32.MaxValue, inclusive. - in {0}:token 0x{1}+0x{2} + in {0}:token 0x{1:x}+0x{2:x} in {0}:line {1} diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs index 5b37dc81b2c48..18dee01eb7f09 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs @@ -207,7 +207,7 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) string word_At = SR.GetResourceString(nameof(SR.Word_At), defaultString: "at"); // We also want to pass in a default for inFileLineNumber. string inFileLineNum = SR.GetResourceString(nameof(SR.StackTrace_InFileLineNumber), defaultString: "in {0}:line {1}"); - string inFileILOffset = SR.GetResourceString(nameof(SR.StackTrace_InFileILOffset), defaultString: "in {0}:token 0x{1}+0x{2}"); + string inFileILOffset = SR.GetResourceString(nameof(SR.StackTrace_InFileILOffset), defaultString: "in {0}:token 0x{1:x}+0x{2:x}"); Span bytes = stackalloc byte[4]; bool fFirstFrame = true; for (int iFrameIndex = 0; iFrameIndex < _numOfFrames; iFrameIndex++) @@ -326,22 +326,16 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) sb.Append(' '); sb.AppendFormat(CultureInfo.InvariantCulture, inFileLineNum, fileName, sf.GetFileLineNumber()); } - else if (LocalAppContextSwitches.ILOffsetToStackTrace) + else if (LocalAppContextSwitches.ILOffsetToStackTrace && mb.ReflectedType != null) { + string assemblyName = System.IO.Path.GetFileName(mb.ReflectedType.Assembly.Location); try { - BinaryPrimitives.WriteInt32BigEndian(bytes, mb.MetadataToken); - string methodToken = HexConverter.ToString(bytes.Slice(0, 4), HexConverter.Casing.Lower).TrimStart('0'); - BinaryPrimitives.WriteInt32BigEndian(bytes, sf.GetILOffset()); - string ilOffset = HexConverter.ToString(bytes.Slice(0, 4), HexConverter.Casing.Lower).TrimStart('0'); - string? assemblyName = System.IO.Path.GetFileName(mb.ReflectedType?.Assembly.Location); - if (assemblyName != null) - { - sb.Append(' '); - sb.AppendFormat(CultureInfo.InvariantCulture, inFileILOffset, assemblyName, methodToken, ilOffset); - } + int token = mb.MetadataToken; + sb.Append(' '); + sb.AppendFormat(CultureInfo.InvariantCulture, inFileILOffset, assemblyName, token, sf.GetILOffset()); } - catch (Exception) {} + catch (System.InvalidOperationException) {} } } From 90479a11ea6f2084565ae81a650cc30ff1a1ff56 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Wed, 31 Mar 2021 08:54:16 +0900 Subject: [PATCH 04/14] Use switch to activate ILOffset in stacktrace - Change switch name from ILOffsetsToStackTrace to ShowILOffset - Use "Switch.System.Diagnostics.StackTrace.ShowILOffsets" to activate ILOffset in stacktrace - Use "DOTNET_" prefix for environment variable name (DOTNET_ILOffsetToStackTrace) --- .../Common/src/System/LocalAppContextSwitches.Common.cs | 9 +++++++-- .../src/System/Diagnostics/StackTrace.cs | 4 ++-- .../src/System/LocalAppContextSwitches.cs | 6 +++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs index 765ff993d2fe1..5e76241bd6d36 100644 --- a/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs +++ b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs @@ -56,9 +56,14 @@ private static bool GetSwitchDefaultValue(string switchName) return true; } - if (switchName == "Switch.System.Diagnostics.StackTrace.ILOffsetToStackTrace") + if (switchName == "Switch.System.Diagnostics.StackTrace.ShowILOffsets") { - return Environment.GetEnvironmentVariable("COMPlus_ILOffsetToStackTrace") == "1" ? true : false; + if (!AppContext.TryGetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", out bool ret)) + { + ret = Environment.GetEnvironmentVariable("DOTNET_ILOffsetToStackTrace") == "1" ? true : false; + } + + return ret; } return false; diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs index 18dee01eb7f09..796966748700d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs @@ -326,9 +326,9 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) sb.Append(' '); sb.AppendFormat(CultureInfo.InvariantCulture, inFileLineNum, fileName, sf.GetFileLineNumber()); } - else if (LocalAppContextSwitches.ILOffsetToStackTrace && mb.ReflectedType != null) + else if (LocalAppContextSwitches.ShowILOffsets && mb.ReflectedType != null) { - string assemblyName = System.IO.Path.GetFileName(mb.ReflectedType.Assembly.Location); + string assemblyName = mb.ReflectedType.Module.ScopeName; try { int token = mb.MetadataToken; diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs index 9a50ae5e852fe..a5dcec5c5409a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs +++ b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs @@ -48,11 +48,11 @@ public static bool SerializationGuard get => GetCachedSwitchValue("Switch.System.Runtime.Serialization.SerializationGuard", ref s_serializationGuard); } - private static int s_ilOffsetToStackTrace; - public static bool ILOffsetToStackTrace + private static int s_showILOffset; + public static bool ShowILOffsets { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => GetCachedSwitchValue("Switch.System.Diagnostics.StackTrace.ILOffsetToStackTrace", ref s_ilOffsetToStackTrace); + get => GetCachedSwitchValue("Switch.System.Diagnostics.StackTrace.ShowILOffsets", ref s_showILOffset); } } From 7b3f838d8d8974e15c14690a793fc6d2d58ae958 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 1 Apr 2021 15:04:11 +0900 Subject: [PATCH 05/14] Use AppContextConfigHelper for get switch value --- .../src/System/LocalAppContextSwitches.Common.cs | 10 ---------- .../src/System/LocalAppContextSwitches.cs | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs index 5e76241bd6d36..bcbc8d56093ac 100644 --- a/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs +++ b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs @@ -56,16 +56,6 @@ private static bool GetSwitchDefaultValue(string switchName) return true; } - if (switchName == "Switch.System.Diagnostics.StackTrace.ShowILOffsets") - { - if (!AppContext.TryGetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", out bool ret)) - { - ret = Environment.GetEnvironmentVariable("DOTNET_ILOffsetToStackTrace") == "1" ? true : false; - } - - return ret; - } - return false; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs index a5dcec5c5409a..7a5dffe554fd1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs +++ b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs @@ -49,11 +49,21 @@ public static bool SerializationGuard } private static int s_showILOffset; + private static bool GetDefaultShowILOffsetSetting() + { + if (s_showILOffset < 0) return false; + if (s_showILOffset > 0) return true; + + bool isSwitchEnabled = AppContextConfigHelper.GetBooleanConfig("Switch.System.Diagnostics.StackTrace.ShowILOffsets", "DOTNET_ILOffsetToStackTrace"); + s_showILOffset = isSwitchEnabled ? 1 : -1; + + return isSwitchEnabled; + } + public static bool ShowILOffsets { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => GetCachedSwitchValue("Switch.System.Diagnostics.StackTrace.ShowILOffsets", ref s_showILOffset); + get => GetDefaultShowILOffsetSetting(); } - } } From af7c84b29d720381296ba38f9352ba4946770399 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 1 Apr 2021 15:07:59 +0900 Subject: [PATCH 06/14] Add test case for ILOffsetToStackTrace --- .../ExceptionTestAssembly.cs | 12 ++++ .../ExceptionTestAssembly.csproj | 10 ++++ .../ExceptionTestAssembly2.cs | 12 ++++ .../ExceptionTestAssembly2.csproj | 10 ++++ .../tests/StackTraceTests.cs | 59 +++++++++++++++++++ ...System.Diagnostics.StackTrace.Tests.csproj | 6 +- 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.cs create mode 100644 src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.csproj create mode 100644 src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs create mode 100644 src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.cs b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.cs new file mode 100644 index 0000000000000..49902b1000c02 --- /dev/null +++ b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +class Program +{ + public static void Foo() + { + throw new NullReferenceException(); + } +} diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.csproj b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.csproj new file mode 100644 index 0000000000000..d7b60a977d7ce --- /dev/null +++ b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly/ExceptionTestAssembly.csproj @@ -0,0 +1,10 @@ + + + true + netstandard2.0 + None + + + + + diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs new file mode 100644 index 0000000000000..052c260337a99 --- /dev/null +++ b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +class Program2 +{ + public static void Foo() + { + throw new NullReferenceException(); + } +} diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj new file mode 100644 index 0000000000000..41803acd00c37 --- /dev/null +++ b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj @@ -0,0 +1,10 @@ + + + true + netstandard2.0 + None + + + + + diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs index 894374ba29b4d..1a5893aa50abd 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs @@ -5,7 +5,9 @@ using System.Globalization; using System.Linq; using System.Reflection; +using System.Reflection.Emit; using System.Runtime.CompilerServices; +using System.IO; using Xunit; namespace System.Diagnostics @@ -31,6 +33,17 @@ namespace System.Diagnostics.Tests { public class StackTraceTests { + private const string s_sourceTestAssemblyName = "ExceptionTestAssembly.dll"; + private const string s_sourceTestAssembly2Name = "ExceptionTestAssembly2.dll"; + private string SourceTestAssemblyPath { get; } = Path.Combine(Environment.CurrentDirectory, s_sourceTestAssemblyName); + private string SourceTestAssembly2Path { get; } = Path.Combine(Environment.CurrentDirectory, s_sourceTestAssembly2Name); + + static StackTraceTests() + { + // enable Switch.System.Diagnostics.StackTrace.ShowILOffsets for ToString_ShowILOffset TC + AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); + } + [Fact] public void MethodsToSkip_Get_ReturnsZero() { @@ -300,6 +313,52 @@ public void ToString_NullFrame_ThrowsNullReferenceException() Assert.Equal(Environment.NewLine, stackTrace.ToString()); } + [Fact] + public void ToString_ShowILOffset() + { + // Normal loading case + var asm = Assembly.LoadFrom(SourceTestAssemblyPath); + try + { + asm.GetType("Program").GetMethod("Foo").Invoke(null, null); + } + catch (Exception e) + { + Assert.Contains(":token ", e.InnerException.StackTrace); + } + + // Assembly.Load(Byte[]) case + var inMemBlob = File.ReadAllBytes(SourceTestAssembly2Path); + var asm2 = Assembly.Load(inMemBlob); + try + { + asm2.GetType("Program2").GetMethod("Foo").Invoke(null, null); + } + catch (Exception e) + { + Assert.Contains(":token ", e.InnerException.StackTrace); + } + + // AssmblyBuilder.DefineDynamicAssembly() case + AssemblyName asmName = new AssemblyName("ExceptionTestAssembly3"); + AssemblyBuilder asmBldr = AssemblyBuilder.DefineDynamicAssembly(asmName, AssemblyBuilderAccess.Run); + ModuleBuilder modBldr = asmBldr.DefineDynamicModule(asmName.Name); + TypeBuilder tBldr = modBldr.DefineType("Program3"); + MethodBuilder mBldr = tBldr.DefineMethod("Foo", MethodAttributes.Public | MethodAttributes.Static, null, null); + ILGenerator ilGen = mBldr.GetILGenerator(); + ilGen.ThrowException(typeof(NullReferenceException)); + ilGen.Emit(OpCodes.Ret); + Type t = tBldr.CreateType(); + try + { + t.InvokeMember("Foo", BindingFlags.InvokeMethod, null, null, null); + } + catch (Exception e) + { + Assert.Contains(":token ", e.InnerException.StackTrace); + } + } + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static StackTrace NoParameters() => new StackTrace(); [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj b/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj index b112648c18363..c06c4811cbeae 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj +++ b/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj @@ -17,4 +17,8 @@ - \ No newline at end of file + + + + + From 62350d8ba9ae5a94e4026836bf013dace05165b8 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Fri, 2 Apr 2021 06:18:59 +0900 Subject: [PATCH 07/14] Remove unused variable --- .../System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs index 796966748700d..c8dcf85ec7720 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs @@ -208,7 +208,6 @@ internal void ToString(TraceFormat traceFormat, StringBuilder sb) // We also want to pass in a default for inFileLineNumber. string inFileLineNum = SR.GetResourceString(nameof(SR.StackTrace_InFileLineNumber), defaultString: "in {0}:line {1}"); string inFileILOffset = SR.GetResourceString(nameof(SR.StackTrace_InFileILOffset), defaultString: "in {0}:token 0x{1:x}+0x{2:x}"); - Span bytes = stackalloc byte[4]; bool fFirstFrame = true; for (int iFrameIndex = 0; iFrameIndex < _numOfFrames; iFrameIndex++) { From c54d679e51abf784aa68e233fead98dac1271af8 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Fri, 2 Apr 2021 10:53:09 +0900 Subject: [PATCH 08/14] Use RemoteExecutor.Invoke for testing --- .../ExceptionTestAssembly2.cs | 12 --- .../ExceptionTestAssembly2.csproj | 10 -- .../tests/StackTraceTests.cs | 94 ++++++++++--------- ...System.Diagnostics.StackTrace.Tests.csproj | 2 +- 4 files changed, 50 insertions(+), 68 deletions(-) delete mode 100644 src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs delete mode 100644 src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs deleted file mode 100644 index 052c260337a99..0000000000000 --- a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; - -class Program2 -{ - public static void Foo() - { - throw new NullReferenceException(); - } -} diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj b/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj deleted file mode 100644 index 41803acd00c37..0000000000000 --- a/src/libraries/System.Diagnostics.StackTrace/tests/ExceptionTestAssembly2/ExceptionTestAssembly2.csproj +++ /dev/null @@ -1,10 +0,0 @@ - - - true - netstandard2.0 - None - - - - - diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs index 1a5893aa50abd..e9de9c45e34c5 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs @@ -8,6 +8,7 @@ using System.Reflection.Emit; using System.Runtime.CompilerServices; using System.IO; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Diagnostics @@ -33,17 +34,6 @@ namespace System.Diagnostics.Tests { public class StackTraceTests { - private const string s_sourceTestAssemblyName = "ExceptionTestAssembly.dll"; - private const string s_sourceTestAssembly2Name = "ExceptionTestAssembly2.dll"; - private string SourceTestAssemblyPath { get; } = Path.Combine(Environment.CurrentDirectory, s_sourceTestAssemblyName); - private string SourceTestAssembly2Path { get; } = Path.Combine(Environment.CurrentDirectory, s_sourceTestAssembly2Name); - - static StackTraceTests() - { - // enable Switch.System.Diagnostics.StackTrace.ShowILOffsets for ToString_ShowILOffset TC - AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); - } - [Fact] public void MethodsToSkip_Get_ReturnsZero() { @@ -313,50 +303,64 @@ public void ToString_NullFrame_ThrowsNullReferenceException() Assert.Equal(Environment.NewLine, stackTrace.ToString()); } - [Fact] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void ToString_ShowILOffset() { + string SourceTestAssemblyPath = Path.Combine(Environment.CurrentDirectory, "ExceptionTestAssembly.dll"); + // Normal loading case - var asm = Assembly.LoadFrom(SourceTestAssemblyPath); - try + RemoteExecutor.Invoke((asmPath) => { - asm.GetType("Program").GetMethod("Foo").Invoke(null, null); - } - catch (Exception e) - { - Assert.Contains(":token ", e.InnerException.StackTrace); - } + AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); + var asm = Assembly.LoadFrom(asmPath); + try + { + asm.GetType("Program").GetMethod("Foo").Invoke(null, null); + } + catch (Exception e) + { + Assert.Contains(":token ", e.InnerException.StackTrace); + } + }, SourceTestAssemblyPath).Dispose(); // Assembly.Load(Byte[]) case - var inMemBlob = File.ReadAllBytes(SourceTestAssembly2Path); - var asm2 = Assembly.Load(inMemBlob); - try + RemoteExecutor.Invoke((asmPath) => { - asm2.GetType("Program2").GetMethod("Foo").Invoke(null, null); - } - catch (Exception e) - { - Assert.Contains(":token ", e.InnerException.StackTrace); - } + AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); + var inMemBlob = File.ReadAllBytes(asmPath); + var asm2 = Assembly.Load(inMemBlob); + try + { + asm2.GetType("Program").GetMethod("Foo").Invoke(null, null); + } + catch (Exception e) + { + Assert.Contains(":token ", e.InnerException.StackTrace); + } + }, SourceTestAssemblyPath).Dispose(); // AssmblyBuilder.DefineDynamicAssembly() case - AssemblyName asmName = new AssemblyName("ExceptionTestAssembly3"); - AssemblyBuilder asmBldr = AssemblyBuilder.DefineDynamicAssembly(asmName, AssemblyBuilderAccess.Run); - ModuleBuilder modBldr = asmBldr.DefineDynamicModule(asmName.Name); - TypeBuilder tBldr = modBldr.DefineType("Program3"); - MethodBuilder mBldr = tBldr.DefineMethod("Foo", MethodAttributes.Public | MethodAttributes.Static, null, null); - ILGenerator ilGen = mBldr.GetILGenerator(); - ilGen.ThrowException(typeof(NullReferenceException)); - ilGen.Emit(OpCodes.Ret); - Type t = tBldr.CreateType(); - try + RemoteExecutor.Invoke(() => { - t.InvokeMember("Foo", BindingFlags.InvokeMethod, null, null, null); - } - catch (Exception e) - { - Assert.Contains(":token ", e.InnerException.StackTrace); - } + AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); + AssemblyName asmName = new AssemblyName("ExceptionTestAssembly"); + AssemblyBuilder asmBldr = AssemblyBuilder.DefineDynamicAssembly(asmName, AssemblyBuilderAccess.Run); + ModuleBuilder modBldr = asmBldr.DefineDynamicModule(asmName.Name); + TypeBuilder tBldr = modBldr.DefineType("Program"); + MethodBuilder mBldr = tBldr.DefineMethod("Foo", MethodAttributes.Public | MethodAttributes.Static, null, null); + ILGenerator ilGen = mBldr.GetILGenerator(); + ilGen.ThrowException(typeof(NullReferenceException)); + ilGen.Emit(OpCodes.Ret); + Type t = tBldr.CreateType(); + try + { + t.InvokeMember("Foo", BindingFlags.InvokeMethod, null, null, null); + } + catch (Exception e) + { + Assert.Contains(":token ", e.InnerException.StackTrace); + } + }).Dispose(); } [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj b/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj index c06c4811cbeae..93673781e705f 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj +++ b/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj @@ -2,6 +2,7 @@ $(NetCoreAppCurrent) true + true @@ -19,6 +20,5 @@ - From 870543aeba856ffd3f3b0cc2aefc06c668af7286 Mon Sep 17 00:00:00 2001 From: "ws77.cho" Date: Thu, 8 Apr 2021 06:29:11 +0900 Subject: [PATCH 09/14] Update src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs Co-authored-by: Noah Falk --- .../src/System/LocalAppContextSwitches.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs index 7a5dffe554fd1..86ef0387e1b69 100644 --- a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs +++ b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs @@ -54,7 +54,7 @@ private static bool GetDefaultShowILOffsetSetting() if (s_showILOffset < 0) return false; if (s_showILOffset > 0) return true; - bool isSwitchEnabled = AppContextConfigHelper.GetBooleanConfig("Switch.System.Diagnostics.StackTrace.ShowILOffsets", "DOTNET_ILOffsetToStackTrace"); + bool isSwitchEnabled = AppContextConfigHelper.GetBooleanConfig("Switch.System.Diagnostics.StackTrace.ShowILOffsets", "DOTNET_SYSTEM_DIAGNOSTICS_STACKTRACE_SHOWILOFFSETS"); s_showILOffset = isSwitchEnabled ? 1 : -1; return isSwitchEnabled; From 8ee5242b4b1f5e9ffa1968e7697bd7213d2033ae Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 8 Apr 2021 09:29:16 +0900 Subject: [PATCH 10/14] TC contents are added --- .../tests/StackTraceTests.cs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs index e9de9c45e34c5..acc3cd76293f5 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs @@ -9,6 +9,7 @@ using System.Runtime.CompilerServices; using System.IO; using Microsoft.DotNet.RemoteExecutor; +using System.Text.RegularExpressions; using Xunit; namespace System.Diagnostics @@ -306,10 +307,12 @@ public void ToString_NullFrame_ThrowsNullReferenceException() [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void ToString_ShowILOffset() { - string SourceTestAssemblyPath = Path.Combine(Environment.CurrentDirectory, "ExceptionTestAssembly.dll"); + string AssemblyName = "ExceptionTestAssembly.dll"; + string SourceTestAssemblyPath = Path.Combine(Environment.CurrentDirectory, AssemblyName); + string regPattern = @":token 0x([a-f0-9]*)\+0x([a-f0-9]*)"; // Normal loading case - RemoteExecutor.Invoke((asmPath) => + RemoteExecutor.Invoke((asmPath, asmName, p) => { AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); var asm = Assembly.LoadFrom(asmPath); @@ -319,12 +322,13 @@ public void ToString_ShowILOffset() } catch (Exception e) { - Assert.Contains(":token ", e.InnerException.StackTrace); + Assert.Contains(asmName, e.InnerException.StackTrace); + Assert.True(Regex.Match(e.InnerException.StackTrace, p).Success); } - }, SourceTestAssemblyPath).Dispose(); + }, SourceTestAssemblyPath, AssemblyName, regPattern).Dispose(); // Assembly.Load(Byte[]) case - RemoteExecutor.Invoke((asmPath) => + RemoteExecutor.Invoke((asmPath, asmName, p) => { AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); var inMemBlob = File.ReadAllBytes(asmPath); @@ -335,12 +339,13 @@ public void ToString_ShowILOffset() } catch (Exception e) { - Assert.Contains(":token ", e.InnerException.StackTrace); + Assert.Contains(asmName, e.InnerException.StackTrace); + Assert.True(Regex.Match(e.InnerException.StackTrace, p).Success); } - }, SourceTestAssemblyPath).Dispose(); + }, SourceTestAssemblyPath, AssemblyName, regPattern).Dispose(); // AssmblyBuilder.DefineDynamicAssembly() case - RemoteExecutor.Invoke(() => + RemoteExecutor.Invoke((p) => { AppContext.SetSwitch("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); AssemblyName asmName = new AssemblyName("ExceptionTestAssembly"); @@ -358,9 +363,10 @@ public void ToString_ShowILOffset() } catch (Exception e) { - Assert.Contains(":token ", e.InnerException.StackTrace); + Assert.Contains("RefEmit_InMemoryManifestModule", e.InnerException.StackTrace); + Assert.True(Regex.Match(e.InnerException.StackTrace, p).Success); } - }).Dispose(); + }, regPattern).Dispose(); } [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] From a3e2476e157a665cc22c83c2504676d9431d9121 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 8 Apr 2021 10:14:05 +0900 Subject: [PATCH 11/14] Remove setting code using enviornment variable remove setting code using environment variable and change switch default value to true. --- .../src/System/LocalAppContextSwitches.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs index 86ef0387e1b69..ed450d7494369 100644 --- a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs +++ b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs @@ -54,7 +54,7 @@ private static bool GetDefaultShowILOffsetSetting() if (s_showILOffset < 0) return false; if (s_showILOffset > 0) return true; - bool isSwitchEnabled = AppContextConfigHelper.GetBooleanConfig("Switch.System.Diagnostics.StackTrace.ShowILOffsets", "DOTNET_SYSTEM_DIAGNOSTICS_STACKTRACE_SHOWILOFFSETS"); + bool isSwitchEnabled = AppContextConfigHelper.GetBooleanConfig("Switch.System.Diagnostics.StackTrace.ShowILOffsets", true); s_showILOffset = isSwitchEnabled ? 1 : -1; return isSwitchEnabled; From a10ebfcced44c5bcd956930261e3d179b7630043 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 8 Apr 2021 10:29:50 +0900 Subject: [PATCH 12/14] code clean-up. remove unused code --- .../System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs index c8dcf85ec7720..10865d4432158 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs @@ -8,7 +8,6 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Text; -using System.Buffers.Binary; namespace System.Diagnostics { From c63a0c8f0a241c75e99327bf199608925266ae0b Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Thu, 8 Apr 2021 10:48:08 +0900 Subject: [PATCH 13/14] Sort using statements --- .../System.Diagnostics.StackTrace/tests/StackTraceTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs index acc3cd76293f5..a2cdb7a912eea 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs @@ -3,13 +3,13 @@ using System.Collections.Generic; using System.Globalization; +using System.IO; using System.Linq; using System.Reflection; using System.Reflection.Emit; using System.Runtime.CompilerServices; -using System.IO; -using Microsoft.DotNet.RemoteExecutor; using System.Text.RegularExpressions; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Diagnostics From 8c8532686238292637ccc0ddefce79cecb91a748 Mon Sep 17 00:00:00 2001 From: Woongsuk Cho Date: Mon, 12 Apr 2021 14:29:48 +0900 Subject: [PATCH 14/14] Skipt test in mono interpreter mode. Related issue : https://github.com/dotnet/runtime/issues/51096 --- .../System.Diagnostics.StackTrace/tests/StackTraceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs index a2cdb7a912eea..0464a22a44014 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs @@ -305,6 +305,7 @@ public void ToString_NullFrame_ThrowsNullReferenceException() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/51096", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoInterpreter))] public void ToString_ShowILOffset() { string AssemblyName = "ExceptionTestAssembly.dll";