DevHeads.net

Review Request 110563: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

Review request for Build System and kdelibs.

Description
Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple times into the same process (with Bsymbolic-functions flag)" on kde-core-devel ( <a href="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2" title="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2">http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2</a> ) symbols from QtUitools.a are not hidden by default in Qt4 and thus will be added to the public symbols of the module/shared lib they are linked to. And thus can appear multiple times in the same process, resulting in symbol clashes and leading to problems at least with the Bsymbolic-functions flag or when being possibly incompatible versions.

Attached patch sees to solve that problem, by adding a macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags depending on the platform/linker used.

Only issue is that instead of some variable I had to use "QtUiTools.a" as I found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case another platform needs another name/prefix here?

Patch is against 4.10 branch, so I hope to get this in 4.10.4

<a href="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY" title="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY">http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_...</a> shows that there are some more places where the symbols need hiding, but I first want feedback on the proposed approach.

Diffs
cmake/modules/FindKDE4Internal.cmake cb63285
cmake/modules/KDE4Macros.cmake 3db4e24
kjsembed/kjsembed/CMakeLists.txt d70f260
kross/modules/CMakeLists.txt d245fd8
kross/qts/CMakeLists.txt d8cb4a5
plasma/CMakeLists.txt 674550d

Diff: <a href="http://git.reviewboard.kde.org/r/110563/diff/" title="http://git.reviewboard.kde.org/r/110563/diff/">http://git.reviewboard.kde.org/r/110563/diff/</a>

Testing

Thanks,

Friedrich W. H. Kossebau

Comments

Re: Review Request 110563: Crash fix: hide symbols from static l

By Friedrich W. H.... at 05/22/2013 - 06:19

(Updated May 22, 2013, 11:19 a.m.)

Review request for Build System, kdelibs, Alexander Neundorf, and Thiago Macieira.

Changes
Put "Crash" keyword in title, to get more attention.
Also set Alex and Thiago explicitely as reviewers.

Want to get this in ASAP, before 4.10.4

Summary (updated)
Crash fix: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

Description
Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple times into the same process (with Bsymbolic-functions flag)" on kde-core-devel ( <a href="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2" title="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2">http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2</a> ) symbols from QtUitools.a are not hidden by default in Qt4 and thus will be added to the public symbols of the module/shared lib they are linked to. And thus can appear multiple times in the same process, resulting in symbol clashes and leading to problems at least with the Bsymbolic-functions flag or when being possibly incompatible versions.

Attached patch sees to solve that problem, by adding a macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags depending on the platform/linker used.

Only issue is that instead of some variable I had to use "QtUiTools.a" as I found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case another platform needs another name/prefix here?

Patch is against 4.10 branch, so I hope to get this in 4.10.4

<a href="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY" title="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY">http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_...</a> shows that there are some more places where the symbols need hiding, but I first want feedback on the proposed approach.

Diffs
cmake/modules/FindKDE4Internal.cmake cb63285
cmake/modules/KDE4Macros.cmake 3db4e24
kjsembed/kjsembed/CMakeLists.txt d70f260
kross/modules/CMakeLists.txt d245fd8
kross/qts/CMakeLists.txt d8cb4a5
plasma/CMakeLists.txt 674550d

Diff: <a href="http://git.reviewboard.kde.org/r/110563/diff/" title="http://git.reviewboard.kde.org/r/110563/diff/">http://git.reviewboard.kde.org/r/110563/diff/</a>

Testing

Thanks,

Friedrich W. H. Kossebau

Re: Review Request 110563: Crash fix: hide symbols from static l

By Friedrich W. H.... at 05/22/2013 - 13:45

(Updated May 22, 2013, 6:45 p.m.)

Review request for Build System, kdelibs, Alexander Neundorf, and Thiago Macieira.

Changes
documentation for the macro moved to top of FindKDE4Internal.cmake

Description
Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple times into the same process (with Bsymbolic-functions flag)" on kde-core-devel ( <a href="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2" title="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2">http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2</a> ) symbols from QtUitools.a are not hidden by default in Qt4 and thus will be added to the public symbols of the module/shared lib they are linked to. And thus can appear multiple times in the same process, resulting in symbol clashes and leading to problems at least with the Bsymbolic-functions flag or when being possibly incompatible versions.

Attached patch sees to solve that problem, by adding a macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags depending on the platform/linker used.

Only issue is that instead of some variable I had to use "QtUiTools.a" as I found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case another platform needs another name/prefix here?

Patch is against 4.10 branch, so I hope to get this in 4.10.4

<a href="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY" title="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY">http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_...</a> shows that there are some more places where the symbols need hiding, but I first want feedback on the proposed approach.

Diffs (updated)
cmake/modules/FindKDE4Internal.cmake cb63285
cmake/modules/KDE4Macros.cmake 3db4e24
kjsembed/kjsembed/CMakeLists.txt d70f260
kross/modules/CMakeLists.txt d245fd8
kross/qts/CMakeLists.txt d8cb4a5
plasma/CMakeLists.txt 674550d

Diff: <a href="http://git.reviewboard.kde.org/r/110563/diff/" title="http://git.reviewboard.kde.org/r/110563/diff/">http://git.reviewboard.kde.org/r/110563/diff/</a>

Testing

Thanks,

Friedrich W. H. Kossebau

Re: Review Request 110563: Crash fix: hide symbols from static l

By Friedrich W. H.... at 09/17/2014 - 14:36

(Updated Sept. 17, 2014, 7:36 nachm.)

Status
This change has been discarded.

Review request for Build System, kdelibs, Alexander Neundorf, and Thiago Macieira.

Repository: kdelibs

