Runtime: Implement System.Diagnostics.StackTrace/StackFrame

[nexussays]

Per discussion in dotnet/runtime#14447, this issue is to design & implement equivalents for StackFrame and StackTrace. taking into account any possible API changes that might be necessary to better fit the goals of .NET Core.

Implementation discussion may want to reference dotnet/runtime#14609

[nexussays]

Potentially relevant: http://blogs.msdn.com/b/dotnet/archive/2012/08/28/evolving-the-reflection-api.aspx

[jakesays-old]

What work needs to be done to restore this feature? I was looking at the coreclr repo and all the code to make this work seems to be in place. StackTrace and StackFrame are in mscorlib.dll, and the ecall method implementation of StackTrace.GetStackFramesInternal() is present in the clr. What else is needed?

[noahfalk]

dotnet/runtime#14447 had many different contributors on it and not all their scenarios were identical. The code I see in the repo right now supports scenarios where you want to programmatically inspect the stack trace from an exception. That part of the code was not present when the issue was first opened, so some of the scenarios have now been addressed.

The current code does not support other scenarios such as getting the callstack for the current thread. The next step is for someone to go through all the requests in dotnet/runtime#14447, determine what needs remain unfulfilled, and then make a specific proposal about new APIs that will address them. That proposal might consist of porting additional APIs from the full framework, or it might consist of alternate APIs designed to achieve the same goals. I'd also love to see suggestions about how we can guide the community to use these diagnostic APIs in reasonable ways to avoid some of the pain some developers hit in the past. That could be documentation, best practices, API naming/discoverability, carefully selecting how the APIs function, or maybe something else?

[nexussays]

The code I see in the repo right now supports scenarios where you want to programmatically inspect the stack trace from an exception.

Could you drop a link? I can't seem to find this in the repo.

[noahfalk]

The actual implementation is tucked down in mscorlib in the coreclr repo, making it harder to find : )
https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Diagnostics/Stacktrace.cs

The reference assembly you would build against is here:
https://github.com/dotnet/corefx/tree/master/src/System.Diagnostics.StackTrace/ref

At build time for .Net Core we should be auto-generated the runtime assembly that has typeforwarders so that references to System.Diagnostics.StackTrace and friends bind against the implementation in mscorlib.

I haven't built and tested an example myself against the repo, but given the source I assumed things were in working order. If that isn't the case then certainly we'd need to add some additional work to get it fixed up.

[nexussays]

Ah, thanks, I had found the CoreFX one but the CoreCLR one eluded me.

So I suppose when anyone gets a chance to test and confirm this is working we can close the issue.

[aggieben]

The current code does not support other scenarios such as getting the callstack for the current thread.

I don't think this issue should be closed until this scenario is addressed because it was the primary motivation behind dotnet/runtime#14447, which I opened. In my particular case, having to throw an exception to get a stacktrace would make the diagnostic tool (miniprofiler) pretty intrusive and would make timings less meaningful.

I realize that I haven't contributed anything aside from the issue itself, and I wish I could have done more by now, but things don't always work out the way I wish they would, especially when it comes to volunteer time. Be that as it may, I don't think the issue is resolved.

[nexussays]

Good point. All that should require is adding a new constructor to StackTrace:

c# public sealed partial class StackTrace { public StackTrace(System.Exception exception, bool needFileInfo) { }

The implementation in CoreCLR seems to already fit the use-case if that API addition were made.

[tushargupta51]

I am doing something similar to @aggieben and would really like to see Stacktrace().GetFrames() kind of API in coreclr.
Scenario: We have an EventSource based logger that is shared by several projects. We want to log the assembly name and version whenever a method uses the logger to log any messages and we are using the info in the stack frame for that.
I haven't yet gone through all of the comments in dotnet/runtime#14447 (its pretty huge!) so not sure where we landed up with the API request.

[304NotModified]

+1 from the NLog team!

[phenixdotnet]

Hi.
@304NotModified say we need it to port NLog to coreclr so I would like to know if someone is working on this subject ? If not what are the expectations for this ? Can we just add missing constructors and continue to use the ref implementation ?

As a test I added the default constructor in coreclr :

System.Diagnostics.StackTrace t = new System.Diagnostics.StackTrace();

My simple test run on coreclr x64 darwin and mono runtime (with and without dnx), I just don't get the filename and line number.

Regards

[304NotModified]

Is someone working on this?

Missing this feature is a big showstopper for logging tools, so also NLog. We (NLog) are one of the few non-Microsoft (open source) libraries which are in the Nuget top-100, and we like to stay there ;)

[noahfalk]

We're tracking it and we know its important, but nobody is working on it right now to the best of my knowledge. As soon as someone starts ideally we'd get the 'up-for-grabs' tag removed to ensure multiple people aren't needless duplicating work.

[304NotModified]

@noahfalk thanks for the update.

As this is an "up for grabs", can someone point us in the right direction how this can be implemented? Is the source in Stacktrace.cs in coreclr/mscorlib as good start?

FYI we (NLog) need it mostly to find the type of the caller. If there was another way, that would be also nice - To bad there is no CallerTypeNameAttribute yet, see https://github.com/dotnet/roslyn/issues/351.

[noahfalk]

@304NotModified - I'll trust you are aware that the stack trace makes no guarantee you will get the immediate caller frame. Optimizations can remove those frames from the stack. Also on AOT you might get the right method but be unable to resolve its name at runtime. CallerTypeName would put you on much sounder footing if/when it becomes available.

I think these would be the next steps for that constructor - but admittedly I might be a little out of date as I don't work in the CoreFX part of the source that frequently. @weshaggard might be able to comment if I am missing anything.
1) Add the constructor to the reference API definition https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/ref/System.Diagnostics.StackTrace.cs
2) Change the assembly version: https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/ref/System.Diagnostics.StackTrace.csproj
3) Go through the API review process.
4) Add an implementation for AOT: https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/src/System/Diagnostics/StackFrame.netcore50aot.cs
(the implementation for coreclr is already present in mscorlib.dll, I bet it works unchanged)
5) Add a test for it in a new tests directory here: https://github.com/dotnet/corefx/tree/master/src/System.Diagnostics.StackTrace. You can see other sibling assemblies that have tests as an example.
6) Open the PR and get the implementation reviewed.

[304NotModified]

@304NotModified - I'll trust you are aware that the stack trace makes no guarantee you will get the immediate caller frame. Optimizations can remove those frames from the stack.

Well that isn't the case when using [MethodImpl(MethodImplOptions.NoInlining)].

Also on AOT you might get the right method but be unable to resolve its name at runtime.

Can you elaborate on that?

CallerTypeName would put you on much sounder footing if/when it becomes available.

Maybe. But currently we would like to use the caller attribute, but we can't. It's a binary breaking change in the core of whole NLog. And it looks like that if new caller attributes are added to the compiler/C#, we have again breaking changes - so it's not future proof at all.. :(

think these would be the next steps for that constructor - but admittedly I might be a little out of date as I don't work in the CoreFX part of the source that frequently.

Thanks! Very helpful!

I guess we can't use this extern not in dotnet 5.3? But what about 5.4?

[noahfalk]

Well that isn't the case when using [MethodImpl(MethodImplOptions.NoInlining)] .

You might still run into issues with jitted code tail calls or unexpected callers injected by C# refactoring the IL (iterators, async, future language features)? I don't know for a fact that you will, but I'm also not promising anyone that there exists any specific set of steps that guarantees you won't.

For .Net Native, metadata (which includes method names) are not included for all methods by default to reduce the size of binaries. At compile time, the compiler does a fairly sophisticated analysis trying to deduce what metadata will be necessary and includes only that metadata. However the only way to be certain that stack traces would always have method names is to include metadata for all methods. Doing this would largely defeat the purpose of the optimization. Thus on .Net Native any given StackFrame may have a null MethodInfo property. The StackFrame instead provided an IP which can be resolved with PDBs. We've got an open feature request to allow developers to opt-in to including method names in the binary without including other kinds of metadata, but that's as far as it has gotten at this point.

