Closed Bug 712281 Opened 13 years ago Closed 13 years ago

Check for dladdr during configure

Categories

(NSPR :: NSPR, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file, 1 obsolete file)

The support for dladdr is currently hardcoded in prlink.c as not being available for Android. In fact, it is available in the headers for Android >= 2.2. The way this is handled in the main configure is to use ANDROID_VERSION to decide whether dladdr is available or not, but since I'm planning to allow nspr to use dladdr even when the headers don't have it, it would be easier if it were overridable without changing the Android headers being used. All in all, the simple solution is just to check whether the function is available or not with an AC_CHECK.
Attached patch Check for dladdr during configure (obsolete) (deleted) — Splinter Review
Attachment #583131 - Flags: review?(wtc)
Assignee: wtc → mh+mozilla
Comment on attachment 583131 [details] [diff] [review] Check for dladdr during configure Thanks for the patch. In nsprpub/configure.in: >+AC_CHECK_FUNCS(dladdr) There is an existing AC_CHECK_FUNCS in nsprpub/configure.in: AC_CHECK_FUNCS(lchown strerror) We can add dladdr to the existing AC_CHECK_FUNCS. When I run the updated configure on Linux, it says: checking for dladdr... no I think it is because the test is performed without -ldl. In nsprpub/pr/src/linking/prlink.c: > PR_IMPLEMENT(char *) > PR_GetLibraryFilePathname(const char *name, PRFuncPtr addr) > { >-#if defined(USE_DLFCN) && !defined(ANDROID) \ >+#if defined(USE_DLFCN) && defined(HAVE_DLADDR) \ > && (defined(SOLARIS) || defined(FREEBSD) \ > || defined(LINUX) || defined(__GNU__) || defined(__GLIBC__) \ > || defined(DARWIN)) If the AC_CHECK_FUNCS(dladdr) autoconf test works as expected, then this whole ifdef can be simplified to be just #if defined(USE_DLFCN) && defined(HAVE_DLADDR) Correct?
Attachment #583131 - Flags: review?(wtc) → review-
(In reply to Wan-Teh Chang from comment #2) > Comment on attachment 583131 [details] [diff] [review] > Check for dladdr during configure > > Thanks for the patch. > > In nsprpub/configure.in: > > >+AC_CHECK_FUNCS(dladdr) > > There is an existing AC_CHECK_FUNCS in nsprpub/configure.in: > > AC_CHECK_FUNCS(lchown strerror) > > We can add dladdr to the existing AC_CHECK_FUNCS. > > When I run the updated configure on Linux, it says: > > checking for dladdr... no > > I think it is because the test is performed without -ldl. Oh, so I guess that the main configure in the mozilla tree is adding -ldl to the LDFLAGS. > In nsprpub/pr/src/linking/prlink.c: > > > PR_IMPLEMENT(char *) > > PR_GetLibraryFilePathname(const char *name, PRFuncPtr addr) > > { > >-#if defined(USE_DLFCN) && !defined(ANDROID) \ > >+#if defined(USE_DLFCN) && defined(HAVE_DLADDR) \ > > && (defined(SOLARIS) || defined(FREEBSD) \ > > || defined(LINUX) || defined(__GNU__) || defined(__GLIBC__) \ > > || defined(DARWIN)) > > If the AC_CHECK_FUNCS(dladdr) autoconf test works as expected, then > this whole ifdef can be simplified to be just > > #if defined(USE_DLFCN) && defined(HAVE_DLADDR) > > Correct? In theory, yes.
Attachment #583402 - Flags: review?(wtc)
Attachment #583131 - Attachment is obsolete: true
Blocks: 683127
Comment on attachment 583402 [details] [diff] [review] Check for dladdr during configure Is there an autoconf macro that we can use to test if the header <dlfcn.h> declares the dladdr() function?
(In reply to Wan-Teh Chang from comment #5) > Comment on attachment 583402 [details] [diff] [review] > Check for dladdr during configure > > Is there an autoconf macro that we can use to test if > the header <dlfcn.h> declares the dladdr() function? I don't think there is. If you want to write such a test, make sure to define _GNU_SOURCE when we define it (android and linux), because without it on linux, you don't get dladdr.
Wan-Teh, how do you want to go forward on this bug?
Comment on attachment 583402 [details] [diff] [review] Check for dladdr during configure I'd s/SAVE_LDFLAGS/_SAVE_LDFLAGS/ when landing, though.
Attachment #583402 - Flags: review?(wtc) → review?(ted.mielczarek)
Comment on attachment 583402 [details] [diff] [review] Check for dladdr during configure Review of attachment 583402 [details] [diff] [review]: ----------------------------------------------------------------- I think this is reasonable.
Attachment #583402 - Flags: review?(ted.mielczarek) → review+
Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.319; previous revision: 1.318 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.321; previous revision: 1.320 done Checking in pr/src/linking/prlink.c; /cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v <-- prlink.c new revision: 3.111; previous revision: 3.110 done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
This broke NSPR for me apparently. See bug 724243
Comment on attachment 583402 [details] [diff] [review] Check for dladdr during configure Review of attachment 583402 [details] [diff] [review]: ----------------------------------------------------------------- Sorry about the late review. ::: nsprpub/pr/src/linking/prlink.c @@ -1362,5 @@ > { > -#if defined(USE_DLFCN) && !defined(ANDROID) \ > - && (defined(SOLARIS) || defined(FREEBSD) \ > - || defined(LINUX) || defined(__GNU__) || defined(__GLIBC__) \ > - || defined(DARWIN)) We should verify the configure script detects dladdr on Solaris, FreeBSD, and Mac OS X. Otherwise it'd be safer to say #if defined(USE_DLFCN) && (defined(HAVE_DLADDR) \ && (defined(SOLARIS) || defined(FREEBSD) \ || defined(DARWIN)) Based on my inspection of nsprpub/configure.in, I believe the configure script adds -ldl to OS_LIBS on these three platforms, so it should be able to detect dladdr.
Priority: -- → P2
Target Milestone: --- → 4.9
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: