Skip to content
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

StackFrame.GetILOffset returns -1 for ReJITted methods #8296

Closed
pharring opened this issue Jun 6, 2017 · 9 comments
Closed

StackFrame.GetILOffset returns -1 for ReJITted methods #8296

pharring opened this issue Jun 6, 2017 · 9 comments
Assignees
Labels
area-Diagnostics-coreclr backlog-cleanup-candidate An inactive issue that has been marked for automated closure. bug no-recent-activity
Milestone

Comments

@pharring
Copy link
Contributor

pharring commented Jun 6, 2017

Title says it all. This was tested on CoreCLR 2.0.0-preview2-006098 and desktop .Net 4.6.2
To be clear, this is System.Diagnostics.StackFrame.GetILOffset()
I can give background offline, if required, but @noahfalk has the details.

@mikem8361 mikem8361 self-assigned this Jun 6, 2017
@lt72
Copy link
Contributor

lt72 commented Jun 7, 2017

@noahfalk: PTAL

@noahfalk
Copy link
Member

noahfalk commented Jun 7, 2017

@lt72 - Its a known pre-existing issue that this doesn't work properly, but it would be good to fix. Working around it isn't pleasant and we definately can't afford to have tiered jitting cause a similar impact.

@AndyAyersMS
Copy link
Member

@noahfalk can you expound a bit more on why it doesn't work, and roughly what needs to be fixed?

@noahfalk
Copy link
Member

noahfalk commented Jun 7, 2017

The issue I'm aware of (which may not be the only issue present) is a lock ordering problem. We've got both the original IL->native offset data and the rejit provided IL'->IL mapping data, but they are in data structures the code can't easily access due to lock ordering. I'm guessing a potential solution is to avoid dealing with the cached IL->native data held under the debugger lock and instead go directly to the lower level IL->native data being maintained by the codemanager.

If you were worried that this might be JIT related I don't think there is any issue in that area.

@noahfalk
Copy link
Member

noahfalk commented Jun 7, 2017

If someone is planning on working on this in the near term I'm happy to go into more detail, I just didn't expect this would get any traction right away.

@lt72 lt72 assigned davmason and unassigned mikem8361 Nov 7, 2017
@lt72
Copy link
Contributor

lt72 commented Nov 7, 2017

@davmason: PTAL

@lt72
Copy link
Contributor

lt72 commented Feb 21, 2018

@davmason: did you get a chance to dig more into this?

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
MichalStrehovsky pushed a commit to MichalStrehovsky/runtime that referenced this issue Oct 12, 2020
There are multiple cases were we don't need just the pointer for an external variable but the value that is stored at this location.
So far this was done with the PREPARE_EXTERNAL_VAR followed by an ldr x? [x?]. The PREPARE_EXTERNAL_VAR macro needs two instructions (adrp + add). As the ldr instruction supports an offset we can eliminate the add for this use case. The two new macros PREPARE_EXTERNAL_VAR_INDIRECT and PREPARE_EXTERNAL_VAR_INDIRECT_W make use of this.
@ghost
Copy link

ghost commented Oct 27, 2022

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@ghost ghost added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Oct 27, 2022
@ghost
Copy link

ghost commented Nov 10, 2022

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@ghost ghost closed this as completed Nov 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Diagnostics-coreclr backlog-cleanup-candidate An inactive issue that has been marked for automated closure. bug no-recent-activity
Projects
None yet
Development

No branches or pull requests

7 participants