-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Move JIT debug image registration to CodeMemory #9470
Conversation
#[cfg(all( | ||
any(target_os = "linux", target_os = "macos"), |
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.
Since these tests are #[ignore]
d by default, and run fine on Windows when explicitly requested, it seems there is no reason to have these guards anymore.
4e4577c
to
7097540
Compare
JIT images correspond to ELF images, which may represent multiple modules within a single component.
7097540
to
5765fd2
Compare
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 for the PR as well as for the issue! This looks like the right fix to me. I suspect that writing a test for this will be relatively hard, so otherwise having the preexisting tests pass I think is sufficient for this.
I also think that the bug you highlighted where we're double-registering with the native profiler is probably also an issue as well. If you'd like that's ok to defer to a second PR to fix, or if you'd like bundling it in here I think would be reasonable too
Opened #9478 about this. The fix there is going to be a little trickier since the profiler's "get name" callback operates on actual per-module state. |
JIT images correspond to ELF images, which may represent multiple modules within a single component.
Fixes #9461.
Tested manually on a moderately large component as well as with the existing LLDB tests.