Closed
Bug 854517
Opened 11 years ago
Closed 11 years ago
Integrate valgrind into B2G builds
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: qdot, Assigned: qdot)
References
Details
Attachments
(9 files, 6 obsolete files)
2.00 KB,
text/plain
|
Details | |
1016 bytes,
patch
|
Details | Diff | Splinter Review | |
943 bytes,
patch
|
Details | Diff | Splinter Review | |
3.51 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
4.59 KB,
patch
|
jseward
:
review+
|
Details | Diff | Splinter Review |
30.34 KB,
patch
|
jseward
:
review+
|
Details | Diff | Splinter Review |
838 bytes,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
6.11 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
442 bytes,
patch
|
Details | Diff | Splinter Review |
Take ben's instructions, and make the build system do it. This includes getting the kernel with expanded memory, integrating valgrind building into android, putting it all on the phone, etc...
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Ok. Built off the head of a valgrind git I found on github that I'll probably fork over to mozilla-b2g. Using ndk and platform directories from inside our build, got a version of valgrind onto the phone that seems to run fine, though I have yet to actually try to check b2g with it, just tried a couple of different things on the phone. I figure we'll maintain a fork of valgrind that we'll just rebase our patches on top of for the time being, and that will build assuming engineering build flags are on?
Assignee | ||
Comment 4•11 years ago
|
||
Found that android is actually running their own android fork of valgrind now: https://android.googlesource.com/platform/external/valgrind/ After talking to jseward some, it sounds like our best bet is to make our own fork following his changes, so we aren't waiting for whatever may or may not come into android's valgrind repo. He's looking at integrating some of the patches from both bent and the android repo over the next few weeks, and until then we can just run a branch with those patches sitting on top while we wait for integration. Our own tracking repo will be at http://www.github.com/mozilla-b2g/valgrind
Comment 5•11 years ago
|
||
It looks like the github.com branch is missing libvex. I pulled the svn repo instead and that built fine.
Assignee | ||
Comment 6•11 years ago
|
||
It's actually a completely different repo. there's mozilla-b2g/valgrind and mozilla-b2g/vex.
Assignee | ||
Comment 7•11 years ago
|
||
The instructions in the android readme for valgrind assume you're using an NDK. As FxOS is a special snowflake of a build tree, we scatter the needed directories all over the place. Drop this script in your valgrind checkout to set it up for hand building in B2G. This will all be automated by the time this lands, this is just a stopgap.
Comment 8•11 years ago
|
||
Comment on attachment 729283 [details]
bent's instructions for phone valgrind
With |export DISABLE_JEMALLOC=1| in my .userconfig, I get:
configure: error: --enable-replace-malloc requires --enable-jemalloc
Removing it allows the build to proceed.
Comment 9•11 years ago
|
||
Comment on attachment 729283 [details]
bent's instructions for phone valgrind
Another problem I'm running into: it looks like |adb push $GECKO_OBJDIR/toolkit/library/libxul.so /sdcard| never finishes. I sniffed the adb connection with Wireshark and see ~150MB get transferred (this is to the emulator, so it's pretty quick) and then the connection stalls (Wireshark says the emulator-side's receive window is closed). I've left adb-push running for hours and no progress.
Any sense of how long this should take?
Comment 10•11 years ago
|
||
Comment on attachment 729283 [details]
bent's instructions for phone valgrind
I've been examining this with various sizes of files, and have found that I can reliably send a 50MiB file, but not a 100MiB file.
Also, in |adb push $GECKO_OBJDIR/toolkit/library/libxul.so /sdcard|, the destination needs to be /sdcard/, or you'll get (e.g.) "failed to copy '50MB.file' to '/sdcard': Is a directory".
Comment 11•11 years ago
|
||
I just realized the emulator only creates an 512MiB 'sdcard' by default, which of course isn't big enough to hold the ~750MiB libxul.so; I modified run-emulator.sh to create an 8GiB sdcard image instead, but still only see ~100-150MiB of libxul.so get transferred before the TCP receive window crashes.
Comment 12•11 years ago
|
||
It looks like adb-push to the emulator is randomly broken. A better approach may be to drop the desired libxul.so into 'out/target/product/generic/data' and then run |$B2G/bld.sh userdataimage| to include the file in the /data partition. The /data partition will grow from the default of 512MiB to accommodate the larger file, but without any free space, which will prevent b2g from starting. To make more space, you need to increase --partition-size in $B2G/run-emulator.sh from 512 to (e.g.) 1024. (This will also increase the size of the /system partition as well.)
Comment 13•11 years ago
|
||
Comment on attachment 729283 [details]
bent's instructions for phone valgrind
When modifying /system/bin/b2g.sh, |vdc volume mount sdcard| must appear _before_ any LD_PRELOAD statements, or the command will segfault and Valgrind won't be able to write to /sdcard.
Assignee | ||
Comment 14•11 years ago
|
||
Need to add valgrind includes to configure.in for base and js
Assignee | ||
Comment 15•11 years ago
|
||
Adds valgrind to image if environment variable is set
Attachment #748256 -
Flags: review?(mwu)
Assignee | ||
Comment 16•11 years ago
|
||
Adding valgrind to b2g.mk, adding build flags for gecko if B2G_VALGRIND environment variable is defined
Attachment #748257 -
Flags: review?(mwu)
Assignee | ||
Comment 17•11 years ago
|
||
Assignee | ||
Comment 18•11 years ago
|
||
All I'm doing in this patch is adding a pregenerated libvex_guest_offsets.h file for inclusion. However, I want to make sure that's an OK thing to do via Julian. Google's android forks of valgrind do this, since there's no configure to be run per platform.
Attachment #749008 -
Flags: review?(jseward)
Assignee | ||
Comment 19•11 years ago
|
||
Assignee | ||
Comment 20•11 years ago
|
||
Valgrind patches for b2g tree integration. Really just involves a symlink to vex (since I'm not sure repo deals with repos-in-repos well), the patches julian already made for bent, the android.mk (modified from the android project's tree), and a pregenerated config.h. Once again, mainly looking for verification that nothing is wildly wrong, it does seem to build and run ok using these flags.
Attachment #749009 -
Attachment is obsolete: true
Attachment #749012 -
Flags: review?(jseward)
Comment 21•11 years ago
|
||
Comment on attachment 749008 [details] [diff] [review] Patch 5 (v1) - VEX changes for B2G valgrind branch Review of attachment 749008 [details] [diff] [review]: ----------------------------------------------------------------- That seems OK. libvex_guest_offsets.h contains offsets of some fields of the struct VexGuestARMState defined in libvex_guest_arm.h, so that V's JIT can generate code that references that struct. Hence libvex_guest_offsets.h will need to be updated when the struct changes, but it basically hardly ever changes (< once per year). In any case I think that V will fail in a very obvious way if the two get out of sync.
Attachment #749008 -
Flags: review?(jseward) → review+
Comment 22•11 years ago
|
||
Comment on attachment 749012 [details] [diff] [review] Patch 6 (v1) - Valgrind patches for B2G valgrind branch Review of attachment 749012 [details] [diff] [review]: ----------------------------------------------------------------- Seems fine.
Attachment #749012 -
Flags: review?(jseward) → review+
Comment 23•11 years ago
|
||
Comment on attachment 748256 [details] [diff] [review] Patch 2 (v1) - Changes to platform-build for valgrind We don't use this copy of b2g.mk anymore (unless you want to uplift to v1-train)
Attachment #748256 -
Flags: review?(mwu)
Comment 24•11 years ago
|
||
Comment on attachment 748257 [details] [diff] [review] Patch 3 (v1) - Valgrind changes for gonk-misc repo Review of attachment 748257 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g.mk @@ +15,5 @@ > sources.xml \ > $(NULL) > > +ifneq ($(B2G_VALGRIND),) > +PRODUCT_PACKAGES += \ Can we get this into a list maintained in the valgrind fork? Then we can just include external/valgrind/valgrind.mk or whatever here.
Updated•11 years ago
|
Attachment #748257 -
Flags: review?(mwu)
Assignee | ||
Comment 25•11 years ago
|
||
Changed list of needed packages to be included from repo instead of having verbose listing in file.
Attachment #750091 -
Flags: review?(mwu)
Assignee | ||
Updated•11 years ago
|
Attachment #748257 -
Attachment is obsolete: true
Assignee | ||
Comment 26•11 years ago
|
||
Comment on attachment 748256 [details] [diff] [review] Patch 2 (v1) - Changes to platform-build for valgrind Obsoleting old build patch for now. Will open new bug for uplifting valgrind to v1-train if needed.
Attachment #748256 -
Attachment is obsolete: true
Assignee | ||
Comment 27•11 years ago
|
||
Backed out a bit too far on the last patch, was missing default-gecko-config changes.
Attachment #750091 -
Attachment is obsolete: true
Attachment #750091 -
Flags: review?(mwu)
Attachment #750095 -
Flags: review?(mwu)
Updated•11 years ago
|
Attachment #750095 -
Flags: review?(mwu) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #748250 -
Flags: review?(khuey)
Attachment #748250 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 28•11 years ago
|
||
Just planning on tracking the fxos branch heads for the moment.
Attachment #751779 -
Flags: review?(mwu)
Assignee | ||
Updated•11 years ago
|
Attachment #751779 -
Attachment description: Patch 4 (v1) - B2G Manifests Update → Patch 4 (v2) - B2G Manifests Update
Assignee | ||
Updated•11 years ago
|
Attachment #748950 -
Attachment is obsolete: true
Comment 29•11 years ago
|
||
(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #28) > Created attachment 751779 [details] [diff] [review] > Patch 4 (v2) - B2G Manifests Update > > Just planning on tracking the fxos branch heads for the moment. It looks like this is against the v1-train manifests. We probably want the master manifests, unless you're planning to uplift the gonk-misc changes.
Assignee | ||
Comment 30•11 years ago
|
||
Patched against b2g-manifest repo master branch instead of v1-train
Attachment #751779 -
Attachment is obsolete: true
Attachment #751779 -
Flags: review?(mwu)
Attachment #751799 -
Flags: review?(mwu)
Comment 31•11 years ago
|
||
Comment on attachment 751799 [details] [diff] [review] Patch 4 (v3) - B2G Manifests Update Looks ok to me. Just be sure to watch TBPL/#developers in case anything goes horribly wrong when this lands.
Attachment #751799 -
Flags: review?(mwu) → review+
Assignee | ||
Comment 32•11 years ago
|
||
Commits for Patch 4 - https://github.com/mozilla-b2g/b2g-manifest/commit/e819e3792dfe2b2f434261b2e47e2279830de45f https://github.com/mozilla-b2g/b2g-manifest/commit/e16fe0ebb94ef527bea4876b4ef994fb4c577d8a
Assignee | ||
Comment 33•11 years ago
|
||
This is actually a component of Patch 3, but it goes in a different repo.
Assignee | ||
Comment 34•11 years ago
|
||
Trying patch 4 commit again: https://github.com/mozilla-b2g/b2g-manifest/commit/09429853a96a23b8c7f8c8fee74b4500a34d2e49
Assignee | ||
Comment 35•11 years ago
|
||
Patch 4 revert /again/ due to symlink not working. Need to change include path to deal with the ../../VEX hardlink in some parts of the gdbserver code in valgrind.
Assignee | ||
Comment 36•11 years ago
|
||
Patch 1 to birch: https://hg.mozilla.org/projects/birch/rev/e38cc9c56e9a
Assignee | ||
Comment 37•11 years ago
|
||
Patch 4 landed cleanly, finally: https://github.com/mozilla-b2g/b2g-manifest/commit/77ffcb4ac6d84e16baed265e0163dad318e36c1f
Comment 38•11 years ago
|
||
Comment on attachment 749012 [details] [diff] [review] Patch 6 (v1) - Valgrind patches for B2G valgrind branch Review of attachment 749012 [details] [diff] [review]: ----------------------------------------------------------------- ::: Android.mk @@ +14,5 @@ > + > +LOCAL_PATH:= $(call my-dir) > + > +ifeq ($(TARGET_DEVICE), generic) > + ANDROID_HARDWARE := ANDROID_HARDWARE_emulator It can also be 'generic_x86' for emulator-x86 and 'generic_mips' for emualtor-mips. @@ +59,5 @@ > + > +# Build libvex-($TARGET_ARCH)-linux.a > +include $(CLEAR_VARS) > + > +LOCAL_MODULE := libvex-$(TARGET_ARCH)-linux You're building target specific static library. Do we really have to embed $(TARGET_ARCH) in module name? @@ +417,5 @@ > + callgrind/jumps.c \ > + callgrind/main.c \ > + callgrind/sim.c \ > + callgrind/threads.c \ > + cachegrind/cg-arch.c trailing ws.
Comment 39•11 years ago
|
||
Comment on attachment 752859 [details] [diff] [review] Patch 7 (v1) - Package Include File for Valgrind Repo >PRODUCT_PACKAGES += \ > libvex-arm-linux \ Since you have |libvex-$(TARGET_ARCH)-linux| in external/valgrind/Android.mk, you should at least follow here to prevent build errors in other target arch.
Assignee | ||
Comment 40•11 years ago
|
||
gonk-misc commit for patch 3: https://github.com/mozilla-b2g/gonk-misc/commit/5240bf92be218c3a9e785e389903795ecb19893b All patches are landed, emulator-x86 followup happening in bug 875517. SHERIFF: Bug can be closed when Patch 1 (https://hg.mozilla.org/projects/birch/rev/e38cc9c56e9a) goes birch -> m-c
Comment 41•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e38cc9c56e9a https://hg.mozilla.org/mozilla-central/rev/b549f0213812
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•