Re: (ITS#4982) link libldap_r explicitly with the threading libraries
by rra＠stanford.edu
Kurt Zeilenga <kurt(a)OpenLDAP.org> writes:
> libtool should properly handle -pthread. If it doesn't, that should be
> fixed instead of kludging things up with -lpthread. -pthread tells gcc
> to do the right thing. The right thing might not be to link in
> -lpthread. (On some versions of FreeBSD, the right think is -lc_r.)
> -pthread also might do more than link in a library.
> As OpenLDAP includes its own copy of libtool, a local fix can be
> provided until the upstream fixes it right.
I think the above was meant for ITS #4981?
This issue, ITS#4982, is that no threading flags (not -pthread, not
-lpthread, not anything) are passed into the final link line of libldap_r,
as near as I can tell. It's possible that I'm reading the makefile wrong
and this has been fixed in some other way since the Debian patch had been
applied, but libraries/libldap_r/Makefile.in contains:
UNIX_LINK_LIBS = $(LDAP_LIBLBER_LA) $(AC_LIBS) $(SECURITY_LIBS)
and build/lib-shared.mk has:
$(LIBRARY): version.lo
$(LTLINK_LIB) -o $@ $(OBJS) version.lo $(LINK_LIBS)
including only LINK_LIBS, and not the regular LIBS variable (which is what
includes LTHREAD_LIBS via XXXLIBS). It could be that I'm missing some
contribution to LTLINK_LIB handled by the threading macros, though.
--
Russ Allbery (rra(a)stanford.edu) <http://www.eyrie.org/~eagle/>
16 years
Re: (ITS#4982) link libldap_r explicitly with the threading libraries
by kurt＠OpenLDAP.org
libtool should properly handle -pthread. If it doesn't, that should be
fixed instead of kludging things up with -lpthread. -pthread tells gcc
to do the right thing. The right thing might not be to link in -
lpthread.
(On some versions of FreeBSD, the right think is -lc_r.) -pthread also
might do more than link in a library.
As OpenLDAP includes its own copy of libtool, a local fix can be
provided
until the upstream fixes it right.
-- Kurt
On May 24, 2007, at 1:25 AM, rra(a)stanford.edu wrote:
> Hallvard B Furuseth <h.b.furuseth(a)usit.uio.no> writes:
>> Hallvard B Furuseth writes:
>
>>> Is this the second half of the Debian patch you posted in
>>> ITS#4981 - so
>>> it'd be wrong to apply one one of these patches?
>
>> Sorry, "wrong to apply _only_ one of".
>
>> (I.e. does ITS#4981 introduce breakage which ITS#4982 fixes?) Maybe
>> everything worked well before we switched to libtool...
>
> No, they should be independent, I think. I believe that the
> rationale for
> 4982 is still correct; if you use pthread functions, you need to
> explicitly link against the library to avoid getting unversioned
> references. Unversioned references will *normally* not cause a
> problem,
> so they're a bit of a time bomb. They keep working fine right up
> until
> the library makes an incompatible change in the data structures,
> and then
> they break.
>
> --
> Russ Allbery (rra(a)stanford.edu) <http://www.eyrie.org/
> ~eagle/>
>
>
16 years
Re: (ITS#4982) link libldap_r explicitly with the threading libraries
by rra＠stanford.edu
Hallvard B Furuseth <h.b.furuseth(a)usit.uio.no> writes:
> Hallvard B Furuseth writes:
>> Is this the second half of the Debian patch you posted in ITS#4981 - so
>> it'd be wrong to apply one one of these patches?
> Sorry, "wrong to apply _only_ one of".
> (I.e. does ITS#4981 introduce breakage which ITS#4982 fixes?) Maybe
> everything worked well before we switched to libtool...
No, they should be independent, I think. I believe that the rationale for
4982 is still correct; if you use pthread functions, you need to
explicitly link against the library to avoid getting unversioned
references. Unversioned references will *normally* not cause a problem,
so they're a bit of a time bomb. They keep working fine right up until
the library makes an incompatible change in the data structures, and then
they break.
--
Russ Allbery (rra(a)stanford.edu) <http://www.eyrie.org/~eagle/>
16 years
Re: (ITS#4982) link libldap_r explicitly with the threading libraries
by h.b.furuseth＠usit.uio.no
Hallvard B Furuseth writes:
> Is this the second half of the Debian patch you posted in ITS#4981 - so
> it'd be wrong to apply one one of these patches?
Sorry, "wrong to apply _only_ one of".
(I.e. does ITS#4981 introduce breakage which ITS#4982 fixes?) Maybe
everything worked well before we switched to libtool...
--
Regards,
Hallvard
16 years
Re: (ITS#4982) link libldap_r explicitly with the threading libraries
by h.b.furuseth＠usit.uio.no
rra(a)stanford.edu writes:
> Since 2.2.23, Debian has been carrying the following patch to the libldap_r
> build process to explicitly link the library against the thread libraries.
> (...)
Is this the second half of the Debian patch you posted in ITS#4981 - so
it'd be wrong to apply one one of these patches?
--
Regards,
Hallvard
16 years
Re: (ITS#4981) Try -lpthread before -pthread
by h.b.furuseth＠usit.uio.no
rra(a)stanford.edu writes:
> (...) libtool does not pass -pthread through, -lpthread seems
> to work though.
> (...)
> + OL_PTHREAD_TRY([-lpthread], [ol_cv_pthread_lpthread])
> OL_PTHREAD_TRY([-pthread], [ol_cv_pthread_pthread])
> (...)
This may be correct for Debian, but looks dubious as a general patch.
On other systems that have a thread compiler switch it may be wrong to
link the thread library "by hand" and omit the compiler switch,
otherwise what's the compiler switch for?
Their comments seem to indicate libtool is a better place to fix it.
An OpenLDAP patch would likely need some 'if <debian>' code.
(Note, I do not know Debian, nor specifics about differing Linux
variants.)
--
Regards,
Hallvard
16 years
Re: (ITS#4983) libldap_r/tls.c compile failure
by h.b.furuseth＠usit.uio.no
kurt(a)OpenLDAP.org writes:
> tls.c: In function `tls_thread_self':
> tls.c:412: error: invalid operands to binary /
> tls.c:415: warning: comparison between pointer and integer
Well, that was quick... my tls.c rev 1.154 patch, tls_thread_self(),
deliberately causes that for non-integer thread types. I wanted
feedback from systems where that happens, since the OpenSSL doc says
such systems are rare and this code can break there.
The CRYPTO_set_id_callback manpage (from openssl/doc/crypto/threads.pod)
says the function expects an integer thread ID, but that the function is
not needed if getpid() returns different answers for different threads.
And that this may be dangerous to relay on because one might compile a
program on a machine where this is true and then run it on a machine
where it is false.
Anyway, possibly the correct fix is to not call this function on
FreeBSD, it depends on getpid(). I have no idea. But since it
presumably worked before:
The simplest fix is to just remove the 'ok' and 'Check' code. But then
we can again silently compile to bad code when the thread type is not an
integer.
Next simplest: simplify the 'ok' check to:
enum { ok = sizeof(ldap_pvt_thread_t) <= sizeof(long) };
which will still let us know of systems where the conversion to ulong
clearly loses data. Not quite a safe, since pointer<->integer
conversions can do who-knows-what. But I guess OpenLDAP depends on
those elsewhere.
(Can sizeof(long) be < sizeof(pointer) on FreeBSD?)
Also in any case cast tls_thread_self()'s return value to u.long.
--
Regards,
Hallvard
16 years
(ITS#4983) libldap_r/tls.c compile failure
by kurt＠OpenLDAP.org
Full_Name: Kurt Zeilenga
Version: HEAD
OS: FreeBSD RELENG_6
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (24.182.55.218)
Submitted by: kurt
/bin/sh ../..//libtool --mode=compile cc -g -O2 -I../../include
-I../../include -DLDAP_R_COMPILE -I./../libldap -I/usr/local/include
-I/usr/pkg/include/db44 -I/usr/pkg/include -DLDAP_LIBRARY -c tls.c
cc -g -O2 -I../../include -I../../include -DLDAP_R_COMPILE -I./../libldap
-I/usr/local/include -I/usr/pkg/include/db44 -I/usr/pkg/include -DLDAP_LIBRARY
-c tls.c -fPIC -DPIC -o .libs/tls.o
tls.c: In function `tls_thread_self':
tls.c:412: error: invalid operands to binary /
tls.c:415: warning: comparison between pointer and integer
tls.c:415: error: enumerator value for `ok' not integer constant
tls.c:416: error: negative width in bit-field `dummy'
tls.c:416: error: size of array `Check' is negative
tls.c:418: warning: return makes integer from pointer without a cast
16 years
(ITS#4982) link libldap_r explicitly with the threading libraries
by rra＠stanford.edu
Full_Name: Russ Allbery
Version: 2.3.35
OS: Debian
URL:
Submission from: (NULL) (171.64.19.147)
Since 2.2.23, Debian has been carrying the following patch to the libldap_r
build process to explicitly link the library against the thread libraries. The
comment at the time is:
* libraries/libldap_r/Makefile.in: Code that uses pthreads *must* be
linked with -pthread, even if it's a library; without this, the
libldap_r library ends up with dangling unversioned reference to
pthread_create() which gets resolved to a wrong version that causes
segfaults on 64-bit platforms. Closes: #304549.
See http://bugs.debian.org/304549 for the specific problems that this caused.
--- openldap2.3-2.3.11~/libraries/libldap_r/Makefile.in 2005-12-01
13:48:50.000000000 +0100
+++ openldap2.3-2.3.11/libraries/libldap_r/Makefile.in 2005-12-02
10:05:17.637342000 +0100
@@ -56,7 +56,7 @@
XXLIBS = $(SECURITY_LIBS) $(LUTIL_LIBS)
XXXLIBS = $(LTHREAD_LIBS)
NT_LINK_LIBS = $(LDAP_LIBLBER_LA) $(AC_LIBS) $(SECURITY_LIBS)
-UNIX_LINK_LIBS = $(LDAP_LIBLBER_LA) $(AC_LIBS) $(SECURITY_LIBS)
+UNIX_LINK_LIBS = $(LDAP_LIBLBER_LA) $(AC_LIBS) $(SECURITY_LIBS)
$(LTHREAD_LIBS)
.links : Makefile
@for i in $(XXSRCS); do \
16 years
(ITS#4981) Try -lpthread before -pthread
by rra＠stanford.edu
Full_Name: Russ Allbery
Version: 2.3.35
OS: Debian
URL:
Submission from: (NULL) (171.64.19.147)
Debian has been carrying the included patch for a while, originally added
against 2.1.22 with:
+ Fixed the undefined symbols in libldap_r.so.2 (closes: #195990).
| configure.in: Try -lpthread before -pthread to link the thread
library. libtool does not pass -pthread through, -lpthread seems
to work though.
and then refreshed some time later against 2.2.23 with:
* configure.in: reinstate the remainder of the fix for 195990 from
2.1.22-2: give preference to -lpthread over -pthread in configure.in,
because some archs (mipsel, at least) don't like -pthread.
A quick search of other ITSes didn't reveal this particular report, but it did
look like people building on AIX were manually setting library flags that may
produce the same result. I'm sure that none of Debian's platforms will break
with -lpthread instead of -pthread since we've been using this for a while,
although I'm not sure if that's sufficient.
I'd ideally like to see Debian stop carrying this patch around as a divergence,
particularly since it means that the package has to re-run Autoconf during the
build.
Index: openldap2.3/configure.in
===================================================================
--- openldap2.3.orig/configure.in 2007-05-23 15:05:12.000000000 -0700
+++ openldap2.3/configure.in 2007-05-23 15:09:38.000000000 -0700
@@ -1489,6 +1489,7 @@ case $ol_with_threads in auto | yes | po
dnl OL_PTHREAD_TRY([-mt], [ol_cv_pthread_mt])
OL_PTHREAD_TRY([-kthread], [ol_cv_pthread_kthread])
+ OL_PTHREAD_TRY([-lpthread], [ol_cv_pthread_lpthread])
OL_PTHREAD_TRY([-pthread], [ol_cv_pthread_pthread])
OL_PTHREAD_TRY([-pthreads], [ol_cv_pthread_pthreads])
OL_PTHREAD_TRY([-mthreads], [ol_cv_pthread_mthreads])
16 years