Closed Bug 747033 Opened 8 years ago Closed 8 years ago

Implement dl_iterate_phdr in the android linker

Categories

(Core :: mozglue, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jrmuizel, Assigned: glandium)

Details

Attachments

(1 file, 1 obsolete file)

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.
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)
We're running into this problem in both Android and Linux. Is this a problem that's caused by our custom loader?
OS: Mac OS X → All
Hardware: x86 → All
(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.
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.
Attachment #617443 - Flags: review?(taras.mozilla)
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 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.
Ehsan is right, I missed the part about the callback return value in the dl_iterate_phdr manpage.
Attachment #617488 - Flags: review?(taras.mozilla)
Attachment #617443 - Attachment is obsolete: true
Attachment #617443 - Flags: review?(taras.mozilla)
(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 on attachment 617488 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker

I think Nathan should displace me on linker stuff
Attachment #617488 - Flags: review?(taras.mozilla) → review?(nfroyd)
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.
Attachment #617488 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/bb9548c09c19
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.