Description
Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple times into the same process (with Bsymbolic-functions flag)" on kde-core-devel ( <a href="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2" title="http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2">http://lists.kde.org/?t=136829863100001&amp;r=1&amp;w=2</a> ) symbols from QtUitools.a are not hidden by default in Qt4 and thus will be added to the public symbols of the module/shared lib they are linked to. And thus can appear multiple times in the same process, resulting in symbol clashes and leading to problems at least with the Bsymbolic-functions flag or when being possibly incompatible versions.

Attached patch sees to solve that problem, by adding a macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags depending on the platform/linker used.

Only issue is that instead of some variable I had to use "QtUiTools.a" as I found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case another platform needs another name/prefix here?

Patch is against 4.10 branch, so I hope to get this in 4.10.4

<a href="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY" title="http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_LIBRARY">http://lxr.kde.org/search?v=4.10-branch&amp;filestring=&amp;string=QT_QTUITOOLS_...</a> shows that there are some more places where the symbols need hiding, but I first want feedback on the proposed approach.

Diffs
cmake/modules/FindKDE4Internal.cmake cb63285
cmake/modules/KDE4Macros.cmake 3db4e24
kjsembed/kjsembed/CMakeLists.txt d70f260
kross/modules/CMakeLists.txt d245fd8
kross/qts/CMakeLists.txt d8cb4a5
plasma/CMakeLists.txt 674550d

Diff: <a href="https://git.reviewboard.kde.org/r/110563/diff/" title="https://git.reviewboard.kde.org/r/110563/diff/">https://git.reviewboard.kde.org/r/110563/diff/</a>

Testing

Thanks,

Friedrich W. H. Kossebau

Re: Review Request 110563: Crash fix: hide symbols from static l

By Alexander Neundorf at 05/22/2013 - 13:28

The documentation for the macro should be at the top of FindKDE4Internal.cmake, where all the documentation is.

For the technical side: this flag is new to me. Is it the only possible solution ? Thiago ?

- Alexander Neundorf

On May 22, 2013, 11:19 a.m., Friedrich W. H. Kossebau wrote:

Re: Review Request 110563: Crash fix: hide symbols from static l

By =?utf-8?Q?Thoma... at 05/27/2013 - 19:15

It does usually rather not cause the kind of issue where you'd say "oh, yes - that's because of the linker behavior" and the better solution was to upstream version tag symbols - so one gets consistent behavior and also has more fine grained control over protection/acceleration.

This general consideration aside, in the particular incident Thiago already pointed that QtUitools hides symbols in Qt5 (in the mailing ist, iirc) so it's certainly the proper solution until then - i didn't mean to question that aspect.

The only problem would be the limited presence of that linker option (seems GNU ld only and also on certain architectures) - still better than nothing, until and if the next Qt version would backport hiding the symbols as Thiago also ... implied ... ;-)

- Thomas

On May 22, 2013, 6:45 p.m., Friedrich W. H. Kossebau wrote:

Re: Review Request 110563: Crash fix: hide symbols from static l

By Friedrich W. H.... at 05/27/2013 - 18:36

Now the KDE packages for OpenSUSE are said to have been created with that flag already for a while, given Hrvoje Senjan saying "we are using it 4 years
already, this is first known issue it's caused so far" (<a href="https://bugzilla.novell.com/show_bug.cgi?id=819437#c14" title="https://bugzilla.novell.com/show_bug.cgi?id=819437#c14">https://bugzilla.novell.com/show_bug.cgi?id=819437#c14</a>)

<a href="http://qt-project.org/wiki/Performance_Tip_Startup_Time" title="http://qt-project.org/wiki/Performance_Tip_Startup_Time">http://qt-project.org/wiki/Performance_Tip_Startup_Time</a> says: "Use -Bsymbolic-functions for your shared libraries.", more or less, with no big warnings. (Please note your warnings there, to have more downstream rtfm :) )

Surely -Bsymbolic-functions should be used with care.

Still, we have the problem that QtUitools.a exposes all its symbols on Linux & similar. Which means symbol clashes if loaded multiple times in the same process. With and without -Bsymbolic-functions. One could fix QtUitools.a for 4.8.future. Or see to do on our side what can be done, i.e. explicitely hide those symbols when linking the now existing versions of the static lib QtUitools.a into our code, so any of our code does not even try to lookup its symbols in the wrong place.

Which is what the proposed patch does, offer the option to mark an extern static library to be linked without exposing its symbols, on those platform where it is needed.

What other solution would there be?

- Friedrich W. H.

On May 22, 2013, 6:45 p.m., Friedrich W. H. Kossebau wrote:

Re: Review Request 110563: Crash fix: hide symbols from static l

By =?utf-8?Q?Thoma... at 05/22/2013 - 14:03

What it does:
<a href="http://www.technovelty.org/c/what-exactly-does-bsymblic-do.html" title="http://www.technovelty.org/c/what-exactly-does-bsymblic-do.html">http://www.technovelty.org/c/what-exactly-does-bsymblic-do.html</a>

My 2ยข
There're various issues with it and i dare to claim that you more or less want to use it alongside --dynamic-list only.
Alternatively one would use __attribute__((visibility("[hidden|protected]"))) or the "-version-script" switch to protect/accelerate *certain* funtion/calls. (protected visibility is afair gcc only, though)

If "downstream" applies it, i'd frankly tell "downstream" to rtfm and not just push everything claimed to "make it fasta!!!"

This is by no means sth. one should apply "uninformed" since it has impact on the runtime behavior that the developer needs to know about ("whoops, my trick to shadow friend qt_foo() to gain access to protected/private d->bar only works sometiems" - yes, one should not hack, but one should also be aware that this hack can "legally" fail and not wonder why it works here and on distro X but fails on distro Y)

- Thomas

On May 22, 2013, 6:45 p.m., Friedrich W. H. Kossebau wrote: