This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch 2/2] Add add_p_vaddr to dwfl_report_elf


On Tue, 2013-04-30 at 14:38 +0200, Jan Kratochvil wrote:
> jankratochvil/compatversion-baseaddpvaddr
>
> commit 2ecbc7f264d2956e5f93f49dba2b8aba482b989f
> Author: Jan Kratochvil <jan.kratochvil@redhat.com>
> Date:   Tue Apr 30 14:27:16 2013 +0200
> 
>     Add parameter add_p_vaddr to dwfl_report_elf.
>     
>     libdwfl/
>     	* dwfl_report_elf.c (__libdwfl_report_elf): Add parameter add_p_vaddr.
>     	Set it to true for ET_EXEC and ET_CORE.  Provide alternative
>     	setup of START and BIAS if !ADD_P_VADDR.  Set END from BIAS, not BASE.
>     	(dwfl_report_elf): Add parameter add_p_vaddr.  Pass it down.  Add
>     	NEW_VERSION.
>     	(_compat_without_add_p_vaddr_dwfl_report_elf) <SHARED>: New, with
>     	COMPAT_VERSION.
>     	* libdwfl.h (dwfl_report_elf): Add parameter add_p_vaddr.  Describe it.
>     	* libdwflP.h (__libdwfl_report_elf): Add parameter add_p_vaddr.
>     	* link_map.c (report_r_debug): Use true add_p_vaddr for dwfl_report_elf.
>     	* linux-kernel-modules.c (report_kernel): Use false add_p_vaddr for
>     	dwfl_report_elf.
>     	* offline.c (process_elf): Use true add_p_vaddr for dwfl_report_elf.
>     
>     tests/
>     	* dwfl-report-elf-align.c: Use false add_p_vaddr for dwfl_report_elf.
>     
>     Signed-off-by: Jan Kratochvil <jan.kratochvil@redhat.com>

This looks good to me. One tiny comment on the new documentation:

> +   ADD_P_VADDR has meaning only for ET_DYN files.  With ADD_P_VADDR true BASE
> +   then specifies in-memory address with subtracted on-disk file prelinked base
> +   address (such as the GNU C library l_addr field).  With ADD_P_VADDR false
> +   BASE is in-memory absolute address start for any relocatable ELF type.  */
>  extern Dwfl_Module *dwfl_report_elf (Dwfl *dwfl, const char *name,
>  				     const char *file_name, int fd,
> -				     GElf_Addr base);
> +				     GElf_Addr base, bool add_p_vaddr);

I would reword it as follows:

"For ET_EXEC and ET_CORE files BASE and ADD_P_VADDR will be ignored.
ADD_P_VADDR is only used for ET_DYN files.  When ADD_P_VADDR is true the
module start address is calculated by taking the ELF phdr p_vaddr into
account (which might have been set when the shared library was
prelinked).  When ADD_P_VADDR is false the module start address will be
equal to BASE for any relocatable ELF type."

But that is mostly because that seems to be what we actually do in the
code. The reference to l_addr might be confusing since the comments in
link.h were wrong till very recently (when you fixed them).

Feel free to ignore me though if you think your description is more
accurate.

Thanks,

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]