Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Implement dl_iterate_phdr in the android linker

RESOLVED FIXED in mozilla15

Status

()

Core
mozglue
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jrmuizel, Assigned: glandium)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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
(Assignee)

Comment 3

5 years ago
(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.
(Assignee)

Comment 5

5 years ago
Created attachment 617443 [details] [diff] [review]
Implement dl_iterate_phdr in the custom linker
Attachment #617443 - Flags: review?(taras.mozilla)
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Comment 8

5 years ago
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.
Attachment #617488 - Flags: review?(taras.mozilla)
(Assignee)

Updated

5 years ago
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 10

5 years ago
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+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb9548c09c19
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/bb9548c09c19
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.