[RNP] librnp.so contains symbols newer than supported libstdc++
Categories
(Thunderbird :: Build Config, defect, P1)
Tracking
(thunderbird78 fixed)
Tracking | Status | |
---|---|---|
thunderbird78 | --- | fixed |
People
(Reporter: rjl, Assigned: rjl)
References
Details
Attachments
(3 files, 4 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta+
|
Details |
Bug 1633778 revealed that the initial Nightly builds of librnp.so on Linux contains symbols from a version of libstdc++ more recent than intended. For compatibility across as many Linux distributions as possible, the goal is to keep the minimum required version at 4.8.1, as that's what Botan requires.
Anything newer than GLIBCXX_3.4.19 is too new.
_ZNSt6chrono3_V212system_clock3nowEv (GLIBCXX_3.4.19)
_ZTINSt6thread6_StateE (GLIBCXX_3.4.22)
_ZNSt3_V216generic_categoryEv (GLIBCXX_3.4.21)
_ZNSt28__atomic_futex_unsigned_base19_M_futex_notify_allEPj (GLIBCXX_3.4.21)
_ZNSt28__atomic_futex_unsigned_base19_M_futex_wait_untilEPjjbNSt6chrono8durationIlSt5ratioILl1ELl1EEEENS2_IlS3_ILl1ELl1000000000EEEE (GLIBCXX_3.4.21)
Assignee | ||
Comment 1•5 years ago
|
||
I thought I had this, but my test build did not work.
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
Assignee | ||
Comment 4•5 years ago
|
||
Comment 5•5 years ago
|
||
We need to check if we require support for multithreaded operation.
Inside the RNP codebase, the terms "thread" or "mutex" aren't used. Apparently RNP doesn't use threads by itself.
JavaScript allows worker threads. Currently, we don't use that with OpenPGP. However, in theory, it might be useful to perform costly operations, e.g. key generation, in a worker thread. But before we could do that, we'd have to check with the RNP library developers if that's a safe to do.
If we consider to use RNP with multiple threads, then the RNP implementation would have to rely on the Botan library to protect itself against races, too. With threads disabled, you don't get such protection.
So today, it might be ok to disable threading in Botan, but this requirement might change in the future. I will double check with the RNP developers and will give an update.
Comment 6•5 years ago
|
||
We got a response, RNP doesn't do any locking and doesn't have multithreading support currently.
However, RNP has the concept of a global library context, and when using separate contexts on separate threads, and if the application (Thunderbird) uses appropriate synchronization to avoid using both contexts in parallel, then it might be safe (from the RNP library perspective).
This still doesn't answer the question from Botan's perspective. I don't know yet if the Botan maintains any global state that it requires for its operation, which it would have to synchronize across threads.
The fact that Botan's library provides a configuration option for using threads, it might require the protection mechanism this config option provides (e.g. locks) for a safe operation.
So, I'm still a little nervous about your request. It might be fine, if you only require this as a workaround temporarily. But as soon as we'll start to use a worker thread for key generation in Thunderbird, we'll most likely need Botan to be built with support for threads switched on.
Comment 8•5 years ago
|
||
Rob, IIUC:
- the usual approach to achieve certain minimum runtime requirements, is to use an old build environment, that matches the requirements
- you are unable to use an old build environment, and therefore you're using tricks/hacks to minimize the requirements
- although your tricks worked for some symbols earlier, they are no longer working, so you are searching for alternative solutions
- the alternative solution you're currently suggesting is to disable thread safety in the Botan library.
If my understanding is correct, it might be preferable to avoid using a hacked approach, and avoid having to apply limitations to the runtime safety of the libraries we're using, and accept that we have to build the libraries in a separate environment.
Comment 9•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 10•5 years ago
|
||
I've come up with some other ideas.
A) Build RNP and deps with the system GCC/libstdc++. This is GCC 4.9 with the recent updates to the base build image.
B) Build our own compiler. Lots of work, and probably not practical given time constraints.
C) Include libstdc++.so in the build. This is my least favorite given its size.
D) Statically link libstdc++ when building librnp.so. Surprisingly, after stripping its only an additional 1.4MB and seems to work.
Another option that did not work out
E) Convince Clang to use a different GCC/libstdc++. I tried, could not get this working. It may or may not work at all, and is still rather hackish.
I'm leaning towards option A. Option D seems fine. License though?
Comment 11•5 years ago
|
||
(In reply to Rob Lemley [:rjl] from comment #10)
A) Build RNP and deps with the system GCC/libstdc++. This is GCC 4.9 with the recent updates to the base build image.
And dynamically link with libstdc++ and don't bundle it?
Yes, this seems like the obvious choice. Was there a reason why you initially didn't want to use it? Did you assume you couldn't rely on anything outside the Mozilla build environment to be available?
B) Build our own compiler. Lots of work, and probably not practical given time constraints.
C) Include libstdc++.so in the build. This is my least favorite given its size.
Hmm, on my debian system that file is just 1.5 MB, but also not sure about the licensing terms.
D) Statically link libstdc++ when building librnp.so. Surprisingly, after stripping its only an additional 1.4MB and seems to work.
There is a library exception for the binary code, GCC RUNTIME LIBRARY EXCEPTION. We're probably already making use of it, if code is compiled with GCC, IIUC. However we'd have to be very careful to follow its rules, for example it mentions "Eligible Compilation Processes". Would we have to include the source code of libstdc++? Apparently not, but if we want to go there, we should probably involve a licensing expert to be certain. Better avoid if we can.
Assignee | ||
Comment 12•5 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #11)
(In reply to Rob Lemley [:rjl] from comment #10)
A) Build RNP and deps with the system GCC/libstdc++. This is GCC 4.9 with the recent updates to the base build image.
And dynamically link with libstdc++ and don't bundle it?
Yes, this seems like the obvious choice. Was there a reason why you initially didn't want to use it? Did you assume you couldn't rely on anything outside the Mozilla build environment to be available?
Before this debian 8 update to the base build image, the version of gcc and libstdc++ was too old for Botan. Now that things have been updated a bit, this option becomes available.
B) Build our own compiler. Lots of work, and probably not practical given time constraints.
C) Include libstdc++.so in the build. This is my least favorite given its size.Hmm, on my debian system that file is just 1.5 MB, but also not sure about the licensing terms.
Ahh yes, helps if I look at the final stripped copy doesn't it.
D) Statically link libstdc++ when building librnp.so. Surprisingly, after stripping its only an additional 1.4MB and seems to work.
There is a library exception for the binary code, GCC RUNTIME LIBRARY EXCEPTION. We're probably already making use of it, if code is compiled with GCC, IIUC. However we'd have to be very careful to follow its rules, for example it mentions "Eligible Compilation Processes". Would we have to include the source code of libstdc++? Apparently not, but if we want to go there, we should probably involve a licensing expert to be certain. Better avoid if we can.
Agreed. I saw those old bugs from pre-Firefox days. I'd prefer to not relitigate that whole thing.
Option A it is then unless I find some random broken thing forcing us to go with B.
Assignee | ||
Comment 13•5 years ago
|
||
Comment 14•5 years ago
|
||
Why did you move away from Option A ?
I had never heard of libc++ before. Are we sure it's sufficiently stable?
Comment 15•5 years ago
|
||
Library implementations can have subtle differences.
(e.g. https://stackoverflow.com/questions/40758070/difference-of-behavior-betwen-libstdc-and-libc-operator-on-bitset )
I don't know if Botan is tested with the libc++ library. It might be good to execute the "make check" target for Botan, to ensure our respective configurations are working as expected by the developers.
However, the make check target doesn't seem to be available in my local build, even though it's mentioned in the "On Unix" section on https://github.com/randombit/botan/blob/master/doc/building.rst
While reading that page, I found another very important detail. I had earlier recommended to use --minimized-build
I'm reading that it's important that we include the system_rng module, can you please ensure we build it? Thanks.
Comment 16•5 years ago
|
||
I still had an older Botan release locally. "make check" works with a current release.
Botan supports building with clang; clang is from the llvm project; the libc++ you're suggesting is also from the llvm project; I'm guessing that the clang c++ compiler uses libc++, too. If that's right, it seems ok to use libc++ in general.
Nevertheless, it would still be good to run "make check" as part of our build.
Assignee | ||
Comment 17•5 years ago
|
||
(In reply to Rob Lemley [:rjl] from comment #13)
Attempt to circumvent the whole libstdc++ symbols problem by replacing it
altogether
when building on Taskcluster.
Since libc++ and its companion libc++abi are not as ubiquitous as libstdc++,
they are
statically linked. The libc++ license permits this quite explicitly.
libc++ is from the LLVM project. It's gotten more of a following from the *BSD OS's due to its looser licensing than on Linux, but it's gaining more support recently on Linux. It's used now by Android, Fuschia, and Chromium[1] for the past couple of years. And of course OSX. It's been around since 2012 or thereabouts. GCC 11 is looking to support it as an alternative to libstdc++ as well [2].
The gcc approach was not working out as well as I'd hoped unfortunately, and I still had unexpected symbols showing up. I do have to set up a linux32 build of libc++ yet, but I will set that side for the moment and get 64 bit done first. This does not affect non-Taskcluster builds or Windows. OSX was using libc++ already of course.
The resulting librnp.so has symbols from glibc2.2.5 and libgcc3.0.0 IIRC. I should have this ready for formal review tomorrow as well as some quick test results from a handful of Linux distributions hopefully.
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=724628
[2] https://www.phoronix.com/scan.php?page=news_item&px=GCC-11-libcpp-LLVM-Support
Comment 18•5 years ago
|
||
thanks, sounds good
Assignee | ||
Comment 19•5 years ago
|
||
(In reply to Kai Engert (:KaiE:) from comment #16)
Nevertheless, it would still be good to run "make check" as part of our build.
Agreed. My moonshot is to run the GTests in RNP as part of our existing GTest unittests. That may not be totally feasible due to the older version used by Mozilla. But, we need to run something from upstream for RNP and Botan would not hurt either to ensure that the shared library is viable and some Marionette (?) tests for the javascript. Oh and I have some code floating around to help with reviewing updates to the source code for all of these libraries too.
Assignee | ||
Comment 20•4 years ago
|
||
Here is a working Linux64 patch that uses libc++ as described.
Only Linux64 will work, still dealing with Linux32.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 21•4 years ago
|
||
There is no 32-bit libc++ built as part of the Clang toolchain, this builds
a standalone version using the same source tar file that the rest of the
Clang toolchain is built with.
Assignee | ||
Comment 22•4 years ago
|
||
Circumvents the problems with libstdc++ symbols problem by replacing it with
LLVM's libc++ only when building on Taskcluster. libc++abi is a required
companion library.
Since libc++ is not as ubiquitous as libstdc++ on Linux, it is statically linked.
The libc++ license permits this quite explicitly.
When librnp (and more precisely Botan) is compiled and linked this way, there are
no references to libstdc++ in the resulting librnp.so file.
Depends on D80223
Comment 23•4 years ago
|
||
Thanks for finding this solution. Please uplift to 78 beta when ready.
Comment 24•4 years ago
|
||
Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/a09af0b4f7d2
Build lib32libc++ as a toolchain job. r=darktrojan
https://hg.mozilla.org/comm-central/rev/f55f8af7fe9e
Build librnp on Linux statically linking to LLVM's libc++. r=kaie
Assignee | ||
Comment 25•4 years ago
|
||
Comment on attachment 9157693 [details]
Bug 1634209 - Build librnp on Linux statically linking to LLVM's libc++. r?kaie
[Approval Request Comment]
Request is for these commits:
https://hg.mozilla.org/comm-central/rev/a09af0b4f7d2
https://hg.mozilla.org/comm-central/rev/f55f8af7fe9e
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 26•4 years ago
|
||
Comment on attachment 9157693 [details]
Bug 1634209 - Build librnp on Linux statically linking to LLVM's libc++. r?kaie
Approved for beta
Assignee | ||
Comment 27•4 years ago
|
||
bugherder uplift |
Assignee | ||
Updated•4 years ago
|
Description
•