Skip to content

[cDAC] Handle non-mapped images correctly #114688

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

Merged
merged 7 commits into from
Apr 23, 2025

Conversation

max-charlamb
Copy link
Contributor

@max-charlamb max-charlamb commented Apr 15, 2025

Fixes issue where cDAC could not load flat images correctly. Instead of using the RVA to find the correct section and offset, it would previously return the RVA of an unloaded image.

I went through an iteration where I parsed the RVAs manually depending on if the image was loaded, however I felt this was a lot of logic to have in the Module : IData<Module> class. The newest revision utilizes the libraries PEReader to fetch the module's metadata. In my view, the only issue with this current approach is some messiness around wrapping the Target in a Stream. The size doesn't really make sense for this use case.
See alternative fix: #114742

Tested on both loaded and unloaded images using !clrma

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 15, 2025
@max-charlamb max-charlamb added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 15, 2025
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

@max-charlamb max-charlamb marked this pull request as ready for review April 16, 2025 15:23
@Copilot Copilot AI review requested due to automatic review settings April 16, 2025 15:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@max-charlamb max-charlamb merged commit 19a02d5 into dotnet:main Apr 23, 2025
145 of 149 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants