1405345 - ldflags "-Wl,-Bsymbolic" as defined in security/nss/lib/freebl/freebl_base.gypi is ignored

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Petr Sumbera]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170921064520

Steps to reproduce:

shlibsign cores dump while in infinite recursion (RNG_RNGInit calls RNG_RNGInit)
..
 00007fffb6932e0d RNG_RNGInit () + 24
 00007fffb6932e0d RNG_RNGInit () + 24
 00007fffb6733051 nsc_CommonInitialize () + 79
 00007fffb673353f NSC_Initialize () + 33
 00000000004155cc softokn_Init () + b9 (shlibsign.c:600)
 0000000000415ccf main () + 6d8 (shlibsign.c:919)
 0000000000414f93 _start () + 43

The solution is to use '-Wl,-Bsymbolic' during libfreebl3.so. Not sure why it's not being used though...

Comment 1

[Petr Sumbera]

I see '-Wl,-Bsymbolic' in security/nss/lib/freebl/freebl_base.gypi but it's not used.

Also security/nss/lib/freebl/Makefile sets '-Wl,-Bsymbolic'. But it's probably not used during Firefox build.

Any idea where is the problem?

Comment 2

[Franziskus Kiefer [:franziskus]]

Without more information it's hard to tell what's happening.
What and how exactly do you try to build?

Comment 3

[Petr Sumbera]

I build firefox like this:

$ PKG_CONFIG_PATH=/usr/lib/64/pkgconfig ./mach build -v

With following configuration

$ cat mozconfig
ac_add_options --with-system-libevent
ac_add_options --with-system-zlib
ac_add_options --enable-js-shell

From build output I can see that libfreebl3 is linked like this (no '-Wl,-Bsymbolic'):

139:53.55 /scratch/firefox/obj-x86_64-pc-solaris2.11/_virtualenv/bin/python -m mozbuild.action.file_generate /scratch/firefox/security/generate_mapfile.py main out.freebl.def .deps/out.freebl.def.pp /scratch/firefox/security/nss/lib/freebl/freebl.def
139:53.95 libfreebl3.so
139:53.95 rm -f libfreebl3.so
139:53.97 /scratch/firefox/obj-x86_64-pc-solaris2.11/_virtualenv/bin/python /scratch/firefox/config/expandlibs_exec.py --uselist --  /usr/bin/gcc -std=gnu99  -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -fno-strict-aliasing -fno-math-errno -pthread -pipe  -g -O -fno-omit-frame-pointer  -fPIC -shared -Wl,-h,libfreebl3.so -o libfreebl3.so  empty.o aeskeywrap.o alg2268.o alghmac.o arcfive.o arcfour.o blake2b.o blinit.o camellia.o chacha20_vec.o chacha20poly1305.o crypto_primitives.o ctr.o cts.o des.o desblapi.o dh.o drbg.o dsa.o ec.o ecdecode.o curve25519_64.o ec_naf.o ecl.o ecl_gf.o ecl_mult.o ecp_25519.o ecp_256.o ecp_256_32.o ecp_384.o ecp_521.o ecp_aff.o ecp_jac.o ecp_jm.o ecp_mont.o fipsfreebl.o freeblver.o gcm.o hmacct.o jpake.o ldvector.o md2.o md5.o mp_gf2m.o mpcpucache.o mpi.o mplogic.o mpmontg.o mpprime.o poly1305-donna-x64-sse2-incremental-source.o pqg.o rawhash.o rijndael.o rsa.o rsapkcs.o seed.o sha512.o sha_fast.o shvfy.o sysrand.o tlsprfalg.o hacl_curve25519_64.o   -lpthread  -Wl,-z,text     -L/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/bin     ../../../../../config/external/nspr/pr/libnspr4.so ../../../../../security/nss/lib/util/util_nssutil3/libnssutil3.so ../../../../../config/external/nspr/libc/libplc4.so ../../../../../config/external/nspr/ds/libplds4.so    -lsocket  -lpthread
139:55.04 chmod +x libfreebl3.so
139:55.05 gmake[5]: Leaving directory '/scratch/firefox/obj-x86_64-pc-solaris2.11/security/nss/lib/freebl/freebl_freebl3'

I just need to get confirmation that security/nss/lib/freebl/Makefile is not used when nss is build during Firefox build.

And I would like get explanation about security/nss/lib/freebl/freebl_base.gypi and particularly:

$ tail -4 security/nss/lib/freebl/freebl_base.gypi
 'ldflags': [
   '-Wl,-Bsymbolic'
 ],
}

Where is it used?

Thank you!

Comment 4

[Franziskus Kiefer [:franziskus]]

