fix a static qt5 crash when using certain versions of libxcb #5915
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See here for background: https://bugreports.qt.io/browse/QTBUG-34748 libxcb temporarily had an abi breakage which caused crashes when qt was compiled against a non-compatible version. Building qt with -qt-xcb should have shielded us from this issue, except that incompatible headers were used when building qt's wrapper. Make sure those headers aren't picked up by qt's build. Details: qt's build adds a wrapper around the xcb libs when -qt-xcb is used. This is done to avoid having to link to a handful of different libs, which may not be api/abi stable. This build depends on include-order, so that its files are found before the real libxcb headers. Our build (for other reasons related to qt's complicated build-system) injects our prefix into CXXFLAGS. Because libxcb is found in this path, that reverses the include-order, negating the purpose of the wrapper. To fix, libxcb's includes are simply moved to a subdir. pkg-config ensures that they're still found properly when needed. To make things even more interesting, this behavior in qt's .pro files is broken: INCLUDEPATH += $$QMAKE_CFLAGS_XCB The INCLUDEPATH variable is processed by qmake which automatically prefixes each entry with "-I". The QMAKE_CFLAGS_XCB variable comes from pkg-config and already contains -I, making the path look like "-I-I/path/to/xcb/headers". To work around that, CFLAGS/CXXFLAGS are used here rather than INCLUDEPATH.
|
Thanks for analyzing this. Qt's complex hand-rolled build system can really be a drag sometimes. Tested ACK. |
laanwj
added a commit
that referenced
this pull request
Mar 17, 2015
bb44d9e depends: fix a static qt5 crash when using certain versions of libxcb (Cory Fields)
|
Post merge tested ACK (gitian build linux 64bit on ubuntu 14.04). |
hebasto
added a commit
to hebasto/bitcoin
that referenced
this pull request
Mar 7, 2021
A fix for QTBUG-34748 was introduced in bitcoin#5915 (v0.11.0, Qt 5.2.1). QTBUG-34748 was fixed in version 5.3.0. The separated patch file, provided by bitcoin#5915, was dropped in bitcoin#12971 while bumping Qt to 5.9.4 (5.9.6). But libxcb.mk remained unchanged. This change reverts bitcoin#5915 for libxcb.mk.
fanquake
added a commit
that referenced
this pull request
Mar 9, 2021
173ef89 build: Small libxcb.mk improvements (Hennadii Stepanov) 5129b36 build: Clean remnants of QTBUG-34748 fix (Hennadii Stepanov) Pull request description: Hope, this PR will make [transit](#21376) to Qt 5.12.10 neater. A fix for [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was introduced in #5915 (v0.11.0, Qt 5.2.1). [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was [fixed](qt/qtbase@b19b080) in Qt 5.3.0. The separated [`fix-xcb-include-order.patch`](https://github.com/theuni/bitcoin/blob/bb44d9e7546e6118cd91db5bbe471a3ce2ee7fcd/depends/patches/qt/fix-xcb-include-order.patch), provided by #5915, was dropped in #12971 while bumping Qt to 5.9.4 (5.9.6). But `libxcb.mk` remained unchanged. This PR reverts #5915 for `libxcb.mk` as well. ACKs for top commit: practicalswift: cr ACK 173ef89: patch looks correct fanquake: ACK 173ef89 Tree-SHA512: 9815a7e532ff4aa08f9623ded8d5708eca1c9c73ac7a2684419a18c125da7627b44ac3191f2e7978946942c8d0580e73b1a93df624986fb2a13791a68ce1e025
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Mar 9, 2021
173ef89 build: Small libxcb.mk improvements (Hennadii Stepanov) 5129b36 build: Clean remnants of QTBUG-34748 fix (Hennadii Stepanov) Pull request description: Hope, this PR will make [transit](bitcoin#21376) to Qt 5.12.10 neater. A fix for [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was introduced in bitcoin#5915 (v0.11.0, Qt 5.2.1). [QTBUG-34748](https://bugreports.qt.io/browse/QTBUG-34748) was [fixed](qt/qtbase@b19b080) in Qt 5.3.0. The separated [`fix-xcb-include-order.patch`](https://github.com/theuni/bitcoin/blob/bb44d9e7546e6118cd91db5bbe471a3ce2ee7fcd/depends/patches/qt/fix-xcb-include-order.patch), provided by bitcoin#5915, was dropped in bitcoin#12971 while bumping Qt to 5.9.4 (5.9.6). But `libxcb.mk` remained unchanged. This PR reverts bitcoin#5915 for `libxcb.mk` as well. ACKs for top commit: practicalswift: cr ACK 173ef89: patch looks correct fanquake: ACK 173ef89 Tree-SHA512: 9815a7e532ff4aa08f9623ded8d5708eca1c9c73ac7a2684419a18c125da7627b44ac3191f2e7978946942c8d0580e73b1a93df624986fb2a13791a68ce1e025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one's really subtle. Fixes #5910. Confirmed fixed in gitian.
See here for background: https://bugreports.qt.io/browse/QTBUG-34748
libxcb temporarily had an abi breakage which caused crashes when qt was compiled against a non-compatible version. Building qt with -qt-xcb should have shielded us from this issue, except that incompatible headers were used when building qt's wrapper.
Make sure those headers aren't picked up by qt's build.
Details:
qt's build adds a wrapper around the xcb libs when -qt-xcb is used. This is done to avoid having to link to a handful of different libs, which may not be api/abi stable. This build depends on include-order, so that its files are found before the real libxcb headers.
Our build (for other reasons related to qt's complicated build-system) injects our prefix into CXXFLAGS. Because libxcb is found in this path, that reverses the include-order, negating the purpose of the wrapper.
To fix, libxcb's includes are simply moved to a subdir. pkg-config ensures that they're still found properly when needed.
To make things even more interesting, this behavior in qt's .pro files is broken:
INCLUDEPATH += $$QMAKE_CFLAGS_XCB
The INCLUDEPATH variable is processed by qmake which automatically prefixes each entry with "-I". The QMAKE_CFLAGS_XCB variable comes from pkg-config and already contains -I, making the path look like "-I-I/path/to/xcb/headers".
To work around that, CFLAGS/CXXFLAGS are used here rather than INCLUDEPATH.