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

Fix setting managed breakpoints in singlefile apps with direct-mapped R2R sections #84965

Merged
merged 1 commit into from Apr 18, 2023

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Apr 18, 2023

Fixes: #77574

PAGE_EXECUTE_READWRITE would not work for R2R executable sections if they are no-copy mapped from the file. We do that for performance reasons when doing layout in the context of singlefile. Copying is costly. But since the mapping is readonly it is not possible to change protection of mapped memory to anything that is writeable (it would imply that writes are reflected back to the file).

What should work in such scenario is PAGE_EXECUTE_WRITECOPY, so we should try that if setting PAGE_EXECUTE_READWRITE fails.

The fix makes setting breakpoints work and indirectly fixes #77574 as well since getting parallel stacks involves setting breakpoints to intercept thread starts.

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

For the repro in #77574 I now see:

image

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

I have done manual testing of this change by building singlefile apps with a singlefilehost.exe that contains an updated runtime.

I am not sure if there are automated tests for this as the manual repro is somewhat laborious and requires multiple steps (patch the singlefile.exe that SDK uses, build the app, drop appropriate dac and dbi alongside the app...)

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

CC: @tommcdon @mikelle-rogers @gregg-miskelly

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

Only question is - in FlatImageLayout, pages get allocated with CreateFileMapping with PAGE_EXECUTE_READ. Isn't it necessary to have the mapping be RW in the beginning regardless of the view? Or does this fall under the docs saying:

The private page is marked as PAGE_EXECUTE_READWRITE, and the change is written to the new page.

I am used to requiring the initial allocation to be broader than what VirtualAlloc tries to set it to, but I am not sure if this API is different.

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

The NativeAOT failure is #84979

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

Only question is - in FlatImageLayout, pages get allocated with CreateFileMapping with PAGE_EXECUTE_READ. Isn't it necessary to have the mapping be RW in the beginning regardless of the view?

The difference is that this is not allocating any memory. It creates a memory mapping object that could be used to make views and see the file content as memory. We do not want writeable mapping as it would require openning the file in writeable mode, which we do not want.

PAGE_EXECUTE_READ0x20 Allows views to be mapped for read-only, copy-on-write, or execute access.The file handle specified by the hFile parameter must be created with the GENERIC_READ and GENERIC_EXECUTE access rights.Windows Server 2003 and Windows XP:  This value is not available until Windows XP with SP2 and Windows Server 2003 with SP1.

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

Note that flat mapping by itself cannot be executed as-is as it would be laid out like in the file, not like it is intended to be in memory.

@tommcdon tommcdon self-requested a review April 18, 2023 18:39
Copy link
Member

@tommcdon tommcdon left a comment

Choose a reason for hiding this comment

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

Thanks!

@VSadov
Copy link
Member Author

VSadov commented Apr 18, 2023

Thanks!!!

@VSadov VSadov merged commit 425bfa1 into dotnet:main Apr 18, 2023
105 of 108 checks passed
@VSadov VSadov deleted the bpSf branch April 18, 2023 19:08
@ghost ghost locked as resolved and limited conversation to collaborators May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.NET Debugging services don't issue thread create callbacks for single file apps
4 participants