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 configure option '--default-pic' to enable '-fPIC' by default #805

Merged
merged 2 commits into from
Apr 27, 2019
Merged

Add configure option '--default-pic' to enable '-fPIC' by default #805

merged 2 commits into from
Apr 27, 2019

Conversation

umarcor
Copy link
Member

@umarcor umarcor commented Apr 25, 2019

Currently, GHDL can be built as:

OPT_FLAGS="-fPIC" \
LIB_CFLAGS="-fPIC" \
./dist/travis/build.sh -b llvm -p ghdl-llvm-fPIC

Then, sources must be analyzed with -fPIC in order to generate a PIE binary. E.g., in VUnit:

ui.set_compile_option("ghdl.flags", ['-fPIC'])
ui.set_sim_option("ghdl.elab_flags", ['-fPIC', '-Wl,-pie'])

This PR allows to simplify the commands above to:

CONFIG_OPTS="--enable-default-pie " \
./dist/travis/build.sh -b llvm -p ghdl-llvm-fPIC

and

ui.set_sim_option("ghdl.elab_flags", ['-Wl,-pie'])

I.e., if GHDL is built with --enable-default-pie, -fPIC is implicitly added (by default) to any ghdl -a or ghdl -e command.

This PR sets the default of variable default_pie in configure to false. It is enabled automatically if gcc was built with enable-default-pie, as it is now. The new feature is that it can be explicitly set through --enable-default-pie.

However, this alone is not enough to achieve the desired result. PIC_FLAGS must be added to GRT_FLAGS, in order to force all the objects in libghdl.a to be position independent.

Note that I changed the values of default_pie from no|yes to false|true, for coherency.

@umarcor
Copy link
Member Author

umarcor commented Apr 26, 2019

@tgingold, I believe this is ready for reaview now. A test for this PR is available in #803. Running the script on fedora:29, ubuntu:trusty or ubuntu:xenial will not be able to load the binary as a shared library. Instead, if this branch is used (feat-default-pie), all of them will work.

Moreover, I think that #670 and #804 could be safely reverted with this PR, because the use cases are already covered with the now visible configure option; which is a much clearner solution, I think.

@tgingold
Copy link
Member

tgingold commented Apr 27, 2019 via email

@umarcor
Copy link
Member Author

umarcor commented Apr 27, 2019

I have a simple request: don't hijack enable-default-pie, just add a new option like enable-pic

I updated it, but I don't know if that is what you mean.

EDIT

Maybe you meant to add Default_Pic to default_paths.ads.in?

@umarcor umarcor changed the title Add configure option '--enable-default-pie' to enable '-fPIC' by default Add configure option '--default-pic' to enable '-fPIC' by default Apr 27, 2019
@tgingold tgingold merged commit 2737980 into ghdl:master Apr 27, 2019
@tgingold
Copy link
Member

Thanks. I will adjust it a little bit so that libgrt is not built with -fpic by default.

@umarcor umarcor deleted the feat-default-pie branch April 27, 2019 18:34
umarcor added a commit to dbhi/containers that referenced this pull request May 7, 2019
umarcor added a commit to dbhi/containers that referenced this pull request May 8, 2019
umarcor added a commit to dbhi/containers that referenced this pull request May 8, 2019
@Paebbels Paebbels added this to the v0.37 milestone Jul 7, 2019
@umarcor umarcor mentioned this pull request Oct 13, 2019
1 task
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.

None yet

3 participants