It's a binary breaking change in the core of whole NLog...

Certainly I'll let you weight the pros and cons of the different choices. You might decide that despite the issues with StackTrace it is still your best option. I just want you to have all the facts so you can make the best choice.

[304NotModified]

You might still run into issues with jitted code tail calls or unexpected callers injected by C# refactoring the IL (iterators, async, future language features)? I don't know for a fact that you will, but I'm also not promising anyone that there exists any specific set of steps that guarantees you won't.

Agree, we have some logic on that.

For .Net Native, metadata (which includes method names) are not included for all methods by default to reduce the size of binaries.

We mostly need it for the (full) type name. Method name is less important.

Certainly I'll let you weight the pros and cons of the different choices. You might decide that despite the issues with StackTrace it is still your best option. I just want you to have all the facts so you can make the best choice.

Yes, thanks for the info. :+1:

[noahfalk]

We mostly need it for the (full) type name. Method name is less important.

Interesting, wouldn't have expected that. It won't offer any reprieve right now because type names are in the same boat, but definitely something we can keep in mind when trying to do some work to improve the situation

[304NotModified]

1) Add the constructor to the reference API definition https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/ref/System.Diagnostics.StackTrace.cs
2) Change the assembly version: https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/ref/System.Diagnostics.StackTrace.csproj
3) Go through the API review process.
4) Add an implementation for AOT: https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/src/System/Diagnostics/StackFrame.netcore50aot.cs
(the implementation for coreclr is already present in mscorlib.dll, I bet it works unchanged)
5) Add a test for it in a new tests directory here: https://github.com/dotnet/corefx/tree/master/src/System.Diagnostics.StackTrace. You can see other sibling assemblies that have tests as an example.
6) Open the PR and get the implementation reviewed.

Can someone elaborate on 4 ?

[noahfalk]

Sorry I may have referenced the wrong file if that contributed to the confusion:
https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.StackTrace/src/System/Diagnostics/StackTrace.netcore50aot.cs

When you add a new constructor to the reference assembly, it creates a contract that needs to be fulfilled by the various implementation assemblies that might be loaded at runtime. For the CoreCLR based runtime that constructor probably already exists in mscorlib.dll so there is little implementation work to be done. However for AOT (aka .Net Native aka the runtime that is used for UWP apps) mscorlib.dll is not used and no implementation of the new constructor will exist. We will need to design and implement something, even if it is a very trivial something like throwing NotImplementedException.

[304NotModified]

Thanks @noahfalk !

With the current extern in the current implementation, it think it's not trivial to create an aot implementation isn't it?

[noahfalk]

I'm not sure which extern you are refering to? I think adding a constructor that throws NotImplementedException would be striaghtforward. Adding anything useful is probably not feasible without some support directly from the lower layers of the .Net Native runtime that someone here at Microsoft would need to add.

[304NotModified]

I'm not sure which extern you are refering to?

This one, as a start for the implementation.

. Adding anything useful is probably not feasible without some support directly from the lower layers of the .Net Native runtime that someone here at Microsoft would need to add.

I was also afraid for that. Would really help, but dunno how.

[noahfalk]

The AOT implementation probably wouldn't go through that particular call path where the extern you mentioned lives, but in a broad sense, yes the AOT implementation is going to need some kind of internal runtime call.

You can implement this for .Net Core, you just have to stub out the implementation for .Net Native with a throw NotImplementedException.

[304NotModified]

So this will be netstandard2.0 ? https://twitter.com/migueldeicaza/status/780501106443055104

https://github.com/dotnet/standard/blob/master/netstandard/ref/mscorlib.cs#L5035-L5070

[304NotModified]

So this will be netstandard2.0 ? https://twitter.com/migueldeicaza/status/780501106443055104

https://github.com/dotnet/standard/blob/master/netstandard/ref/mscorlib.cs#L5035-L5070

Could someone confirm this?

[danmosemsft]

@304NotModified as you noted since these are in the standard we plan to implement them in this corefx repo. This is trackedi n https://github.com/dotnet/corefx/issues/12260

[joperezr]

Done by dotnet/corefx#12527