> I just need to get confirmation that security/nss/lib/freebl/Makefile is not used when nss is build during Firefox build.

It is not used.

Looking at this section of a Linux build I see

/builds/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python /builds/worker/workspace/build/src/config/expandlibs_exec.py --uselist --  /usr/bin/ccache /builds/worker/workspace/build/src/gcc/bin/gcc -std=gnu99  -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -pipe  -g -freorder-blocks -Os -fno-omit-frame-pointer  -fPIC -shared -Wl,-z,defs -Wl,--gc-sections -Wl,-h,libfreebl3.so -o libfreebl3.so  empty.o lowhash_vector.o   -lpthread -L/builds/worker/workspace/build/src/gtk3/usr/local/lib  -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,--build-id     -Wl,-rpath-link,/builds/worker/workspace/build/src/obj-firefox/dist/bin -Wl,-rpath-link,/usr/local/lib     ../../../../../config/external/nspr/pr/libnspr4.so ../../../../../config/external/nspr/libc/libplc4.so ../../../../../config/external/nspr/ds/libplds4.so -Wl,--version-script,out.freebl_hash.def   -ldl  -lpthread -ldl -lc

which doesn't have a -Wl,-Bsymbolic. So it doesn't seem to be necessary in the way Firefox builds NSS. The gyp NSS build files get translated to mozbuild with [1]. I think it's ignoring all ldflags set by gyp.

[1] https://searchfox.org/mozilla-central/rev/e62604a97286f49993eb29c493672c17c7398da9/python/mozbuild/mozbuild/frontend/gyp_reader.py

Comment 5

[Petr Sumbera]

Jan, don't you have similar problem like this on BSD?

In past this was issue on NetBSD:
http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/www/firefox/distinfo

   Log Message:
   Fix build on DragonFly as RNG_RNGInit was calling itself due to bad
   linkage. I love platform dependent magic in each Makefile.

Comment 6

[Petr Sumbera]

ldflags as defined in security/nss/lib/freebl/freebl_base.gypi are clearly ignored. There should be some way to define ldflags. E.g. via 'MOZBUILD_LDFLAGS'. But clearly it's no implemented yet.

Comment 7

[Petr Sumbera]

Following seems to enables ldflags use on various place again:

--- a/python/mozbuild/mozbuild/frontend/gyp_reader.py   Thu Sep 28 15:49:14 2017 -0700
+++ b/python/mozbuild/mozbuild/frontend/gyp_reader.py   Fri Oct 06 12:07:03 2017 +0000
@@ -282,6 +282,7 @@
             context['ASFLAGS'] = target_conf.get('asflags_mozilla', [])
             if use_defines_in_asflags and defines:
                 context['ASFLAGS'] += ['-D' + d for d in defines]
+            context['LDFLAGS'] = target_conf.get('ldflags', [])
             flags = target_conf.get('cflags_mozilla', [])
             if flags:
                 suffix_map = {

Comment 8

[Petr Sumbera]

Attachment: Bug1405345.patch

This makes build to use ldflags as defined in gypi files. And fixes few Solaris issues which appeared after new ldflags were used.

Comment 9

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8916965 [details] [diff] [review]
Bug1405345.patch

Review of attachment 8916965 [details] [diff] [review]:
-----------------------------------------------------------------

These are two different patches. Please split them and make the config.gypi patch against NSS.
I can review the config.gypi changes. But for the gyp_reader.py changes you should get review from :ted.mielczarek.

::: security/nss/coreconf/config.gypi
@@ +357,4 @@
>                '_REENTRANT',
>              ],
>            }],
> +          [ 'OS!="mac" and OS!="win" and OS!="solaris"', {

What about BSD and other systems?

Comment 10

[Petr Sumbera]

As for BSD it seems that its ld supports '-z noexecstack'.

Comment 11

[Petr Sumbera]

Attachment: Bug1405345-1.patch

Comment 12

[Petr Sumbera]

Attachment: Bug1405345-2.patch

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 8921496 [details] [diff] [review]
Bug1405345-1.patch

I'm a little worried that this may have undesirable effects for other targets on other platforms. There are a lot of ldflags specified in gyp files:
https://dxr.mozilla.org/mozilla-central/search?q=ldflags+path%3A.gyp&redirect=true

Without a full accounting of gyp targets I'm not sure that we won't wind up inserting new link flags we don't want.

Comment 14

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8921497 [details] [diff] [review]
Bug1405345-2.patch

Review of attachment 8921497 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm but can you make the patch for the NSS repo https://hg.mozilla.org/projects/nss/?

Comment 15

[Petr Sumbera]

Ted, it's entirely possible that enabling ldflags can cause some problems (it caused issue even for Solaris).

But I seem to be confused here... I think it's pretty valid reason to be able to specify ldflags someway. I also don't know who wrote gyp files specifying ldflags which were actually never used. And what about the Makefiles (like security/nss/lib/freebl/Makefile)? Are these old one which are never used now? Can you please provide some high level description of what is going on here?

Comment 16

[(not currently active) Ted Mielczarek]

I wrote the gyp files for the NSS build originally by writing scripts to evaluate the Makefiles and produce gyp files, and then hand-editing the output. I would guess that some ldflags were added after the fact, as I definitely did not do any work to support Solaris in that effort. The gyp files can be used to build NSS standalone, but are also used to build NSS as part of Firefox. For the standalone NSS build the ldflags will be used, but when built as part of Firefox they will not.

The Makefiles are simply the old NSS build system. When I landed the gyp build system I left that in place as I assumed there were existing users of it. I'm not actually an NSS peer, so I can't speak to whether it's still supported or deprecated for standalone NSS builds, but it's definitely not used when building Firefox anymore.

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 8921496 [details] [diff] [review]
Bug1405345-1.patch

Review of attachment 8921496 [details] [diff] [review]:
-----------------------------------------------------------------

I think it would be less invasive to explicitly set LDFLAGS in sandbox_vars here for Solaris:
https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/security/moz.build#116

Comment 18

[Petr Sumbera]

I tried following:

diff -r f41930a869a8 security/moz.build
--- a/security/moz.build        Thu Nov 16 00:24:15 2017 +0200
+++ b/security/moz.build        Mon Nov 20 13:58:12 2017 +0000
@@ -113,6 +113,8 @@
         # careful consideration.
         'NO_PGO': True,
     }
+    if CONFIG['OS_TARGET'] == 'SunOS':
+        sandbox_vars['LDFLAGS'] = ['-Wl,-Bsymbolic']
     if CONFIG['OS_TARGET'] == 'WINNT':
         if CONFIG['CPU_ARCH'] == 'x86':
             # This should really be the default.

But it turned up into following error:

203:04.65 Executing: /usr/bin/gcc -std=gnu99 -o certutil -D_FORTIFY_SOURCE=2 -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -fno-sized-deallocation -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -O -fno-omit-frame-pointer @/scratch/firefox/obj-x86_64-pc-solaris2.11/security/nss/cmd/certutil/certutil_certutil/tmpqsE5aw.list -lpthread -Wl,-z,text -Wl,-Bsymbolic -L/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/bin ../../../../../config/external/nspr/pr/libnspr4.so ../../../lib/nss/nss_nss3/libnss3.so ../../../lib/util/util_nssutil3/libnssutil3.so ../../../../../config/external/nspr/libc/libplc4.so ../../../../../config/external/nspr/ds/libplds4.so ../../../lib/smime/smime_smime3/libsmime3.so ../../../lib/ssl/ssl_ssl3/libssl3.so -lsocket -lpthread
203:04.66 /scratch/firefox/obj-x86_64-pc-solaris2.11/security/nss/cmd/certutil/certutil_certutil/tmpqsE5aw.list:
203:04.66     certext.o
203:04.66     certutil.o
203:04.66     keystuff.o
203:04.66     ../../lib/lib_sectool/basicutil.o
203:04.66     ../../lib/lib_sectool/derprint.o
203:04.66     ../../lib/lib_sectool/ffs.o
203:04.66     ../../lib/lib_sectool/moreoids.o
203:04.66     ../../lib/lib_sectool/pk11table.o
203:04.66     ../../lib/lib_sectool/pppolicy.o
203:04.66     ../../lib/lib_sectool/secpwd.o
203:04.66     ../../lib/lib_sectool/secutil.o
203:04.66
203:04.67 ld: fatal: option '-Bsymbolic' is incompatible with building a dynamic executable

So it means that -Bsymbolic cannot be used for linking everything in nss directory.

Any other option?

Comment 19

[Petr Sumbera]

Attachment: Bug 1405345 - Fix Solaris linking when using ldflags from gypi files

Comment 20

[Petr Sumbera]

Attachment: Bug 1405345 - ldflags as defined in gypi shouldn't be ignored at least on Solaris for now

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbfe142d94f9
ldflags as defined in gypi shouldn't be ignored at least on Solaris for now r=froydnj

Comment 22

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/bbfe142d94f9

Comment 23

[Phabricator Automation]

Comment on attachment 9056062 [details]
Bug 1405345 - Fix Solaris linking when using ldflags from gypi files

Revision D26278 was moved to bug 1667989. Setting attachment 9056062 [details] to obsolete.