244095 - several libraries need to be linked with -R $ORIGIN

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Julien Pierre]

As part of the NSS 3.9 integration with Solaris 10, we need to add the above
linker flags . This is so that the Solaris maintainers are happy; and helps NSS
applications (linked with -R NSS_explicit_location) can correctly locate all
.so's  without LD_LIBRARY_PATH having to be set.

We also need to do this for NSPR .so's.

The list of libraries that need to be fixed is :
libssl3.so
libsoftokn3.so (apparently for this one, the problem is only on Solaris x86, not
Sparc ...)
libplc4.so
libplds4.so

For NSS, we want this fix in 3.9.2 . For NSPR it's a bit less clear. Should we
make a 4.4.2, or should this go into 4.5 ? I see there is an NSPR 4.5 branch,
but it hasn't RTM'ed yet. Any info is appreciated.

Comment 1

[Julien Pierre]

Attachment: fix softoken build

The -R $ORIGIN linker option was set only for 32-bit Sparc on SunOS. We need it
for all Solaris builds.

Comment 2

[Julien Pierre]

Attachment: fix libssl build

Comment 3

[Wan-Teh Chang]

Comment on attachment 148886 [details] [diff] [review]
fix softoken build

The comment needs to be updated.

Originally libsoftokn3.so only needs to
help to find libfreebl_hybrid_3.so and
libfreebl_pure32_3.so.	Since those two
.so's only exist for 32-bit SPARC, we
don't need -R $ORIGIN for x86 or 64-bit SPARC.

