Closed Bug 783331 Opened 7 years ago Closed 7 years ago

Add Phdr pointer to dl_iterate_phdr implementation

Categories

(Core :: mozglue, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jrmuizel, Assigned: glandium)

References

Details

Attachments

(2 files)

We need this to get the size of the loaded area.
Assignee: nobody → mh+mozilla
Blocks: 749518
OS: Mac OS X → Android
Hardware: x86 → All
Jeff, Vlad, can either of you check this patch works for you?
Attachment #653674 - Flags: review?(nfroyd)
Comment on attachment 653674 [details] [diff] [review]
Add Phdr pointer and count to dl_iterate_phdr implementation

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

::: mozglue/linker/ElfLoader.cpp
@@ +109,5 @@
> +    // Assuming l_addr points to Elf headers (in most cases, this is true),
> +    // get the Phdr location from there.
> +    uint8_t mapped;
> +    // If the page is not mapped, mincore returns an error.
> +    if (!mincore(it->l_addr, PAGE_SIZE, &mapped)) {

l_addr is guaranteed to be aligned because we mmap'd the binary into memory, and l_addr points to the beginning of the binary, yes?
Attachment #653674 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd (:froydnj) from comment #2)
> Comment on attachment 653674 [details] [diff] [review]
> Add Phdr pointer and count to dl_iterate_phdr implementation
> 
> Review of attachment 653674 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mozglue/linker/ElfLoader.cpp
> @@ +109,5 @@
> > +    // Assuming l_addr points to Elf headers (in most cases, this is true),
> > +    // get the Phdr location from there.
> > +    uint8_t mapped;
> > +    // If the page is not mapped, mincore returns an error.
> > +    if (!mincore(it->l_addr, PAGE_SIZE, &mapped)) {
> 
> l_addr is guaranteed to be aligned because we mmap'd the binary into memory,
> and l_addr points to the beginning of the binary, yes?

l_addr is supposed to be aligned, but if it isn't, then it's still better to bail out, and since mincore will return an error in this case anyways...
We need this change to support symbolication on nightly builds, which I want to start doing daily on eideticker. Are we ready to land this?
(In reply to Benoit Girard (:BenWa) from comment #4)
> We need this change to support symbolication on nightly builds, which I want
> to start doing daily on eideticker. Are we ready to land this?

We should probably test that it works :)
What needs to be tested? I can give it a go tomorrow.
(In reply to Benoit Girard (:BenWa) from comment #6)
> What needs to be tested? I can give it a go tomorrow.

An implementation of shared_library_info that uses dl_iterate_phdr. (I don't think one has been written yet)
Yeah, I wanted the patch to be thoroughly tested before landing.
Attached patch add const_castSplinter Review
Fixed compile error:
/home/bgirard/mozilla/kiwifox/tree/mozglue/linker/ElfLoader.cpp: In function 'int __wrap_dl_iterate_phdr(int (*)(dl_phdr_info*, size_t, void*), void*)':
/home/bgirard/mozilla/kiwifox/tree/mozglue/linker/ElfLoader.cpp:113: error: invalid conversion from 'const void*' to 'void*'
This patch works and I was able to write a patch for bug 749518 using it.
https://hg.mozilla.org/mozilla-central/rev/d405d013746d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.