prelink breaks fdpic support in a bunch of places

[Filippo ARCIDIACONO]

Hi Mike,

> -----Original Message-----
> From: Mike Frysinger [mailto:vapier at gentoo.org]
> Sent: Sunday, November 27, 2011 9:12 AM
> To: uclibc at uclibc.org; filippo.arcidiacono at st.com;
> carmelo.amoroso at st.com
> Subject: prelink breaks fdpic support in a bunch of places
> 
> there seems to be a bunch of changes that the associated changelog does
> not seem to cover.  so i don't know if they were changed on purpose, by
> accident, etc...
> 
> > commit a33796043bdef5345bc00a528c942f91a87af8e9
> > ldso: Add runtime prelink support
> >
> > _dl_load_elf_shared_library(int secure,...) ...
> > -	DL_LOADADDR_TYPE lib_loadaddr;
> > +	DL_LOADADDR_TYPE lib_loadaddr = 0;
> 
> why is this necessary ?  nothing else in this func changed.  you can't
> assign directly to a variable of type DL_LOADADDR_TYPE.

You are right. I missed DL_LOADADDR_TYPE for archs not sh.

> 
> > int _dl_fixup(struct dyn_elf *rpnt, struct r_scope_elem *scope, int
> > now_flag) ...
> > -	elf_machine_relative(tpnt->loadaddr, reloc_addr, relative_count);
> > +	if (tpnt->loadaddr
> > +#ifdef __LDSO_PRELINK_SUPPORT__
> > +		|| (!tpnt->dynamic_info[DT_GNU_PRELINKED_IDX])
> > +#endif
> > +		)
> > +		elf_machine_relative(tpnt->loadaddr, reloc_addr,
> relative_count);
> 
> why is tpn->loadaddr now being checked ?


In general, the loadaddr is an offset relative to the first PT_LOAD segment
See b9766aa08c90b6718d5497d6a6cf9e6f737e5141 commit when I introduced also
the
'mapaddr' field needed to execute prelinked application.
This check must be reviewed taking into account, in case of prelinked
application,
The relative relocation have to be done if this offset is != 0, when the
PT_LOAD
Segments cannot be mapped at p_vaddr.

> 
> > DL_START(unsigned long args)
> > ...
> > -	elf_machine_relative(load_addr, rel_addr, relative_count);
> > +	if (load_addr
> > +#ifdef __LDSO_PRELINK_SUPPORT__
> > +		|| !tpnt->dynamic_info[DT_GNU_PRELINKED_IDX]
> > +#endif
> > +		)
> > +		elf_machine_relative(load_addr, rel_addr, relative_count);
> 
> why is load_addr now being checked ?

As discussed before.

> 
> > commit 94cc6edb78a12655c0602a246fa1cbdc8c6d0ad9
> > ldso: Rework global scope handling and symbol lookup mechanism
> >
> > DL_START(unsigned long args)
> > ...
> > -	DL_BOOT_COMPUTE_DYN(dpnt, got, load_addr);
> > +	DL_BOOT_COMPUTE_DYN(dpnt, got, (DL_LOADADDR_TYPE)header);
> 
> why was that cast added and changed from load_addr to header ?

The load_addr is zero when the dynamic linker self is prelinked,
The header instead is the real the base address of the inperpreter.
For sh arch, to have the dynamic section pointer I have to add 
The link-time address of _DYNAMIC section (first element of the GOT),
'got' pointer, to the effective address interpreter is loaded at.

> 
> > commit 637e2b2440f69e22932edd71bd2f0b1210dc32ea
> > ldso: Add implementation of ld.so standalone execution
> >
> > DL_START(unsigned long args)
> > ...
> > -	_dl_elf_main = (int (*)(int, char **, char **))
> > -		auxvt[AT_ENTRY].a_un.a_val;
> > -	_dl_get_ready_to_run(tpnt, load_addr, auxvt, envp, argv
> > -		DL_GET_READY_TO_RUN_EXTRA_ARGS);
> > +	_dl_elf_main = (int (*)(int, char **, char **))
> > +		_dl_get_ready_to_run(tpnt, (DL_LOADADDR_TYPE) header,
> auxvt, envp,
> > +                        argv, DL_GET_READY_TO_RUN_EXTRA_ARGS);
> 
> why was load_addr changed to header and the cast added ?

Same reason as before. Further in this case the 'header' field is 
Used in _dl_get_ready_to_run as mapaddr of the dynamic linker.

> 
> > commit b9766aa08c90b6718d5497d6a6cf9e6f737e5141
> > ldso: Fix loadaddr and mappaddr when prelink support is enabled.
> >
> > add_ldso(struct elf_resolve *tpnt,...) ...
> > +	tpnt->mapaddr = load_addr;
> 
> mapaddr is of type ElfW(Addr) but load_addr is of type
> DL_LOADADDR_TYPE.  you cannot assign one to the other.

Right.

> 
> > _dl_load_elf_shared_library(int secure, ...)  ...
> >		tryaddr = (piclib == 2 ? 0
> >			: (char *) (ppnt->p_vaddr & PAGE_ALIGN)
> > -				+ (piclib ? libaddr : 0));
> > +				+ (piclib ? libaddr : lib_loadaddr));
> 
> you can't read the value of lib_loadaddr directly, so this change is
> broken.

Ok!
The lib_loadaddr must be reviewed for other archs other than sh.
Here, In case loading prelinked SO, piclib is 0 and it needs to consider 
'lib_loadaddr', this is != 0 if for some reason the PT_LOAD segment can't be
Mapped at p_vaddr field (this is an absolute address if prelinked), when
mapping a PT_LOAD segment.

> there were more in the original patch, but it seems a change since (by
> Bernd?)
> fixed the others.
> 
> > -   /* For a non-PIC library, the addresses are all absolute */
> > -   if (piclib) {
> > +   /*
> > +    * The dynamic_addr must be take into acount lib_loadaddr value,
> to note
> > +    * it is zero when the SO has been mapped to the elf's physical
> addr
> > +    */
> > +   if (lib_loadaddr) {
> >		dynamic_addr = (unsigned long) DL_RELOC_ADDR(lib_loadaddr,
> dynamic_addr);
> >     }
> 
> we have piclib because you can't read lib_loadaddr directly (it's of
> type
> DL_LOADADDR_TYPE), so this is broken
> 
> > -	tpnt->ppnt = (ElfW(Phdr) *) DL_RELOC_ADDR(tpnt->loadaddr, epnt-
> >e_phoff);
> > +	tpnt->ppnt = (ElfW(Phdr) *) DL_RELOC_ADDR(tpnt->mapaddr, epnt-
> >e_phoff);
> 
> we needed this to be the loadaddr, not the mapaddr ... they aren't the
> same
> type, so they aren't directly interchangeable


Definitively it needs to review handling of DL_LOADDADDR_TYPE variables
To extend the prelink support also for fdpic archs.

> -mike
> -mike

Filippo.