Now the comment should mention the other
dependencies (NSPR .so's).

Please explain why only libssl3.so and
libsoftokn3.so need to be linked with
-R $ORIGIN.  It seems that all .so's
should need this.

Comment 4

[Wan-Teh Chang]

The only exception should be libnspr4.so, which
only depends on system libraries.

libsmime3.so, and probably libnssckbi.so should
also need to be linked with -R $ORIGIN. (libnss3.so
is already linked with -R $ORIGIN.)

Comment 5

[Julien Pierre]

Wan-Teh,

I don't believe the problem these patch solves isn't a practical problem. I
haven't seen an actual runtime case where changes are required.

The problem is that the Solaris team has added the requirement that "ldd xxx.so"
resolves all explicit dependencies for all libraries shipped with Solaris 10. 
This test failed on the libraries I listed. The failures were reported by the
Solaris team. The patches only help the ldd command succeed. I believe they are
harmless.

You are right that there are other libraries that need the same changes (except
for libnspr4.so, as you correctly point out). I will find out why they weren't
reported to us.

Comment 6

[Wan-Teh Chang]

Comment on attachment 148887 [details] [diff] [review]
fix libssl build

Suggest removing the explicit list of dependencies
from the comment, so that this comment can be used
in the makefiles for other libraries.

Comment 7

[Wan-Teh Chang]

Comment on attachment 148886 [details] [diff] [review]
fix softoken build

Suggest removing "(libfreebl_*.so)" from the
comment because it is no longer accurate.

Comment 8

[Julien Pierre]

Wan-Teh, while working on an updated patch, I noticed that libnssckbi.so doesn't
appear to have any dependencies according to ldd ! Indeed, nm shows that all the
NSPR symbols are linked statically. Is this intentional ? I still can't figure
why it doesn't even depend on libc, though.

Also, could you please answer my question about NSPR at the top of this bug ?
Should I open a separate NSPR bug for it ?

Comment 9

[Wan-Teh Chang]

We probably should do this on the NSPR_4_5_BRANCH
and target the NSPR 4.5.1 release.  (NSPR 4.5 is
defined to be the NSPR version in Mozilla 1.7 final.
Anything change for NSPR 4.5 requires mozilla.org
drivers' approval.)

It is okay to use this bug for NSPR patches.  If
you can open an NSPR bug, that's certainly the right
thing.

Comment 10

[Julien Pierre]

Attachment: consolidated NSS patch

This patch updates the config for all the NSS shared libraries that we ship
(ie. everything but fortezza).

Comment 11

[Wan-Teh Chang]

Comment on attachment 148985 [details] [diff] [review]
consolidated NSS patch

r=wtc.

I don't think lib/ckfw/builtins/config.mk needs this patch because it only
depends on system libraries.

Comment 12

[Julien Pierre]

Checked in to NSS_3_9_BRANCH :

Checking in ckfw/builtins/config.mk;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/config.mk,v  <--  config.mk
new revision: 1.7.16.1; previous revision: 1.7
done
Checking in freebl/Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.53.14.1; previous revision: 1.53
done
Checking in nss/config.mk;
/cvsroot/mozilla/security/nss/lib/nss/config.mk,v  <--  config.mk
new revision: 1.21.16.1; previous revision: 1.21
done
Checking in smime/config.mk;
/cvsroot/mozilla/security/nss/lib/smime/config.mk,v  <--  config.mk
new revision: 1.18.16.1; previous revision: 1.18
done
Checking in softoken/config.mk;
/cvsroot/mozilla/security/nss/lib/softoken/config.mk,v  <--  config.mk
new revision: 1.14.16.1; previous revision: 1.14
done
Checking in ssl/config.mk;
/cvsroot/mozilla/security/nss/lib/ssl/config.mk,v  <--  config.mk
new revision: 1.15.16.1; previous revision: 1.15
done

And the tip :

Checking in lib/ckfw/builtins/config.mk;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/config.mk,v  <--  config.mk
new revision: 1.9; previous revision: 1.8
done
Checking in lib/freebl/Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.55; previous revision: 1.54
done
Checking in lib/nss/config.mk;
/cvsroot/mozilla/security/nss/lib/nss/config.mk,v  <--  config.mk
new revision: 1.23; previous revision: 1.22
done
Checking in lib/smime/config.mk;
/cvsroot/mozilla/security/nss/lib/smime/config.mk,v  <--  config.mk
new revision: 1.20; previous revision: 1.19
done
Checking in lib/softoken/config.mk;
/cvsroot/mozilla/security/nss/lib/softoken/config.mk,v  <--  config.mk
new revision: 1.16; previous revision: 1.15
done
Checking in lib/ssl/config.mk;
/cvsroot/mozilla/security/nss/lib/ssl/config.mk,v  <--  config.mk
new revision: 1.18; previous revision: 1.17
done

Comment 13

[Julien Pierre]

Attachment: consolidated NSPR patch

Comment 14

[Wan-Teh Chang]

Comment on attachment 149248 [details] [diff] [review]
consolidated NSPR patch

This patch is basically correct.  Since both the Solaris
ld and GNU ld support the -R option, you should add the
new code outside the ifdef NS_USE_GCC block.  In other
words,

> ifdef NS_USE_GCC
> ifdef GCC_USE_GNU_LD
> MKSHLIB += -Wl,--version-script,$(MAPFILE)
> else
> MKSHLIB += -Wl,-M,$(MAPFILE)
> endif
> else
> MKSHLIB += -M $(MAPFILE)
>+# The -R '$ORIGIN' linker option instructs this library to search for its
>+# dependencies in the same directory where it resides.
>+MKSHLIB += -R '$$ORIGIN'
> endif
> endif

the three new lines should be added between the last two
endif's.

Comment 15

[Julien Pierre]

Attachment: updated patch

Comment 16

[Julien Pierre]

I have checked this in to NSPR_4_5_BRANCH . Wan-Teh, could you please check this
in to the tip and then close the bug ? Thanks.

Checking in lib/ds/Makefile.in;
/cvsroot/mozilla/nsprpub/lib/ds/Makefile.in,v  <--  Makefile.in
new revision: 1.32.4.1; previous revision: 1.32
done
Checking in lib/libc/src/Makefile.in;
/cvsroot/mozilla/nsprpub/lib/libc/src/Makefile.in,v  <--  Makefile.in
new revision: 1.28.4.1; previous revision: 1.28
done

Comment 17

[Wan-Teh Chang]

Comment on attachment 149303 [details] [diff] [review]
updated patch

I checked in this patch on the NSPR trunk (NSPR 4.6)
and NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.8 alpha 2).