Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 5.6k
Add cDac contract for ResolveFrameHelper#129261
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
0d9f7069f19732b6c8c7a3d2fcc92a8047eFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -48,4 +48,10 @@ public override void HandleFaultingExceptionFrame(FaultingExceptionFrame frame) | ||
| // that does not support holding any extended state. | ||
| _holder.Context.ContextFlags &= ~(uint)(ContextFlagsValues.CONTEXT_XSTATE & ContextFlagsValues.CONTEXT_AREA_MASK); | ||
| } | ||
| public override void HandleResolveHelperFrame(ResolveHelperFrame frame) | ||
| { | ||
| base.HandleResolveHelperFrame(frame); | ||
| UpdateArgumentRegisters(frame.TransitionBlockPtr); | ||
| } | ||
leculver marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| namespace Microsoft.Diagnostics.DataContractReader.Data; | ||
| [CdacType(nameof(DataType.ResolveHelperFrame))] | ||
| internal partial class ResolveHelperFrame : IData<ResolveHelperFrame> | ||
| { | ||
| [Field] public TargetPointer TransitionBlockPtr { get; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -166,6 +166,7 @@ public enum DataType | ||
| InlinedCallFrame, | ||
| SoftwareExceptionFrame, | ||
| FramedMethodFrame, | ||
| ResolveHelperFrame, | ||
leculver marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| FuncEvalFrame, | ||
| ResumableFrame, | ||
| FaultingExceptionFrame, | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was test-only code for Native AOT. I do not think it is currently reachable.
Are you able to actually hit this in tests?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is from code review only. I mostly want to make sure that we aren't missing a cDac contract for some scenario which would require servicing later. This seems to be the only coreclr!Frame that isn't wrapped. If this is pure test code that will never ship, I will close this PR. That wasn't obvious from source inspection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scanning through it looks like
DOTNET_JitForceControlFlowGuard=1can still reach this, and this code is dac-ized in the legacy dac. Seems like it would be safer to wrap it in case we need to debug something where it shows up.I'm happy to close without merging if you disagree though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think it is reachable currently.
ResolveHelperFrameis only used byVSD_ResolveWorkerForInterfaceLookupSlot.VSD_ResolveWorkerForInterfaceLookupSlotis only called byJIT_InterfaceLookupForSlot.JIT_InterfaceLookupForSlotID isCORINFO_HELP_INTERFACELOOKUP_FOR_SLOT.CORINFO_HELP_INTERFACELOOKUP_FOR_SLOTis never emitted by the JIT currently.I think you can delete this whole chain if you would like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'll close this one and open a new one to do exactly that.