Re: [PATCH] Fix dladdr return value when cannot find symbol
On Fri, Feb 08, 2008 at 10:24:40AM +0100, Carmelo AMOROSO wrote: Bernd Schmidt wrote: Carmelo AMOROSO wrote: based on the patch from Nickolai, here you can find a comprehensive patch to fix dladdr function. With the current implementation, the invocation of dladdr((void *) 1, dlinfo) will fill dlinfo.dli_fname with the name of the application itself and dlinfo.dli_fbase with 0 (the current value of tpnt-loadaddr for the main app). The patch solves this adding a new field into the struct elf_resolve named DL_LOADADDR_TYPE mapaddr. For shared objects, mapaddr has exactly the same value as loadaddr, while for the main app it contains the address of the first PT_LOAD segmented loaded. loadaddr will keep it's value and will used exactly as is (as a in memory offset with respect to relative addresses from ELF sections). While in the DL_ADDR_IN_LOADADDR macro, the new field mapaddr needs to be used. This is broken on FD-PIC, since you treat DL_LOADADDR_TYPE variables as pointers, defeating the point of abstracting away the type. The following patch makes it compile on Blackfin, but I'm unable to test this on a non-FDPIC system. Does it still work for you? Bernd Hi Bernd, the fix is fine for me too.. sorry I did not take care of the specificity of bfin. Please, go ahead and commit it. I'll update the nptl branch too. Ever since this patch was installed, i get: ldso/libdl/libdl.c: In function 'dladdr': ldso/libdl/libdl.c:746: warning: assignment makes pointer from integer without a cast To bypass this alleged bug, i suspect something like the paste below could be appropriate, but i'd prefer if somebody else would have a look and fix this. TIA.. Index: uClibc/ldso/libdl/libdl.c === --- uClibc/ldso/libdl/libdl.c (revision 21490) +++ uClibc/ldso/libdl/libdl.c (working copy) @@ -743,7 +743,7 @@ int dladdr(const void *__address, Dl_inf /* Set the info for the object the address lies in */ __info-dli_fname = pelf-libname; - __info-dli_fbase = pelf-mapaddr; + __info-dli_fbase = (void*)(pelf-mapaddr); symtab = (ElfW(Sym) *) (pelf-dynamic_info[DT_SYMTAB]); strtab = (char *) (pelf-dynamic_info[DT_STRTAB]); ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Bernhard Fischer wrote: On Fri, Feb 08, 2008 at 10:24:40AM +0100, Carmelo AMOROSO wrote: Bernd Schmidt wrote: Carmelo AMOROSO wrote: based on the patch from Nickolai, here you can find a comprehensive patch to fix dladdr function. With the current implementation, the invocation of dladdr((void *) 1, dlinfo) will fill dlinfo.dli_fname with the name of the application itself and dlinfo.dli_fbase with 0 (the current value of tpnt-loadaddr for the main app). The patch solves this adding a new field into the struct elf_resolve named DL_LOADADDR_TYPE mapaddr. For shared objects, mapaddr has exactly the same value as loadaddr, while for the main app it contains the address of the first PT_LOAD segmented loaded. loadaddr will keep it's value and will used exactly as is (as a in memory offset with respect to relative addresses from ELF sections). While in the DL_ADDR_IN_LOADADDR macro, the new field mapaddr needs to be used. This is broken on FD-PIC, since you treat DL_LOADADDR_TYPE variables as pointers, defeating the point of abstracting away the type. The following patch makes it compile on Blackfin, but I'm unable to test this on a non-FDPIC system. Does it still work for you? Bernd Hi Bernd, the fix is fine for me too.. sorry I did not take care of the specificity of bfin. Please, go ahead and commit it. I'll update the nptl branch too. Ever since this patch was installed, i get: ldso/libdl/libdl.c: In function 'dladdr': ldso/libdl/libdl.c:746: warning: assignment makes pointer from integer without a cast To bypass this alleged bug, i suspect something like the paste below could be appropriate, but i'd prefer if somebody else would have a look and fix this. TIA.. Index: uClibc/ldso/libdl/libdl.c === --- uClibc/ldso/libdl/libdl.c (revision 21490) +++ uClibc/ldso/libdl/libdl.c (working copy) @@ -743,7 +743,7 @@ int dladdr(const void *__address, Dl_inf /* Set the info for the object the address lies in */ __info-dli_fname = pelf-libname; - __info-dli_fbase = pelf-mapaddr; + __info-dli_fbase = (void*)(pelf-mapaddr); symtab = (ElfW(Sym) *) (pelf-dynamic_info[DT_SYMTAB]); strtab = (char *) (pelf-dynamic_info[DT_STRTAB]); Fixed as suggested in rev 21493. Thanks, Carmelo ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Carmelo AMOROSO wrote:
based on the patch from Nickolai, here you can find a comprehensive
patch to fix
dladdr function.
With the current implementation, the invocation of dladdr((void *) 1,
dlinfo)
will fill dlinfo.dli_fname with the name of the application itself and
dlinfo.dli_fbase with 0 (the current value of tpnt-loadaddr for the
main app).
The patch solves this adding a new field into the struct elf_resolve
named DL_LOADADDR_TYPE mapaddr.
For shared objects, mapaddr has exactly the same value as loadaddr,
while for the
main app it contains the address of the first PT_LOAD segmented loaded.
loadaddr will keep it's value and will used exactly as is (as a in
memory offset
with respect to relative addresses from ELF sections).
While in the DL_ADDR_IN_LOADADDR macro, the new field mapaddr needs to
be used.
This is broken on FD-PIC, since you treat DL_LOADADDR_TYPE variables as
pointers, defeating the point of abstracting away the type.
The following patch makes it compile on Blackfin, but I'm unable to test
this on a non-FDPIC system. Does it still work for you?
Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
Index: ldso/ldso/ldso.c
===
--- ldso/ldso/ldso.c (revision 20945)
+++ ldso/ldso/ldso.c (working copy)
@@ -135,7 +135,7 @@ void _dl_get_ready_to_run(struct elf_res
char **argv
DL_GET_READY_TO_RUN_EXTRA_PARMS)
{
- DL_LOADADDR_TYPE app_loadaddr = NULL;
+ ElfW(Addr) app_mapaddr = NULL;
ElfW(Phdr) *ppnt;
ElfW(Dyn) *dpnt;
char *lpntstr;
@@ -277,8 +278,8 @@ void _dl_get_ready_to_run(struct elf_res
relro_addr = ppnt-p_vaddr;
relro_size = ppnt-p_memsz;
}
- if (!app_loadaddr (ppnt-p_type == PT_LOAD)) {
- app_loadaddr = ppnt-p_vaddr;
+ if (!app_mapaddr (ppnt-p_type == PT_LOAD)) {
+ app_mapaddr = DL_RELOC_ADDR (app_tpnt-loadaddr, ppnt-p_vaddr);
}
if (ppnt-p_type == PT_DYNAMIC) {
dpnt = (ElfW(Dyn) *) DL_RELOC_ADDR(app_tpnt-loadaddr, ppnt-p_vaddr);
@@ -326,7 +327,7 @@ void _dl_get_ready_to_run(struct elf_res
_dl_symbol_tables = rpnt = (struct dyn_elf *) _dl_malloc(sizeof(struct dyn_elf));
_dl_memset(rpnt, 0, sizeof(struct dyn_elf));
rpnt-dyn = _dl_loaded_modules;
- app_tpnt-mapaddr = app_loadaddr;
+ app_tpnt-mapaddr = app_mapaddr;
app_tpnt-rtld_flags = unlazy | RTLD_GLOBAL;
app_tpnt-usage_count++;
app_tpnt-symbol_scope = _dl_symbol_tables;
Index: ldso/ldso/dl-hash.c
===
--- ldso/ldso/dl-hash.c (revision 20945)
+++ ldso/ldso/dl-hash.c (working copy)
@@ -152,7 +152,8 @@ struct elf_resolve *_dl_add_elf_hash_tab
hash_addr += tpnt-nbucket;
tpnt-chains = hash_addr;
}
- tpnt-loadaddr = tpnt-mapaddr = loadaddr;
+ tpnt-loadaddr = loadaddr;
+ tpnt-mapaddr = DL_RELOC_ADDR(loadaddr, 0);
for (i = 0; i DYNAMIC_SIZE; i++)
tpnt-dynamic_info[i] = dynamic_info[i];
return tpnt;
Index: ldso/ldso/bfin/dl-sysdep.h
===
--- ldso/ldso/bfin/dl-sysdep.h (revision 20945)
+++ ldso/ldso/bfin/dl-sysdep.h (working copy)
@@ -95,7 +95,7 @@ struct funcdesc_ht;
#define DL_LOADADDR_TYPE struct elf32_fdpic_loadaddr
#define DL_RELOC_ADDR(LOADADDR, ADDR) \
- (__reloc_pointer ((void*)(ADDR), (LOADADDR).map))
+((ElfW(Addr))__reloc_pointer ((void*)(ADDR), (LOADADDR).map))
#define DL_ADDR_TO_FUNC_PTR(ADDR, LOADADDR) \
((void(*)(void)) _dl_funcdesc_for ((void*)(ADDR), (LOADADDR).got_value))
@@ -189,7 +189,7 @@ while (0)
#define DL_FIND_HASH_VALUE(TPNT, TYPE_CLASS, SYM) \
(((TYPE_CLASS) ELF_RTYPE_CLASS_DLSYM) \
ELF32_ST_TYPE((SYM)-st_info) == STT_FUNC \
- ? _dl_funcdesc_for (DL_RELOC_ADDR ((TPNT)-loadaddr, (SYM)-st_value),\
+ ? _dl_funcdesc_for ((void *)DL_RELOC_ADDR ((TPNT)-loadaddr, (SYM)-st_value), \
(TPNT)-loadaddr.got_value) \
: DL_RELOC_ADDR ((TPNT)-loadaddr, (SYM)-st_value))
Index: ldso/include/dl-hash.h
===
--- ldso/include/dl-hash.h (revision 20945)
+++ ldso/include/dl-hash.h (working copy)
@@ -34,7 +34,7 @@ struct elf_resolve {
struct elf_resolve * next;
struct elf_resolve * prev;
/* Nothing after this address is used by gdb. */
- DL_LOADADDR_TYPE mapaddr;/* Address at which ELF segments (either main app and DSO) are mapped into */
+ ElfW(Addr) mapaddr;/* Address at which ELF segments (either main app and DSO) are mapped into */
enum {elf_lib, elf_executable,program_interpreter, loaded_file} libtype;
struct dyn_elf * symbol_scope;
unsigned short usage_count;
Index: ldso/libdl/libdl.c
===
--- ldso/libdl/libdl.c
Re: [PATCH] Fix dladdr return value when cannot find symbol
Carmelo AMOROSO wrote: based on the patch from Nickolai, here you can find a comprehensive patch to fix dladdr function. With the current implementation, the invocation of dladdr((void *) 1, dlinfo) will fill dlinfo.dli_fname with the name of the application itself and dlinfo.dli_fbase with 0 (the current value of tpnt-loadaddr for the main app). Could you put a testcase for this into tests/? Bernd -- This footer brought to you by insane German lawmakers. Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Bernd Schmidt wrote: Carmelo AMOROSO wrote: based on the patch from Nickolai, here you can find a comprehensive patch to fix dladdr function. With the current implementation, the invocation of dladdr((void *) 1, dlinfo) will fill dlinfo.dli_fname with the name of the application itself and dlinfo.dli_fbase with 0 (the current value of tpnt-loadaddr for the main app). Could you put a testcase for this into tests/? Bernd Yes, sure. I'll ask my colleague to add it. Carmelo ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
On Jan 18, 2008 2:48 AM, Joakim Tjernlund [EMAIL PROTECTED] wrote: On Fri, 2008-01-18 at 09:08 +0100, Carmelo AMOROSO wrote: Carmelo AMOROSO wrote: Carmelo AMOROSO wrote: Hello, based on the patch from Nickolai, here you can find a comprehensive patch to fix dladdr function. Sorry, I missed a fix into dl-hash.c too. Find the missing change into the attached patch. Carmelo Hi, I'd like to commit the patch to fix dl-addr implementation. Any comments/objections ? otherwise I'll go ahead. Cheers, Carmelo Nope, go ahead Jocke Is this a uClibc-0.9.29.* only problem? -- Kevin Day ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Carmelo AMOROSO wrote: Hello, based on the patch from Nickolai, here you can find a comprehensive patch to fix dladdr function. Sorry, I missed a fix into dl-hash.c too. Find the missing change into the attached patch. Carmelo Index: ldso/ldso/dl-hash.c === --- ldso/ldso/dl-hash.c (revision 20834) +++ ldso/ldso/dl-hash.c (working copy) @@ -152,7 +152,7 @@ hash_addr += tpnt-nbucket; tpnt-chains = hash_addr; } - tpnt-loadaddr = loadaddr; + tpnt-loadaddr = tpnt-mapaddr = loadaddr; for (i = 0; i DYNAMIC_SIZE; i++) tpnt-dynamic_info[i] = dynamic_info[i]; return tpnt; ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
On Wednesday 12 December 2007, Carmelo AMOROSO wrote: while running more tests on this (indeed never used dladdr in the past), which tests ? our dl tests certainly dont have the coverage anywhere close to what i wish they did ... -mike signature.asc Description: This is a digitally signed message part. ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Carmelo AMOROSO wrote:
Nickolai Zeldovich wrote:
On Dec 9, 2007 11:26 AM, Carmelo Amoroso [EMAIL PROTECTED] wrote:
Attached patch tries to fix it. The GNU_HASH part needs to be fixed
accordingly.
Any comments ?
I think that should work. Minor point:
--- ldso/libdl/libdl.c (revision 20633)
+++ ldso/libdl/libdl.c (working copy)
@@ -723,8 +723,11 @@
_dl_if_debug_print(Module \%s\ at %p\n,
tpnt-libname,
DL_LOADADDR_BASE(tpnt-loadaddr));
- if (DL_ADDR_IN_LOADADDR((ElfW(Addr)) __address, tpnt, pelf))
+ if (DL_ADDR_IN_LOADADDR((ElfW(Addr)) __address, tpnt,
pelf)) {
pelf = tpnt;
+ /* Found, stop looping */
+ break;
+ }
}
if (!pelf) {
I don't think this change is necessary or correct. With this change,
the code can exit having found any shared library mapped at any lower
address. Instead, you need to iterate over all loaded ELF files to
find the nearest shared library mapped at a lower address (see the
comment for DL_ADDR_IN_LOADADDR in dl-defs.h).
Nickolai.
Yes, you're right. I'm doing some tests and I'll commit a comprehensive
patch.
Thanks,
Carmelo
Hi,
while running more tests on this (indeed never used dladdr in the past),
we have found that
the default implementation of DL_ADDR_IN_LOADADDR macro is broken for
app_tpnt.
Infact, for *non* PIE executable (like the most of the cases) the
app_tpnt-loadaddr is set to 0
being the difference between entry point from auxv and entry point from
ELF program header.
So, according to the macro definition in dl-defs.h the comparison
tpnt-loadaddr __address will be always true (also passing 0x1 as
__address), and and the end of the loop
it will return the pelf pointing to app_tpnt.
I've seen that bfin and fvr use a own implementation of
DL_ADDR_IN_LOADADDR macro... I'm wondering
if someone using these archs have experienced this kind of problems or not.
We'll go to fix it (and test for sh4), but I'm expecting to have a
common better implementation.
Cheers,
Carmelo
___
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
___
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
___
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Nickolai Zeldovich wrote:
On Dec 9, 2007 11:26 AM, Carmelo Amoroso [EMAIL PROTECTED] wrote:
Attached patch tries to fix it. The GNU_HASH part needs to be fixed
accordingly.
Any comments ?
I think that should work. Minor point:
--- ldso/libdl/libdl.c (revision 20633)
+++ ldso/libdl/libdl.c (working copy)
@@ -723,8 +723,11 @@
_dl_if_debug_print(Module \%s\ at %p\n,
tpnt-libname,
DL_LOADADDR_BASE(tpnt-loadaddr));
- if (DL_ADDR_IN_LOADADDR((ElfW(Addr)) __address, tpnt, pelf))
+ if (DL_ADDR_IN_LOADADDR((ElfW(Addr)) __address, tpnt, pelf))
{
pelf = tpnt;
+ /* Found, stop looping */
+ break;
+ }
}
if (!pelf) {
I don't think this change is necessary or correct. With this change,
the code can exit having found any shared library mapped at any lower
address. Instead, you need to iterate over all loaded ELF files to
find the nearest shared library mapped at a lower address (see the
comment for DL_ADDR_IN_LOADADDR in dl-defs.h).
Nickolai.
Yes, you're right. I'm doing some tests and I'll commit a comprehensive
patch.
Thanks,
Carmelo
___
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
___
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Nickolai Zeldovich wrote:
The attached patch makes dladdr() return 0 when it cannot find a
matching symbol. The current behavior of returning 1 makes it
impossible for callers to know whether the supplied Dl_info buffer has
been filled in with meaningful data.
Nickolai.
Hi Nickolai,
I had a look at your patch.
According to what man page says on dladdr (on my MacOSX, I'll check on Linux
too)
ERRORS
If an image containing addr cannot be found, dladdr() returns 0. On suc-
cess, a non-zero value is returned.
If the image containing addr is found, but no nearest symbol was found,
the dli_sname and dli_saddr fields are set to NULL.
your patch seems not be correct. Indeed dladdr should be fixed in a better
way by setting dli_sname and dli_saddr to NULL whn symbol hasn't been found,
and setting always dli_fname and dli_fbase (if there is an object in which
the addr lies in).
This behavior is the same as glibc.
Attached patch tries to fix it. The GNU_HASH part needs to be fixed accordingly.
Anyway, thanks for pointing it out
Any comments ?
Cheers,
Carmelo
Fix dladdr by filling with NULL dlinfo entries related to
symbol name and symbol address. If the object in which the address
lies in has been found, then the dinfo fields dli_fname and dli_fbase
have to be filled indipendently of the symbol matching.
Signed-off-by: Carmelo Amoroso [EMAIL PROTECTED]
Index: ldso/libdl/libdl.c
===
--- ldso/libdl/libdl.c (revision 20633)
+++ ldso/libdl/libdl.c (working copy)
@@ -723,8 +723,11 @@
_dl_if_debug_print(Module \%s\ at %p\n,
tpnt-libname,
DL_LOADADDR_BASE(tpnt-loadaddr));
- if (DL_ADDR_IN_LOADADDR((ElfW(Addr)) __address, tpnt, pelf))
+ if (DL_ADDR_IN_LOADADDR((ElfW(Addr)) __address, tpnt, pelf)) {
pelf = tpnt;
+ /* Found, stop looping */
+ break;
+ }
}
if (!pelf) {
@@ -739,9 +742,12 @@
char *strtab;
ElfW(Sym) *symtab;
unsigned int hn, si, sn, sf;
- ElfW(Addr) sa;
+ ElfW(Addr) sa = 0;
- sa = 0;
+ /* Set the info for the object the address lies in */
+ __info-dli_fname = pelf-libname;
+ __info-dli_fbase = (void *) DL_LOADADDR_BASE(pelf-loadaddr);
+
symtab = (ElfW(Sym) *) (pelf-dynamic_info[DT_SYMTAB]);
strtab = (char *) (pelf-dynamic_info[DT_STRTAB]);
@@ -763,10 +769,14 @@
}
if (sf) {
- __info-dli_fname = pelf-libname;
- __info-dli_fbase = (void *)
DL_LOADADDR_BASE(pelf-loadaddr);
+ /* A nearest symbol has been found; fill the entries */
__info-dli_sname = strtab + symtab[sn].st_name;
__info-dli_saddr = (void *)sa;
+ } else {
+ /* No symbol found, fill entries with NULL value,
+ only the containing object will be returned. */
+ __info-dli_sname = NULL;
+ __info-dli_saddr = NULL;
}
return 1;
}
___
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
Re: [PATCH] Fix dladdr return value when cannot find symbol
Nickolai Zeldovich wrote: The attached patch makes dladdr() return 0 when it cannot find a matching symbol. The current behavior of returning 1 makes it impossible for callers to know whether the supplied Dl_info buffer has been filled in with meaningful data. Nickolai. I'll have a look. Please inlined patch are preferable. Regards, Carmelo ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc ___ uClibc mailing list uClibc@uclibc.org http://busybox.net/cgi-bin/mailman/listinfo/uclibc
