Skip to content

Commit

Permalink
Run System.Diagnostics.StackTrace tests with NAOT (#103151)
Browse files Browse the repository at this point in the history 
Also fixes some corner case issues, like whether an empty stacktrace should stringify into a newline.
  • Loading branch information
@MichalStrehovsky
MichalStrehovsky committed Jun 10, 2024
1 parent 7cd8459 commit 0686ce6
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 26 deletions.
15 changes: 8 additions & 7 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/StackTrace.NativeAot.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,19 @@ private void InitializeForIpAddressArray(IntPtr[] ipAddresses, int skipFrames, i
#if !TARGET_WASM
internal void ToString(TraceFormat traceFormat, StringBuilder builder)
{
if (_stackFrames == null)
if (_stackFrames != null)
{
return;
}

foreach (StackFrame frame in _stackFrames)
{
frame.AppendToStackTrace(builder);
foreach (StackFrame frame in _stackFrames)
{
frame?.AppendToStackTrace(builder);
}
}

if (traceFormat == TraceFormat.Normal && builder.Length >= Environment.NewLine.Length)
builder.Length -= Environment.NewLine.Length;

if (traceFormat == TraceFormat.TrailingNewLine && builder.Length == 0)
builder.AppendLine();
}
#endif
}
Expand Down
5 changes: 3 additions & 2 deletions .../System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,10 @@ private void EmitPointerTypeName(PointerSignatureHandle pointerSigHandle)
/// <summary>
/// Emit function pointer type.
/// </summary>
private void EmitFunctionPointerTypeName()
private static void EmitFunctionPointerTypeName()
{
_outputBuilder.Append("IntPtr");
// Function pointer types have no textual representation and we have tests making sure
// they show up as empty strings in stack traces, so deliberately do nothing.
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ private static bool GetLinqExpressionsBuiltWithIsInterpretingOnly()
public static bool IsAsyncFileIOSupported => !IsBrowser && !IsWasi;

public static bool IsLineNumbersSupported => !IsNativeAot;
public static bool IsILOffsetsSupported => !IsNativeAot;

public static bool IsInContainer => GetIsInContainer();
public static bool IsNotInContainer => !IsInContainer;
Expand Down
10 changes: 6 additions & 4 deletions src/libraries/System.Diagnostics.StackTrace/tests/StackFrameExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@ public static IEnumerable<object[]> StackFrame_TestData()
yield return new object[] { new StackFrame(int.MaxValue) };
}

[Theory]
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotNativeAot))]
[MemberData(nameof(StackFrame_TestData))]
public void HasNativeImage_StackFrame_ReturnsFalse(StackFrame stackFrame)
{
Assert.False(stackFrame.HasNativeImage());
}

[Theory]
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotNativeAot))]
[MemberData(nameof(StackFrame_TestData))]
public void GetNativeIP_StackFrame_ReturnsZero(StackFrame stackFrame)
{
Assert.Equal(IntPtr.Zero, stackFrame.GetNativeIP());
}

[Theory]
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotNativeAot))]
[MemberData(nameof(StackFrame_TestData))]
public void GetNativeImageBase_StackFrame_ReturnsZero(StackFrame stackFrame)
{
Expand All @@ -43,7 +43,7 @@ public static IEnumerable<object[]> HasMethod_TestData()
yield return new object[] { new StackFrame(int.MaxValue), false };
}

[Theory]
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsILOffsetsSupported))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
[MemberData(nameof(HasMethod_TestData))]
public void HasILOffset_Invoke_ReturnsExpected(StackFrame stackFrame, bool expected)
Expand All @@ -59,6 +59,7 @@ public void HasILOffset_NullStackFrame_ThrowsNullReferenceException()

