-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add System.Diagnostics.StackFrame.GetMethodFromNativeIP API for VS4Mac #61289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
6b7f5a0
dbe2368
9a14e5d
b3a9a60
3aeaed6
308f640
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,9 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Text; | ||
| using System.Reflection; | ||
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace System.Diagnostics | ||
| { | ||
|
|
@@ -50,5 +53,23 @@ private void BuildStackFrame(int skipFrames, bool needFileInfo) | |
| } | ||
|
|
||
| private static bool AppendStackFrameWithoutMethodBase(StringBuilder sb) => false; | ||
|
|
||
| [DllImport(RuntimeHelpers.QCall, EntryPoint = "StackFrame_GetMethodDescFromNativeIP")] | ||
| private static extern RuntimeMethodHandleInternal GetMethodDescFromNativeIP(IntPtr ip); | ||
|
|
||
| /// <summary> | ||
| /// Returns the method info instance for the managed code IP address. | ||
| /// </summary> | ||
| /// <param name="ip">code address</param> | ||
| /// <returns>MethodBase instance for the method or null if IP not found</returns> | ||
| internal static MethodBase? GetMethodFromNativeIP(IntPtr ip) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably add a comment right here saying that the implementation of this method has known race conditions for unloadable code + MethodBase isn't a good design choice for working with NativeAOT, therefore don't make it public in the future without considering those issues. Reduces the chance we forget the history when we need a public API later.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or even better - create issue that tracks adding the public API and link it from here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a TODO describing most of this here: https://github.com/mikem8361/runtime/blob/b3a9a6086dcef769b82a5750489738254b6aee80/src/coreclr/vm/debugdebugger.cpp#L788
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that good enough?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did add a comment to the method's header.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do think having the message that you added right here in the header is valuable. When people convert a private API to public nothing enforces that they will look at implementation code in a separate file. The comment you added calls out the race condition but doesn't mention the design issue for using MethodBase in NativeAOT. As @jkotas suggested filing an issue and linking to that is an improvement over my original suggestion.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm going to use issue #61186 to detail these limitations and to propose the new 7.0 API. |
||
| { | ||
| RuntimeMethodHandleInternal method = GetMethodDescFromNativeIP(ip); | ||
|
|
||
| if (method.Value == IntPtr.Zero) | ||
| return null; | ||
|
|
||
| return RuntimeType.GetMethodBase(null, method); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.