Please add --disable-new-dtags to tests makefile

[karl.s...@canonical.com]

I'm hoping this is the correct place to add this request:

On some systems, dynamic library weirdness causes the tests to fail because it looks in the wrong place for the libraries it needs. There's some more information about this phenomena here: https://blog.qt.io/blog/2011/10/28/rpath-and-runpath/

On systems like Ubuntu, we add the following patch to get the tests to link properly:

--- a/nspr/pr/tests/Makefile.in
+++ b/nspr/pr/tests/Makefile.in
@@ -315,7 +315,7 @@ ifeq ($(OS_ARCH), SunOS)
endif # SunOS

ifeq (,$(filter-out Linux GNU GNU_%,$(OS_ARCH)))
- LDOPTS += -Xlinker -rpath $(ABSOLUTE_LIB_DIR)
+ LDOPTS += -Xlinker -rpath $(ABSOLUTE_LIB_DIR) -Xlinker --disable-new-dtags
ifeq ($(USE_PTHREADS),1)
EXTRA_LIBS = -lpthread
endif


However, it's always best to be able to ship the upstream package unchanged, to reduce the risk of our changes breaking things unintentionally. Would it be possible to add --disable-new-dtags in the upstream tests makefile?


Cheers,
Karl

[pae...@gmail.com]

While the initial suggestion was --disable-new-dtags I took a look why it actually fails.

TL;DR:
- trigger is the linker default of "as-needed"
- errcodes test has NO direct libnspr4 dependencies
- the tests use rpath to overcome lib lookup in the test environment
- since errcodes has no own libnspr4 need it is stripped
- the indirect need errcodes -> libplc4 -> libnspr4 can't be resolved
- suggestion add "-Xlinker --no-as-needed" for the tests in the makefile

--- details ---

Tests in nspr usually can be ran-as is and get a PASS if ok.
For example:
./zerolen
calling PR_Writev with a zero-length buffer
calling PR_Write with a zero-length buffer
calling PR_Send with a zero-length buffer
PASS

But errcodes fails (if not fixed by the pacth in discussion):
./errcodes: error while loading shared libraries: libnspr4.so: cannot open shared object file: No such file or directory

lets compare zerolen and errcodes in that regard:
# ldd zerolen
linux-vdso.so.1 (0x00007ffee0f4a000)
libnspr4.so => /build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib/libnspr4.so (0x00007f53f2fe2000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f53f2fbd000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f53f2dd2000)
libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f53f2dcc000)
librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f53f2dc1000)
/lib64/ld-linux-x86-64.so.2 (0x00007f53f3029000)

# ldd errcodes
linux-vdso.so.1 (0x00007fff1a8ec000)
libplc4.so => /build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib/libplc4.so (0x00007f5991b6d000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f599197e000)
/lib64/ld-linux-x86-64.so.2 (0x00007f5991b7b000)
libnspr4.so => not found

Link step:
x86_64-linux-gnu-gcc zerolen.o -Xlinker -rpath /<<PKGBUILDDIR>>/nspr/pr/tests/../../dist/lib -L../../dist/lib -lplc4 -L../../dist/lib -lnspr4 -lpthread -o zerolen
x86_64-linux-gnu-gcc errcodes.o -Xlinker -rpath /<<PKGBUILDDIR>>/nspr/pr/tests/../../dist/lib -L../../dist/lib -lplc4 -L../../dist/lib -lnspr4 -lpthread -o errcodes

looks the same to me, so why is it different?

One thing I wondered is the rpath setting in the makefile:
I see (matching what I have seen in other places) -Xlinker for each option and argument in e.g. SunOS
LDOPTS += -Xlinker -R -Xlinker $(ABSOLUTE_LIB_DIR)
But for GNU Linux I see:

LDOPTS += -Xlinker -rpath $(ABSOLUTE_LIB_DIR)

I first thought we might actually need:
LDOPTS += -Xlinker -rpath -Xlinker $(ABSOLUTE_LIB_DIR)
But that isn't the trick we need here.


It isn't clear to me yet why this would only affect errcodes but not the others.
I found that errcodes doesn't list libnspr4.so in the headers

# readelf -d zerolen
Tag Type Name/Value
0x0000000000000001 (NEEDED) Shared library: [libnspr4.so]
0x0000000000000001 (NEEDED) Shared library: [libpthread.so.0]
0x0000000000000001 (NEEDED) Shared library: [libc.so.6]
0x000000000000001d (RUNPATH) Library runpath: [/build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib]

# readelf -d errcodes
Tag Type Name/Value
0x0000000000000001 (NEEDED) Shared library: [libplc4.so]
0x0000000000000001 (NEEDED) Shared library: [libc.so.6]
0x000000000000001d (RUNPATH) Library runpath: [/build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib]

Before going further lets check if the build "fix" via --disable-new-dtags is resolvable here still.

# x86_64-linux-gnu-gcc errcodes.o -Xlinker -rpath /build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib -L../../dist/lib -lplc4 -L../../dist/lib -lnspr4 -lpthread -o errcodes; ldd errcodes | grep libnspr4.so
libnspr4.so => not found
# x86_64-linux-gnu-gcc errcodes.o -Xlinker --disable-new-dtags -Xlinker -rpath /build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib -L../../dist/lib -lplc4 -L../../dist/lib -lnspr4 -lpthread -o errcodes; ldd errcodes | grep libnspr4.so
libnspr4.so => /build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib/libnspr4.so (0x00007f634510e000)

So yeah the fix absolutely works here ?!?

The difference is seen in the elf header:
- 0x000000000000001d (RUNPATH) Library runpath: [/build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib]
+ 0x000000000000000f (RPATH) Library rpath: [/build/nspr-WHjrwX/nspr-4.21/nspr/pr/tests/../../dist/lib]

Setting gcc -v and Xlinker --verbose I tried to find the difference to zerolen.

It seems the difference is that zerolen needs nspr4.so "directly" while for errcodes it is only:
libnspr4.so needed by ../../dist/lib/libplc4.so
found libnspr4.so at ../../dist/lib/libnspr4.so
(those lines are not present in zerolen)

Comparing the execution with/without --disable-new-dtags is even less helpful.
All output is the same but the final ldd then resolves correctly.

I checked the verbose output of all tests programs build and it is true, errcodes is the only one "not needig nspr4 directly". So the issue with the library path resolution comes like this:
a) testprog -> nspr4 (ok)
b) testprog -> lib -> nspr4 (ldd fails)

I don't yet fully see how --disable-new-dtags is really related here, as mentioned above all I found is the switch from RUNPATH -> RPATH which is a bit stricter in precedence AFAIK.

I have seen other cases where this second grade dependencies wreaked havoc.
In those cases it was related to --as-needed being set differently.

I experimented with the weaker --no-allow-shlib-undefined but that wasn't enough.
But as I assumed --no-as-needed did the trick.

It would be great to make this test resilent against this issue for the time a build env switches again (as-needed in general is a great feature).

So IMHO the suggested change would better be to add "-Xlinker --no-as-needed"