-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Alternative fix for #6248 (qt+fPIE) #6978
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
Conversation
This allows for fPIE to be used selectively.
But only if qt was built with reduced relocations.
|
Great work, utACK (will test later) |
|
Nice! utACK. |
|
On Ubuntu 15.10 this gives: Which is correct. However on On Ubuntu 14.04 I also get: While I know in fact that that does work. Error in config.log is: Strange. QT_REDUCE_RELOCATIONS is indeed enabled: However it just works with fPIE. Gcc version issue? |
|
@laanwj I believe it's a Qt issue. They tightened the requirements with newer qt versions, -fPIE used to compile fine. But -fPIE + gcc5 + older Qt will still likely cause problems regardless if it does compile. So the real issue is -fPIE + newer gcc regardless of when qt tightened the error. I think it makes sense to always fall back to -fPIC any time that reduce_relocations was used. The alternative would be a blacklist/whitelist for compilers, which i was trying to avoid. |
|
Wouldn't compiling+linking an example program w/ -fPIE and Qt be the most sure way to check? Or is there a specific reason you didn't go that route? |
|
@laanwj It compiles/links fine with the busted config. So there's no point in linking if we're not going to run it, and a run-test is a no-go because it doesn't work with cross. |
|
The alternative would be compiling a stub object with fPIE and objdump to check for relocations. |
|
Huh are we talking about the same issue? |
|
This is the error I get (one for every executable that uses qt): |
|
@laanwj Sorry for being unclear. To summarize the issue: Qt build can enable -Bsymbolic-functions via the reduce-relocations config option. The qt devs assumed that -fPIC or -fPIE would disable copy-relocations, but gcc5 changed that behavior and allows copy-relocs with -fPIE. So -fPIC is the only way to guarantee that -Bsymbolic-functions will work as intended. When Qt discovered this issue (somewhere around 5.4), they added the check in qconfig.h so that compilation would fail in this QT_REDUCE_RELOCATIONS+-fPIE case. Prior to that, the check looked like this: #if !defined(QT_BOOTSTRAPPED) && defined(QT_REDUCE_RELOCATIONS) && defined(__ELF__) && !defined(__PIC__) && !defined(__PIE__)
# error "You must build your code with position independent code if Qt was built with -reduce-relocations. "\
"Compile your code with -fPIC or -fPIE."
#endifJust because compiling succeeds in versions with the old check, that does not mean that it's safe to build/run with gcc5+. So testing to see if something builds against qt+-fPIE is not enough, as older versions weren't aware of the problem. |
|
Ok, fair enough |
|
Tested ACK. |
This allows for fPIE to be used selectively. Github-Pull: bitcoin#6978 Rebased-From: 17c4d9d
But only if qt was built with reduced relocations. Github-Pull: bitcoin#6978 Rebased-From: 69d0513
This allows for fPIE to be used selectively. Github-Pull: bitcoin#6978 Rebased-From: 17c4d9d
This allows for fPIE to be used selectively. Github-Pull: bitcoin#6978 Rebased-From: 17c4d9d
But only if qt was built with reduced relocations. Github-Pull: bitcoin#6978 Rebased-From: 69d0513
|
pardon me for necroing this thread but if -fPIC weakens PaX/Grsec patches then i wonder if silently using it as a fallback option is the right way to solve this. |
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074.
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074.
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074.
Build system improvements Includes commits cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6978 - Only the first commit (second is for QT) - bitcoin/bitcoin#7059 - bitcoin/bitcoin#7603 - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT) - bitcoin/bitcoin#7954 - bitcoin/bitcoin#8314 - Only the second commit (first is for QT) - bitcoin/bitcoin#8504 - Only the first commit (second was undoing something we didn't have) - bitcoin/bitcoin#8520 - bitcoin/bitcoin#8563 - bitcoin/bitcoin#8249 - bitcoin/bitcoin#9156 - bitcoin/bitcoin#9831 - bitcoin/bitcoin#9789 - bitcoin/bitcoin#10766 Part of #2074. # Conflicts: # configure.ac # src/Makefile.am # src/Makefile.gtest.include # src/Makefile.test.include # zcutil/build.sh
Rather than setting -fPIC wholesale, only set it for the qt objects.
The first commit restructures to move the fPIE flag out, the second is the actual change.
Note that -fPIC is used for testing qt until we get to the actual fPIE test. That's to prevent us from bombing out early due to qt's header yelling about PIE before we actually want to test it.