Last Comment Bug 747033 - Implement dl_iterate_phdr in the android linker
: Implement dl_iterate_phdr in the android linker
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: mozglue (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-19 09:06 PDT by Jeff Muizelaar [:jrmuizel]
Modified: 2012-04-25 07:47 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement dl_iterate_phdr in the custom linker (17.43 KB, patch)
2012-04-23 03:41 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Implement dl_iterate_phdr in the custom linker (16.88 KB, patch)
2012-04-23 08:36 PDT, Mike Hommey [:glandium]
nfroyd: review+
Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2012-04-19 09:06:05 PDT
We need this so that we can discover the libraries that we have loaded for use in the profiler. Currently, we use /proc/maps and this is bad. I believe we actually only need dlpi_addr and dlpi_name for our use case.
Comment 1 Mike Hommey [:glandium] 2012-04-19 09:30:46 PDT
You may want to keep both dlpi_addr and /proc/maps info to correlate (since dl_iterate_phdr won't give you where the regions end, except if we add phdr info, in which case you could parse the PT_LOAD entries)
Comment 2 Benoit Girard (:BenWa) 2012-04-19 10:12:36 PDT
We're running into this problem in both Android and Linux. Is this a problem that's caused by our custom loader?
Comment 3 Mike Hommey [:glandium] 2012-04-19 23:15:00 PDT
(In reply to Benoit Girard (:BenWa) from comment #2)
> We're running into this problem in both Android and Linux. Is this a problem
> that's caused by our custom loader?

If "this problem" is the number of segments in /proc/self/maps for libxul.so, it is due to elfhack.
Comment 4 Benoit Girard (:BenWa) 2012-04-19 23:24:03 PDT
That explains why I'm able to profile my builds but Jeff can't profile is. I have elf hack disabled.

Looks like we should disable elfhack for now and work towards implementing dl_iterate_phr.
Comment 5 Mike Hommey [:glandium] 2012-04-23 03:41:59 PDT
Created attachment 617443 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker
Comment 6 Mike Hommey [:glandium] 2012-04-23 03:52:08 PDT
With the attached patch, you should be able to get the information you need. Please note you'll have to define the function yourself on Android, as the headers don't have a definition.

You however won't need a complete or totally accurate definition, the following should suffice:
#ifndef HAVE_DL_ITERATE_PHDR
struct dl_phdr_info {
  void *dlpi_addr;
  const char *dlpi_name;
};

int dl_iterate_phdr(int (*callback)(struct dl_phdr_info *, size_t, void *), void *data)  __attribute__((weak));
#endif

Also note that the old Android linker, still in use for XUL fennec, won't have dl_iterate_phdr, so you want to make its use conditional to MOZ_OLD_LINKER not being defined (it's not an AC_DEFINE, so you'll need the usual ifdef/DEFINES += construct for that).

You may want to r? or f? me on your patch using dl_iterate_phdr :)
Comment 7 :Ehsan Akhgari 2012-04-23 08:25:59 PDT
Comment on attachment 617443 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker

__wrap_dl_iterate_phdr should abort the loop if the callback returns a non-zero value.

Also, technically you should init dlpi_phdr and dpli_phnum in every iteration of the loop, since a broken callback function might overwrite them, and see the changed values in the following calls.
Comment 8 Mike Hommey [:glandium] 2012-04-23 08:36:03 PDT
Created attachment 617488 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker

Ehsan is right, I missed the part about the callback return value in the dl_iterate_phdr manpage.
Comment 9 :Ehsan Akhgari 2012-04-23 08:37:28 PDT
(In reply to Mike Hommey [:glandium] from comment #8)
> Created attachment 617488 [details] [diff] [review]
> Implement dl_iterate_phdr in the custom linker
> 
> Ehsan is right, I missed the part about the callback return value in the
> dl_iterate_phdr manpage.

I knew to look for it cause I also missed it in my own dl_iterate_phdr implementation.  :-)
Comment 10 (dormant account) 2012-04-23 12:05:14 PDT
Comment on attachment 617488 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker

I think Nathan should displace me on linker stuff
Comment 11 Nathan Froyd [:froydnj] 2012-04-24 06:42:57 PDT
Comment on attachment 617488 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker

Review of attachment 617488 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the change below.

::: mozglue/linker/ElfLoader.h
@@ +38,5 @@
> +  struct dl_phdr_info {
> +    Elf::Addr dlpi_addr;
> +    const char *dlpi_name;
> +    const Elf::Phdr *dlpi_phdr;
> +    uint16_t dlpi_phnum;

Please define Elf::Half in Elfxx.h and use it here for consistency's sake.

Note You need to log in before you can comment on or make changes to this bug.