[Ltrace-devel] [PATCH v2] xtensa: add xtensa support
Max Filippov
jcmvbkbc at gmail.com
Sat Jan 3 01:36:25 UTC 2015
Hi Petr,
On Fri, Jan 2, 2015 at 4:32 AM, Petr Machata <pmachata at redhat.com> wrote:
>> +int
>> +arch_elf_init(struct ltelf *lte, struct library *lib)
>> +{
>> + Elf_Scn *scn;
>> + GElf_Shdr shdr;
>> + GElf_Addr relplt_addr;
>> + GElf_Xword relplt_size;
>> + GElf_Phdr phdr;
>> + size_t i;
>> +
>> + for (i = 0; gelf_getphdr(lte->elf, i, &phdr) != NULL; ++i) {
>> + if (phdr.p_type == PT_LOAD) {
>> + lib->arch.loadable_sz =
>> + phdr.p_vaddr + phdr.p_memsz - lte->bias;
>
> Was this tested on a prelinked system? It's confusing that you subtract
No, it wasn't tested on a prelinked system, since AFAIK xtensa is not
supported by the prelink.
> bias to get from ELF address to memory address. Normally you would add
> it.
>
> Also, correct me if I'm wrong, but loadable_sz actually points to the
> end of the region, and hence is not a size. Would loadable_end be a
> better name?
The intention was to calculate the size of loadable part of a library, to be
able to tell later, given library base address and a pointer, if it
points inside
the library or outside of it.
>> + if (elf_get_section_type(lte, SHT_DYNAMIC, &scn, &shdr) < 0
>> + || scn == NULL) {
>> + fail:
>> + error(0, 0, "Couldn't get SHT_DYNAMIC: %s",
>> + elf_errmsg(-1));
>> + return -1;
>> + }
>> +
>> + Elf_Data *data = elf_loaddata(scn, &shdr);
>> + if (data == NULL)
>> + goto fail;
>> +
>> + for (i = 0; i < shdr.sh_size / shdr.sh_entsize; ++i) {
>> + GElf_Dyn dyn;
>> +
>> + if (gelf_getdyn(data, i, &dyn) == NULL)
>> + goto fail;
>> +
>> + if (dyn.d_tag == DT_JMPREL) {
>> + relplt_addr = dyn.d_un.d_ptr;
>> + } else if (dyn.d_tag == DT_PLTRELSZ) {
>> + relplt_size = dyn.d_un.d_val;
>> + }
>> + }
>
> Would calling elf_load_dynamic_entry twice be suitable instead of the
> above block?
Ok.
> The two calls would end up looking up the section by type twice. That
> has negative performance implications. I suspect they are negligible,
> but should you care, would you be also willing to contribute a patch
> that implements on-demand caching of SHT_DYNAMIC inside
> elf_load_dynamic_entry?
>
>> + for (i = 1; i < lte->ehdr.e_shnum; ++i) {
>> + Elf_Scn *scn;
>> + GElf_Shdr shdr;
>> +
>> + scn = elf_getscn(lte->elf, i);
>> + if (scn == NULL || gelf_getshdr(scn, &shdr) == NULL) {
>> + fprintf(stderr, "Couldn't get section header: %s\n",
>> + elf_errmsg(-1));
>> + exit(EXIT_FAILURE);
>> + }
>> + if (shdr.sh_addr == relplt_addr
>> + && shdr.sh_size == relplt_size) {
>
> To get the section pointed to by DT_JMPREL, you could call
> elf_get_section_covering and then check the size in the returned shdr.
> My thinking is that if there is more than one section covering a given
> address, the ELF is probably not structurally sound, and you can safely
> bail out anyway.
Ok.
> Other than these things, the code looks solid as far as I can tell (not
> that I know anything about xtensa though ;) ).
Thanks for the review. I'll post an updated version.
--
Thanks.
-- Max
More information about the Ltrace-devel
mailing list