diff mbox

mips: ldso: dlopen with flag RTLD_NOW should look up the symbols

Message ID CALuKrvCndtaKT72H-wZ085qoPFbqgnYYn_9xVCaKJPQx+FU1Ug@mail.gmail.com
State Superseded
Headers show

Commit Message

xiaoyur347@gmail.com March 16, 2015, 7:36 a.m. UTC
After reading eglibc code, I find it's much similar to uclibc
ldso/ldso/mips/elfinterp.c:_dl_perform_mips_global_got_relocations.
See "
http://www.eglibc.org/svn/branches/eglibc-2_18/libc/ports/sysdeps/mips/dl-machine.h"
elf_machine_got_rel
function for more inspect.
I slightly modify my last patch to speed the fixup function.
Last patch: Go through the whole symtab.
This patch: Go through symtab[tpnt->dynamic_info[DT_MIPS_GOTSYM_IDX], the
last item of symtab]. Actually this is the .got section. And .got section
is subset of symtab.



 [PATCH] MIPS: Scan the symtab for the dependency of the library to
 avoid runtime empty function pointer. The dependency =
 SHN_UNDEF && STB_GLOBAL && (STT_FUNC || STT_OBJECT)


Signed-off-by: Jean Lee <xiaoyur347@gmail.com>
---
 ldso/ldso/mips/elfinterp.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

  rpnt = (ELF_RELOC *) rel_addr;
@@ -170,6 +171,33 @@ int _dl_parse_relocation_information(struct dyn_elf
*xpnt,
  strtab = (char *) tpnt->dynamic_info[DT_STRTAB];
  got = (unsigned long *) tpnt->dynamic_info[DT_PLTGOT];

+ i = tpnt->dynamic_info[DT_MIPS_SYMTABNO_IDX] -
+ tpnt->dynamic_info[DT_MIPS_GOTSYM_IDX];
+ psym = symtab + tpnt->dynamic_info[DT_MIPS_GOTSYM_IDX];
+ for (; i != 0; --i, ++psym) {
+ if (psym->st_name == 0) {
+ continue;
+ }
+
+ if (psym->st_shndx != SHN_UNDEF
+ || ELF_ST_BIND(psym->st_info) != STB_GLOBAL
+ || (ELF_ST_TYPE(psym->st_info) != STT_FUNC
+ && ELF_ST_TYPE(psym->st_info) != STT_OBJECT)) {
+ continue;
+ }
+ symname = strtab + psym->st_name;
+
+ sym_ref.tpnt = NULL;
+ sym_ref.sym = psym;
+ symbol_addr = (unsigned long)_dl_find_hash(symname,
+ scope,
+ tpnt,
+ ELF_RTYPE_CLASS_PLT, &sym_ref);
+ if (symbol_addr == 0) {
+ _dl_dprintf (2, "%s: undefined symbol: %s\n",tpnt->libname, symname);
+ return 1;
+ }
+ }

  for (i = 0; i < rel_size; i++, rpnt++) {
  reloc_addr = (unsigned long *) (tpnt->loadaddr +

Comments

Andrew Bennett March 21, 2015, 9:40 a.m. UTC | #1
>  struct symbol_ref sym_ref;
> + ElfW(Sym) *psym = NULL;
>  /* Now parse the relocation information */
> rel_size = rel_size / sizeof(ElfW(Rel));
>  rpnt = (ELF_RELOC *) rel_addr;
> @@ -170,6 +171,33 @@ int _dl_parse_relocation_information(struct dyn_elf *xpnt,
> strtab = (char *) tpnt->dynamic_info[DT_STRTAB];
>  got = (unsigned long *) tpnt->dynamic_info[DT_PLTGOT];
>
> + i = tpnt->dynamic_info[DT_MIPS_SYMTABNO_IDX] -
> + tpnt->dynamic_info[DT_MIPS_GOTSYM_IDX];
> + psym = symtab + tpnt->dynamic_info[DT_MIPS_GOTSYM_IDX];
> + for (; i != 0; --i, ++psym) {
> + if (psym->st_name == 0) {
> + continue;
> + }
> +
> + if (psym->st_shndx != SHN_UNDEF
> + || ELF_ST_BIND(psym->st_info) != STB_GLOBAL
> + || (ELF_ST_TYPE(psym->st_info) != STT_FUNC
> + && ELF_ST_TYPE(psym->st_info) != STT_OBJECT)) {
> + continue;
> + }
> + symname = strtab + psym->st_name;
> + 
> + sym_ref.tpnt = NULL;
> + sym_ref.sym = psym;
> + symbol_addr = (unsigned long)_dl_find_hash(symname,
> + scope,
> + tpnt,
> + ELF_RTYPE_CLASS_PLT, &sym_ref);
> + if (symbol_addr == 0) {
> + _dl_dprintf (2, "%s: undefined symbol: %s\n",tpnt->libname, symname);
> + return 1;
> + }
> + }
> 
> for (i = 0; i < rel_size; i++, rpnt++) {
>  reloc_addr = (unsigned long *) (tpnt->loadaddr +

Just realised I top-posted my last comment.  My comment is now correctly
bottom-posted below.

Many thanks for the patch.  If my reading of your patch is correct it looks as if it 
will not respect lazy function binding, as it always resolves the function entries.  You 
will therefore need to predicate this to happen only if RTLD_NOW is used.  

Having saying this I am not sure if the fix is in the correct place.  The issue here
is about how the dynamic loader loads different types of ELF files and then deals with 
resolving the ELF's symbols.  I would therefore be happier if the fix was made to the 
core dynamic loader code to handle this case.

Finally, it would be good if your testcase could be added to the tests in tests/dlopen 
so that we can check for this issue in the future.

Many thanks,


Andrew
diff mbox

Patch

diff --git a/ldso/ldso/mips/elfinterp.c b/ldso/ldso/mips/elfinterp.c
index dfe37c5..a83da8d 100644
--- a/ldso/ldso/mips/elfinterp.c
+++ b/ldso/ldso/mips/elfinterp.c
@@ -162,6 +162,7 @@  int _dl_parse_relocation_information(struct dyn_elf
*xpnt,
 #endif

  struct symbol_ref sym_ref;
+ ElfW(Sym) *psym = NULL;
  /* Now parse the relocation information */
  rel_size = rel_size / sizeof(ElfW(Rel));