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

add -fPIC to julia-config.jl #14443

Merged
merged 1 commit into from
Jan 8, 2016
Merged

add -fPIC to julia-config.jl #14443

merged 1 commit into from
Jan 8, 2016

Conversation

mlubin
Copy link
Member

@mlubin mlubin commented Dec 19, 2015

@yuyichao found that -fPIC is needed so that the GC stack is properly initialized. I've tested on linux only.

@yuyichao yuyichao added domain:embedding Embedding Julia using the C API backport pending 0.4 labels Dec 19, 2015
@yuyichao
Copy link
Contributor

Not just jl_pgcstack, but every global variables used by the main executable. Somehow without -fPIC the compiler creates static version of these variables in the executable. However, the linker doesn't seems to be consistent about which version to resolve to. libjulia.so, the executable and dlsym all resolve to the version in the executable but sys.so resolves to the one in libjulia.so.

@yuyichao
Copy link
Contributor

Should probably update the embedding doc too.

@yuyichao yuyichao added the needs docs Documentation for this change is required label Dec 19, 2015
@mlubin
Copy link
Member Author

mlubin commented Dec 19, 2015

done

@tkelman
Copy link
Contributor

tkelman commented Dec 19, 2015

-fPIC gives an annoying warning (and is the default) on windows. Is this needed on mac and 32 bit linux as well? If so it should be unix only.

@tkelman
Copy link
Contributor

tkelman commented Dec 19, 2015

And if julia-config isn't tested, it probably should be so we don't break it (maybe subset of #8757 though)

@mlubin
Copy link
Member Author

mlubin commented Dec 19, 2015

I'm not really in a position to add these tests, all I know is that without -fPIC on linux, the embedding code doesn't work.

@tkelman
Copy link
Contributor

tkelman commented Dec 19, 2015

Needs to at least be unix only, and maybe more specific to just linux? Just 64 bit linux? Or any 64 bit unix? Depending on where it's required.

@mlubin
Copy link
Member Author

mlubin commented Jan 1, 2016

Anyone have an idea if this is needed on OS X also?

@tkelman
Copy link
Contributor

tkelman commented Jan 1, 2016

or freebsd for that matter?

@mlubin
Copy link
Member Author

mlubin commented Jan 1, 2016

I doubt it's harmful though

@yuyichao
Copy link
Contributor

yuyichao commented Jan 1, 2016

We pass -fPIC on everywhere other than windows so we should do that for embedding code too.

@tkelman
Copy link
Contributor

tkelman commented Jan 1, 2016

On Windows it's a really annoying and prominent warning, so absolutely needs to be taken out there.

@mlubin
Copy link
Member Author

mlubin commented Jan 1, 2016

How's this?

@yuyichao yuyichao removed the needs docs Documentation for this change is required label Jan 1, 2016
@yuyichao
Copy link
Contributor

yuyichao commented Jan 1, 2016

LGTM

tkelman added a commit that referenced this pull request Jan 8, 2016
add -fPIC to julia-config.jl
@tkelman tkelman merged commit 9464e6f into JuliaLang:master Jan 8, 2016
@mlubin mlubin deleted the fpic branch January 8, 2016 22:08
mlubin added a commit that referenced this pull request Jan 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:embedding Embedding Julia using the C API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants