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

Various fixes to trampolines for embedding on Windows #39052

Merged
merged 3 commits into from
Dec 31, 2020

Conversation

staticfloat
Copy link
Sponsor Member

@GunnarFarneback this fixed your dynamic embedding test suite on Windows for me; I'd appreciate if you could corroborate.

Many thanks to @Keno and @vchuravy for teaching me how to track down the reason behind the segfault; turns out it was an improperly-named section that wasn't getting the right execute bits set for its pages. The tool to figure this out is vmmap, made by sysinternals, as all good windows tools usually are.

This allows libjulia's initialization routines to properly output to
`stdout` in embedding scenarios.
By naming the section `text` instead of `.text` these sections were
mapped into memory without the executable bit set, causing segfaults
when attempting to use any trampolines on Windows.
There are multiple ways of fixing this, but let's just not let
trampolines get re-set after they've already been set.
@Keno Keno merged commit 1ad6aed into master Dec 31, 2020
@Keno Keno deleted the sf/win_embedding_fixes branch December 31, 2020 03:54
@KristofferC
Copy link
Sponsor Member

I guess backport?

@Keno Keno added the backport 1.6 Change should be backported to release-1.6 label Dec 31, 2020
@Keno
Copy link
Member

Keno commented Dec 31, 2020

yes

@GunnarFarneback
Copy link
Contributor

I can confirm that this fixes the tests for DynamicallyLoadedEmbedding when running the nightly build locally on Windows. It still failed the GitHub actions tests on Windows nightly, https://github.com/GunnarFarneback/DynamicallyLoadedEmbedding.jl/runs/1629567916, but I don't know if that might be lagging behind. The log does include the output of julia --version but since that only says julia version 1.7.0-DEV it's not very helpful.

@staticfloat
Copy link
Sponsor Member Author

Yeah, try it again in a couple of hours and see if it still fails.

@GunnarFarneback
Copy link
Contributor

Waiting was enough. Now it's passing.

@KristofferC KristofferC mentioned this pull request Jan 5, 2021
26 tasks
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Jan 6, 2021
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