Skip to content

[RuntimeDyld][Windows] Allocate space for dllimport things. #9080

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 2 commits into from
Sep 3, 2024

Conversation

al45tair
Copy link

@al45tair al45tair commented Aug 9, 2024

We weren't taking account of the space we require in the stubs for things that are dllimported, and as a result we could hit the assertion failure for running out of stub space. Fix that.

rdar://133473673

We weren't taking account of the space we require in the stubs for
things that are `dllimport`ed, and as a result we could hit the
assertion failure for running out of stub space.  Fix that.

rdar://133473673
@al45tair al45tair requested review from compnerd and lhames August 9, 2024 08:35
@al45tair
Copy link
Author

al45tair commented Aug 9, 2024

@swift-ci Please test

@al45tair
Copy link
Author

al45tair commented Aug 9, 2024

Explanation: We use RuntimeDyld to load and relocate JITted code; this works by loading individual sections from the JITted object, but in order to do the relocations it needs to allocate space for stub code. On Windows, it also needs to allocate space for extra pointers when dealing with dllimported things. However, because of the way it works, it needs to allocate all the space for the section, the stubs and the extra pointers in one go, so there is code to compute the extra space required. That code was missing any support for the dllimport pointers, which in turn could lead to runtime failures because we didn't have enough space for the required stubs and pointers. (This also affects LLDB on Windows, FWIW.)
Risk: Extremely low. This only affects Windows.
Original PR: #9071
Reviewed by: @compnerd
Resolves: rdar://133473673
Tests: Tested manually with some of the Swift interpreter tests at a prompt.

No need for `TargetName` to exist separately, really.

Co-authored-by: Saleem Abdulrasool <[email protected]>
@al45tair al45tair force-pushed the eng/PR-133473673-6.0 branch from d2b82a2 to 3271621 Compare August 9, 2024 08:58
@al45tair
Copy link
Author

al45tair commented Aug 9, 2024

@swift-ci Please test

@al45tair
Copy link
Author

al45tair commented Aug 9, 2024

Windows build failed checkout; it didn't fail because of this PR.

@compnerd
Copy link
Member

compnerd commented Aug 9, 2024

@swift-ci please test windows platform

@fredriss fredriss merged commit f2649c2 into swiftlang:swift/release/6.0 Sep 3, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants