-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Refactor _swift_withWin32DbgHelpLibrary()
to avoid using GetCurrentProcess()
per Microsoft documentation
#62294
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
Refactor _swift_withWin32DbgHelpLibrary()
to avoid using GetCurrentProcess()
per Microsoft documentation
#62294
Conversation
@swift-ci please smoke test |
…ess() per Microsoft documentation
12ad7f0
to
e199f0b
Compare
@swift-ci please smoke test |
@swift-ci please smoke test |
@swift-ci please test windows |
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 don't think I ever noticed the fine print there. This is terrifying, this is terrible, and we should feel bad. Thanks for this fix, sorry for any scars that it may have inflicted upon you.
I particularly enjoy this part (emphasis mine):
So, they called it Maybe we'd be better off just casting the address of something to |
That comment lies. If you pass some other address with I suspect |
Joy 😂 |
@swift-ci please smoke test |
@swift-ci please smoke test |
The Microsoft documentation for the DbgHelp library warns developers not to pass
GetCurrentProcess()
as the handle toSymInitialize()
etc.This change uses a privately-held handle duplicated from
GetCurrentProcess()
. It also takes care to set DbgHelp library options consistently in case something elsewhere callsSymSetOptions()
and sets them to something incompatible with the runtime's expectations.We still have no way to thread-safely use the library in coordination with code outside the runtime, but at least the runtime should avoid stomping on shared system resources.
Resolves #62290.