[Theory]
[ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/103218", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
[MemberData(nameof(HasMethod_TestData))]
public void HasMethod_Invoke_ReturnsExpected(StackFrame stackFrame, bool expected)
{
Expand All @@ -79,6 +80,7 @@ public static IEnumerable<object[]> HasSource_TestData()
}

[Theory]
[ActiveIssue("https://github.com/dotnet/runtime/issues/103218", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
[MemberData(nameof(HasSource_TestData))]
public void HasSource_Invoke_ReturnsExpected(StackFrame stackFrame, bool expected)
{
Expand Down
7 changes: 6 additions & 1 deletion src/libraries/System.Diagnostics.StackTrace/tests/StackFrameTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public void SkipFrames_CallMethod_ReturnsExpected()
[Theory]
[InlineData(int.MinValue)]
[InlineData(int.MaxValue)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/103218", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
public void SkipFrames_ManyFrames_HasNoMethod(int skipFrames)
{
var stackFrame = new StackFrame(skipFrames);
Expand Down Expand Up @@ -123,6 +124,7 @@ public static IEnumerable<object[]> ToString_TestData()

[Theory]
[ActiveIssue("https://github.com/mono/mono/issues/15186", TestRuntimes.Mono)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/103156", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
[MemberData(nameof(ToString_TestData))]
public void ToString_Invoke_ReturnsExpected(StackFrame stackFrame, string expectedToString)
{
Expand Down Expand Up @@ -168,7 +170,10 @@ private static void VerifyStackFrameSkipFrames(StackFrame stackFrame, bool isFil
}
else
{
Assert.True(stackFrame.GetILOffset() >= 0, $"Expected GetILOffset() {stackFrame.GetILOffset()} for {stackFrame} to be greater or equal to zero.");
if (PlatformDetection.IsILOffsetsSupported)
{
Assert.True(stackFrame.GetILOffset() >= 0, $"Expected GetILOffset() {stackFrame.GetILOffset()} for {stackFrame} to be greater or equal to zero.");
}
}

// GetMethod returns null for unknown frames.
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Diagnostics.StackTrace/tests/StackTraceSymbolsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace System.Diagnostics.SymbolStore.Tests
{
public class StackTraceSymbolsTests
{
[Fact]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.HasAssemblyFiles))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/51399", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
public void StackTraceSymbolsDoNotLockFile()
{
Expand Down
24 changes: 14 additions & 10 deletions src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void MethodsToSkip_Get_ReturnsZero()
public void Ctor_Default()
{
var stackTrace = new StackTrace();
VerifyFrames(stackTrace, false);
VerifyFrames(stackTrace, false, 0);
}

[Theory]
Expand All @@ -57,7 +57,7 @@ public void Ctor_Default()
public void Ctor_FNeedFileInfo(bool fNeedFileInfo)
{
var stackTrace = new StackTrace(fNeedFileInfo);
VerifyFrames(stackTrace, fNeedFileInfo);
VerifyFrames(stackTrace, fNeedFileInfo, 0);
}

[Theory]
Expand All @@ -73,7 +73,7 @@ public void Ctor_SkipFrames(int skipFrames)
Assert.Equal(emptyStackTrace.FrameCount - skipFrames, stackTrace.FrameCount);
Assert.Equal(expectedMethods, stackTrace.GetFrames().Select(f => f.GetMethod()));

VerifyFrames(stackTrace, false);
VerifyFrames(stackTrace, false, skipFrames);
}

[Fact]
Expand All @@ -99,7 +99,7 @@ public void Ctor_SkipFrames_FNeedFileInfo(int skipFrames, bool fNeedFileInfo)
Assert.Equal(emptyStackTrace.FrameCount - skipFrames, stackTrace.FrameCount);
Assert.Equal(expectedMethods, stackTrace.GetFrames().Select(f => f.GetMethod()));

VerifyFrames(stackTrace, fNeedFileInfo);
VerifyFrames(stackTrace, fNeedFileInfo, skipFrames);
}

[Theory]
Expand All @@ -117,7 +117,7 @@ public void Ctor_LargeSkipFramesFNeedFileInfo_GetFramesReturnsEmpty(bool fNeedFi
public void Ctor_ThrownException_GetFramesReturnsExpected()
{
var stackTrace = new StackTrace(InvokeException());
VerifyFrames(stackTrace, false);
VerifyFrames(stackTrace, false, 0);
}

[Fact]
Expand All @@ -137,7 +137,7 @@ public void Ctor_EmptyException_GetFramesReturnsEmpty()
public void Ctor_Bool_ThrownException_GetFramesReturnsExpected(bool fNeedFileInfo)
{
var stackTrace = new StackTrace(InvokeException(), fNeedFileInfo);
VerifyFrames(stackTrace, fNeedFileInfo);
VerifyFrames(stackTrace, fNeedFileInfo, 0);
}

[Theory]
Expand Down Expand Up @@ -171,7 +171,7 @@ public void Ctor_Exception_SkipFrames(int skipFrames)
Assert.Equal(expectedMethods, frames.Select(f => f.GetMethod()));
if (frames != null)
{
VerifyFrames(stackTrace, false);
VerifyFrames(stackTrace, false, skipFrames);
}
}

Expand Down Expand Up @@ -211,7 +211,7 @@ public void Ctor_Exception_SkipFrames_FNeedFileInfo(int skipFrames, bool fNeedFi
Assert.Equal(expectedMethods, frames.Select(f => f.GetMethod()));
if (frames != null)
{
VerifyFrames(stackTrace, fNeedFileInfo);
VerifyFrames(stackTrace, fNeedFileInfo, skipFrames);
}
}

Expand Down Expand Up @@ -450,7 +450,7 @@ private class ClassWithConstructor
public ClassWithConstructor() => StackTrace = new StackTrace();
}

private static void VerifyFrames(StackTrace stackTrace, bool hasFileInfo)
private static void VerifyFrames(StackTrace stackTrace, bool hasFileInfo, int skippedFrames)
{
Assert.True(stackTrace.FrameCount > 0);

Expand All @@ -467,7 +467,11 @@ private static void VerifyFrames(StackTrace stackTrace, bool hasFileInfo)
Assert.Equal(0, stackFrame.GetFileLineNumber());
Assert.Equal(0, stackFrame.GetFileColumnNumber());
}
Assert.NotNull(stackFrame.GetMethod());

// On native AOT, the reflection invoke infrastructure uses compiler-generated code
// that doesn't have a reflection method associated. Limit the checks.
if (!PlatformDetection.IsNativeAot || (i + skippedFrames) == 0)
Assert.NotNull(stackFrame.GetMethod());
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/libraries/tests.proj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Data.Common\tests\System.Data.Common.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Diagnostics.DiagnosticSource\tests\TestWithConfigSwitches\System.Diagnostics.DiagnosticSource.Switches.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Diagnostics.PerformanceCounter\tests\System.Diagnostics.PerformanceCounter.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Diagnostics.StackTrace\tests\System.Diagnostics.StackTrace.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Diagnostics.TraceSource\tests\System.Diagnostics.TraceSource.Config.Tests\System.Diagnostics.TraceSource.Config.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Diagnostics.Tracing\tests\System.Diagnostics.Tracing.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Formats.Cbor\tests\System.Formats.Cbor.Tests.csproj" />
Expand Down

0 comments on commit 0686ce6

Please sign in to comment.