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

Compile with --pic on Linux if linker is gcc #1819

Closed
wants to merge 1 commit into from
Closed

Compile with --pic on Linux if linker is gcc #1819

wants to merge 1 commit into from

Conversation

Theodus
Copy link
Contributor

@Theodus Theodus commented Apr 6, 2017

Previously, users on platforms where gcc enables PIE by default were
required to compile with the --pic flag. This change automatically
compiles with --pic on Linux and FreeBSD when the linker is gcc.

Resolves #1811

Previously, users on platforms where gcc enables PIE by default were
required to compile with the --pic flag. This change automatically
compiles with --pic on Linux and FreeBSD when the linker is gcc.
@Theodus Theodus added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Apr 6, 2017
@SeanTAllen
Copy link
Member

Can you explain what is happening here @Theodus. I'm looking at the commit and don't understand how it addresses the --pic issue.

@Theodus
Copy link
Contributor Author

Theodus commented Apr 7, 2017

@SeanTAllen on LInux and FreeBSD the opt.pic is set to true if the linker is cc or gcc: https://github.com/ponylang/ponyc/pull/1819/files#diff-35dfc63f568c5616abecb849fd7abdd3R373
This requires the environment variable CC to be checked before genexe.

@SeanTAllen
Copy link
Member

SeanTAllen commented Apr 7, 2017

I think we need a more robust test for "they are using gcc" than

if(strcmp(opt.linker, "gcc") == 0 || strcmp(opt.linker, "cc") == 0)

I don't know what is possible but a basic string comparison seems very brittle.

@Theodus
Copy link
Contributor Author

Theodus commented Apr 7, 2017

I agree but I don't know what else could be done. I would use the output of --version, but with cc it will not specify that it is gcc, it will just give the same version info and copyright notice:

theodus@blade /mnt/c/Users/theo/dev/ponyc (issue-1811) $ cc --version
cc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. 

@SeanTAllen
Copy link
Member

I think we should discuss at sync and get feedback because I as well don't know what else we can do. Unless we start doing "configure" style probing.

@SeanTAllen SeanTAllen added needs discussion during sync changelog - changed Automatically add "Changed" CHANGELOG entry on merge and removed changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge last week in pony labels Apr 7, 2017
@Theodus
Copy link
Contributor Author

Theodus commented Apr 12, 2017

After discussing on the sync meeting we have decided not to do this.

@Theodus Theodus closed this Apr 12, 2017
@Theodus Theodus deleted the issue-1811 branch April 12, 2017 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always use --pic when using GCC on Linux
3 participants