-
Notifications
You must be signed in to change notification settings - Fork 234
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
Unused? networking libc symbols in Android libmegazord.so #861
Comments
➤ Ryan Kelly commented:
This is a goal of ours as well, so thanks for digging in to the details here!
In principle yes, I'm quite interested in landing something that can achieve this. I believe we've had some issues with lto=fat in the past, ref mozilla/application-services#2535, mozilla/application-services#2531. I don't recall the details, and it may be that the only remaining concern was compile time. But still, we'd need to do some testing to evaluate. Limiting to rust 1.44 seems undesirable though :-( From the build-system patch you linked, do I understand correctly that you're building your own NSS that is linked by application-services, rather than using the NSS-building stuff we have in ./libs in this repo? One alternative I could suggest would be to hack up our NSS build so that it doesn't include these symbols in the first place, but maybe that won't help you given your build setup? I also wonder whether you could solve this problem by forcing appservices to link to NSS dynamically. We go to some trouble to link statically in our default build (search for NSS_STATIC in this repo for the details) since we can't guarantee that consumers will ship their own copy of NSS, but for your purposes it might make sense to disable this and link dynamically against the nss that I assume you're building for Gecko. |
➤ Alex Catarineu commented:
We don't use the the scripts from libs/ directly, but we follow them closely. So, we can hack up our own NSS build and then try to make a PR here and see if this is acceptable for mozilla.
This is something we can try, thanks. We'll spend some time on this and try to find something that is acceptable for Mozilla, and make a PR. It seems the cross-lto solution will not work long-term, since sooner or later there will always be discrepancies between the clang version used by rustc and the one in the android toolchain. |
While working in the new Android version of Tor Browser based on Fenix we took a look at the built
libmegazord.so
and were expecting them not to contain any known libc networking symbol (e.g. from the list in https://searchfox.org/mozilla-central/rev/30e70f2fe80c97bfbfcd975e68538cefd7f58b2a/python/mozbuild/mozbuild/action/check_binary.py#217), since in Android all networking is done by thehttp
client passed viaRustHttpConfig.setClient
at run time. But actually they did contain a few networking symbols, e.g. checking with {{readelf --syms }}:I thought it would be nice to have some assurance that no rust code is calling networking functions directly and try to make sure that the built libraries do not include these (in principle unused) symbols. These seemed to be coming from NSS
libnspr4.so
, so I tried to build with cross-language LTO and indeed these were optimized out. This is a commit for our build system for that: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/merge_requests/71/diffs?commit_id=639b4c92a67db95f80b4b9fa5d2be38d54e1d588.For this to work I had to:
lto=fat
inCargo.toml
, otherwise in some situations I ran into Compiling to Shared Library doesn't always expose reexported symbols rust-lang/rust#50007 and besides the networking symbols were not optimized out.RUSTFLAGS="-Clinker-plugin-lto -Clink-arg=-fuse-ld=lld -Clink-arg=-Wl,-plugin-opt=O2"
Os, so needed to explicitly set it toO2 here.NSS
withCFLAGS="-flto"
andLDFLAGS="-flto"
I tested with rust 1.43 and application-services 61.0.13. Unfortunately, in newer versions using rust 1.45+ this is not working, I assume because it has moved to LLVM10 while the latest android toolchain is still in 9.
Is this (removing unused symbols from built libraries, in particular networking ones) something that you would be interested in achieving? In that case, I assume that a patch that builds with rust 1.44 (for LLVM 9) and
lto=fat
may be difficult to accept. Do you have any other ideas to achieve this symbol removal without cross-language LTO, or some way to make cross-language LTO work in a way that's acceptable for you?The text was updated successfully, but these errors were encountered: