Closed Bug 1378986 Opened 2 years ago Closed 2 years ago

Make elfhack work again on Android

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: snorp, Assigned: glandium)

References

Details

Attachments

(3 files)

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.
See bug 1163171 comment 22.
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, mh+mozilla@glandium.org — 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, bugmail@mozilla.staktrace.com — 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?
Flags: needinfo?(snorp)
(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+
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+
Keywords: leave-open
Pushed by mh@glandium.org:
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
Depends on: 1379835
Depends on: 635961
Pushed by kwierso@gmail.com:
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.
Keywords: leave-open
Depends on: 1499915
No longer depends on: 1499915
You need to log in before you can comment on or make changes to this bug.