Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#8064 closed bug (fixed)

dladdr doesn't work for local symbols

Reported by: unitedroad Owned by: axeld
Priority: normal Milestone: R1
Component: System/libroot.so Version: R1/alpha3
Keywords: dladdr, shared library, so, dlfcn gsoc2012 Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

Hi,

the dladdr function doesn't resolve the library location for for the symbols that are not exposed by the library to outside. This used to work in a previous revision from Haiku from last year.

This is my code:

main.c:

#include <stdio.h>
#include <dlfcn.h>
#include <unistd.h>
#include <string.h>

int main(){
	void (*dladdrerp)();
	char liblocation[1024];
	getcwd(liblocation,1024);
	strcat(liblocation,"/liblso.so");
	void *handle = dlopen(liblocation, RTLD_LAZY);
	dladdrerp = (void(*)())dlsym(handle,"dladdrer");
	dladdrerp();
}

lso.c (where the dladdr function is called):

#include <stdio.h>
#include <dlfcn.h>

int dladdree(){
	
}

int dladdrer(){
	Dl_info info;
	if (dladdr(dladdree,&info)==0){
		printf("dladdree not found\n");
	} else {
		printf("%s\n",info.dli_fname);
	}
}

lso.exp used to create th shared library:

LSO_1.1 {
	global:
		dladdrer;
	local :*;
};

I have created function called "dladdree" that is resolved through dladdr function by "dladdrer". dladdrer prints the location of the shared library containing dladdree in Ubuntu Linux 10.10, FreeBSD 8.1 and a Haiku revision from last year (I can't find the revision number since my nightly doesn't show it in Haiku nightly). However, in the Haiku revision 42917, it prints out "dladdree not found".

I have also attached all the code files with version script and Makefile with this report.

Attachments (3)

dladdr.zip (1.1 KB) - added by unitedroad 8 years ago.
0001-Fix-dladdr-behaviour.patch (6.8 KB) - added by hamish 7 years ago.
0001-Fix-dladdr-behaviour2.patch (7.0 KB) - added by hamish 7 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by anevilyak

What revision was that behavior seen with? If a nightly, hrev42598 may be the culprit.

Changed 8 years ago by unitedroad

Attachment: dladdr.zip added

comment:2 Changed 8 years ago by unitedroad

I tested it on two different nightlies. I found this issue on the newer of the two. I recently built that nightly using the svn revision 42917.

However I am not able to find the svn revision of the nightly on which I didn't get this issue. I built that nightly about 8 months ago using the codebase that was current then.

comment:3 Changed 8 years ago by anevilyak

The only change to that function this year was hrev42598 on August 7th, so that seems to confirm that. Thanks.

comment:4 Changed 7 years ago by hamish

dladdr shouldn't be able to find local symbols.

However, according to the man page (1) even if it doesn't find an exact match, it should return the nearest symbol with a value less than the searched address. And (2) if it can't find any nearest symbol, but the searched address does fall within a loaded library, it should still return that library name and base address, with null symbol address and symbol name.

The pre-hrev42598 implementation satisfied (1), which I guess is why your test worked, while the new one doesn't. Neither satisfies (2).

Since get_symbol_at_address() in the runtime loader is only used by dladdr(), I presume it's OK to just change it? And perhaps rename it get_nearest_symbol_at_address()?

comment:5 Changed 7 years ago by hamish

Has a Patch: set

comment:6 Changed 7 years ago by hamish

Has a Patch: unset

comment:7 Changed 7 years ago by hamish

Sorry about the noise. I thought I was being clever getting rid of the static buffers in dladdr but I realised image_t::name stores the name only, not the full path as required for Dl_info::dli_fname. Would it be OK to add a path field to image_t that register_image could fill in?

Changed 7 years ago by hamish

comment:8 Changed 7 years ago by hamish

Has a Patch: set

comment:9 Changed 7 years ago by hamish

image_t::path already exists. Dunno how I missed that before. Latest patch is hopefully OK now.

comment:10 in reply to:  4 Changed 7 years ago by bonefish

Thanks for the patch. It looks basically OK, but I have two remarks:

  • Please don't use uncommon abbreviations in identifier names. foundSym -> foundSymbol, _symName -> _symbolName.
  • Please don't use goto.

Either may be violated already in the runtime loader code, but that's because we inherited it from NewOS.

Directly using the name pointers from the runtime loader is not so nice (since they can go away any time after unlocking the runtime loader main lock), but since the only alternative is what the previous implementation of dladdr() did -- using static buffers that get overwritten with the next call -- it's either a rock or a hard place. I also prefer your implementation in this case.

comment:11 Changed 7 years ago by hamish

I thought breaking out of a nested loop was one of the few acceptable uses for a goto. So it would be preferable to flag and break then?

dladdr manuals for some platforms do state that the pointers can become invalid when the library in question is unloaded: http://www.commsys.isy.liu.se/cgi-bin/man-cgi?dladdr+3. Hopefully programs that use it take this into account, so passing back pointers from the runtime loader shouldn't be too big a problem.

comment:12 in reply to:  11 Changed 7 years ago by bonefish

Replying to hamish:

I thought breaking out of a nested loop was one of the few acceptable uses for a goto. So it would be preferable to flag and break then?

A flag is fine. Alternatively by introducing an iterator class for the symbol hash table (which could be use elsewhere as well) the double loop could be compacted to a single loop and a simple break would suffice to leave the loop.

Changed 7 years ago by hamish

comment:13 Changed 7 years ago by hamish

Went with a flag -- the hash table is only iterated in two places.

comment:14 Changed 7 years ago by bonefish

Thanks, applied as hrev43953 with one change though: I replaced the found == false by !found.

comment:15 Changed 7 years ago by bonefish

Resolution: fixed
Status: newclosed

comment:16 Changed 7 years ago by mmadia

Keywords: gsoc2012 added
Note: See TracTickets for help on using tickets.