Closed Bug 1378986 Opened 2 years ago Closed 2 years ago
Make elfhack work again on Android
59 bytes, text/x-review-board-request
59 bytes, text/x-review-board-request
1.46 KB, patch
|Details | Diff | Splinter Review|
In bug 1163171, it was mentioned that elfhack does not build with clang, and I wonder if we still needed elfhack at all in 2017.
Try run with elfhack disabled: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e193f54c7dda0f87ac2a091976f90065bd2a0d7&selectedJob=112276700 Autophone results: http://phonedash.mozilla.org/#/2017-07-05/2017-07-06/binning=repo-phonetype-phoneid-test_name&rejected=norejected&errorbars=noerrorbars&errorbartype=standarderror&valuetype=median&geckoview-e10s-nytimes=on&geckoview-nytimes=on&remote-nytimes=on&throbberstart=on&first=on&second=on&autoland=on&mozilla-beta=on&mozilla-central=on&mozilla-inbound=on&try-jwillcox=on&nexus-6p=on&nexus-6p-10=on&nexus-6p-11=on&nexus-6p-12=on It looks like the startup time is about the same with elfhack or not, and the APK size did not change signficiantly.
Status: NEW → UNCONFIRMED
Ever confirmed: false
06:52 <glandium> regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a812ef63de87&tochange=6955309291ee 06:52 <glandium> 4.5 years ago 06:53 <glandium> it's funny that bug 822584 is in that range 06:53 <firebot> https://bugzil.la/822584 — FIXED, email@example.com — Breakpad can't deal with elfhacked binaries on ARM (B2G crash stacks are broken/corrupted) 06:54 <snorp> glandium: wow 06:55 <glandium> My bet is on bug 825151 06:55 <firebot> https://bugzil.la/825151 — FIXED, firstname.lastname@example.org — Switch ARMv7 builds over to using NDK r8c 06:56 <glandium> so, I fixed something elfhack related on arm, and 15 changesets later, elfhack went kaboom from an upgrade of gcc 06:56 <glandium> nice timing
I got past the compilation failures with relro disabled, and got this during stage-package: [task 2017-07-07T03:07:07.499266Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/liblgpllibs.so: Didn't find relocation for DT_INIT_ARRAY's first entry. Skipping [task 2017-07-07T03:07:07.499443Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libomxplugin.so: No gain. Skipping [task 2017-07-07T03:07:07.499626Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libomxpluginkk.so: No gain. Skipping [task 2017-07-07T03:07:07.499806Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libxul.so: Reduced by 2604904 bytes [task 2017-07-07T03:07:07.500050Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libnssckbi.so: Didn't find relocation for DT_INIT_ARRAY's first entry. Skipping [task 2017-07-07T03:07:07.500238Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libnss3.so: Didn't find relocation for DT_INIT_ARRAY's first entry. Skipping [task 2017-07-07T03:07:07.500422Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libsoftokn3.so: Didn't find relocation for DT_INIT_ARRAY's first entry. Skipping [task 2017-07-07T03:07:07.500604Z] 03:07:07 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libfreebl3.so: Didn't find relocation for DT_INIT_ARRAY's first entry. Skipping [task 2017-07-07T03:07:07.500793Z] 03:07:07 INFO - ../../../dist/geckoview/lib/armeabi-v7a/libmozglue.so: Reduced by 8044 bytes [task 2017-07-07T03:07:07.501001Z] 03:07:07 INFO - elfhack: /home/worker/workspace/build/src/build/unix/elfhack/elf.cpp:264: Elf::Elf(std::ifstream&): Assertion `segment->getFileSize() == phdr.p_filesz - gold_adjustment' failed. The main thing in there is the 2.6MB drop in libxul.so size, although compressed, it's likely not to be as big, but that's still going to be that much less memory used in the processes. The DT_INIT_ARRAY thing doesn't harm but it would be worth checking what's up with it, because there might be wins to have if we can actually elfhack the libraries that are being skipped because of it. And the final one is an actual error, and unfortunately, it doesn't tell on which particular file it's happening, but that's definitely something that will need fixing.
Assignee: nobody → mh+mozilla
Summary: Reconsider elfhack → Make elfhack work again on Android
Here's a push with both patches + norelro. https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e4410d7d3974c5248b092de2b33a12c8dc96b6e How do we see how it performs on autophone?
(Note I'm considering fixing bug 635961, which would make elfhack work without disabling relro)
(In reply to Mike Hommey [:glandium] from comment #7) > Here's a push with both patches + norelro. > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=4e4410d7d3974c5248b092de2b33a12c8dc96b6e Note this is on top of your previous push to try, so it's on top of bug 1374647 and bug 1377580. Anyways, a first point of comparison, on the Android 4.0 API15+ opt builds: on m-c (283debe8155a, which is the base of the try push): Size of libxul.so: 22217892 bytes Size of target.apk: 36659110 bytes on try: Size of libxul.so: 22145516 bytes Size of target.apk: 36582170 bytes So it doesn't make much difference on the apk because libxul.so is xz'ed.
Comment on attachment 8884233 [details] Bug 1378986 - Avoid elfhack failing on weird DT_INIT_ARRAYs. https://reviewboard.mozilla.org/r/155184/#review160230
Attachment #8884233 - Flags: review?(nfroyd) → review+
Here's phonedash for the latest build with elfhack enabled: http://phonedash.mozilla.org/#/2017-07-06/2017-07-07/binning=repo-phonetype-phoneid-test_name&rejected=norejected&errorbars=noerrorbars&errorbartype=standarderror&valuetype=median&geckoview-e10s-nytimes=on&geckoview-nytimes=on&remote-blank=on&throbberstart=on&second=on&mozilla-central=on&try-mh=on&nexus-5=on&nexus-5-1=on&nexus-5-3=on&nexus-5-5=on&nexus-5-6=on It looks like there could be a slight improvement on startup time (throbberstart) on the Nexus 5, but it's also well within the noise range so who knows.
Comment on attachment 8884234 [details] Bug 1378986 - Adjust the fake phdr section properly. https://reviewboard.mozilla.org/r/155186/#review160380
Attachment #8884234 - Flags: review?(nfroyd) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/16ff9fdd6dba Avoid elfhack failing on weird DT_INIT_ARRAYs. r=froydnj https://hg.mozilla.org/integration/autoland/rev/9d9b82a56332 Adjust the fake phdr section properly. r=froydnj
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f12698197df5 Avoid crashing in elfhack when the input file has no relocations. r=me a=bustage
Attachment #8890102 - Flags: review?(nfroyd) → review+
Bug 635961 is confirmed to be the last missing piece: [task 2017-07-26T21:32:29.232371Z] 21:32:29 INFO - ../../../dist/geckoview/assets/armeabi-v7a/libxul.so: Reduced by 2568040 bytes This can now be closed.
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
You need to log in before you can comment on or make changes to this bug.