1226179 - 3.21 build failure on OpenBSD with gcc 4.2.1

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Landry Breuil (:gaston)]

Build fails with

In file included from secoid.c:1915:
verref.h: In function 'SECOID_Init':
verref.h:22: error: #pragma GCC diagnostic not allowed inside functions
verref.h:23: error: #pragma GCC diagnostic not allowed inside functions
verref.h:31: warning: unused variable '_nss_version_c'
verref.h:39: error: #pragma GCC diagnostic not allowed inside functions

Probable fallout of the #pragma addition in #1182667

Comment 1

[Landry Breuil (:gaston)]

Another nasty one:

g++ -o OpenBSD5.8_64_OPT.OBJ/ssl_agent_unittest.o -c  -fPIC -DPIC  -ansi -Wall -Wno-switch -pipe -DOPENBSD -DXP_UNIX -UDEBUG -DNDEBUG -pthr
ead -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -I../../external_tests/google_test/gtest/include -
I/usr/local/include/nspr -I../../../dist/OpenBSD5.8_64_OPT.OBJ/include -I../../../dist/public/nss -I../../../dist/private/nss -I../../../di
st/public/nspr -I../../../dist/public/nss -I../../../dist/public/libdbm -I../../../dist/public/gtest -I/usr/local/include -O2 -pipe   -I../
../lib/ssl -std=c++0x ssl_agent_unittest.cc
cc1plus: error: unrecognized command line option "-std=c++0x"


So nss decided to unsupported old compilers on purpose or it's an oversight ?

Comment 2

[Franziskus Kiefer [:franziskus]]

You probably want to disable gtests. Have a look at bug 1225377 comment 3. That should at least solve the problem from comment 1

Comment 3

[Landry Breuil (:gaston)]

Sorry, but only disabling gtests wont fix the #pragma error, i think there needs another check for the gcc version to enable those..

Comment 4

[Landry Breuil (:gaston)]

#pragma GCC diagnostic was added in gcc 4.6.

Testing this in the #ifdef

#if defined(__GNUC__) && !defined(NSS_NO_GCC48) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6))

Comment 5

[Landry Breuil (:gaston)]

Attachment: use #pragma diagnostic only on gcc >= 4.6

Comment 6

[Martin Thomson [:mt:]]

Attachment: bug1226179.patch

The test for the right compiler version is more difficult than you have in your patch.  This should work better.

Comment 7

[Landry Breuil (:gaston)]

Comment on attachment 8699348 [details] [diff] [review]
bug1226179.patch

That doesn't seem to work on 3.21, replacing my patch - which is *not* about -Werror but about #pragma diagnostic :

===>  Building for nss-3.21
coreconf/Werror.mk:55: Unable to find gcc 4.8 or greater, disabling -Werror

But the build still fails the same:

In file included from secoid.c:1915:
verref.h: In function 'SECOID_Init':
verref.h:22: error: #pragma GCC diagnostic not allowed inside functions
verref.h:23: error: #pragma GCC diagnostic not allowed inside functions
verref.h:31: warning: unused variable '_nss_version_c'
verref.h:39: error: #pragma GCC diagnostic not allowed inside functions

Comment 8

[Jan Beich]

Comment 0 also affects FreeBSD 9.x and NetBSD 6.x [1].

[1] https://github.com/jsonn/pkgsrc/commit/86e9200dfb187688af35a55fb6a08580f30cae96

Comment 9

[Martin Thomson [:mt:]]

Attachment: bug1226179.patch

This should make the changes more thorough.

I still don't understand how -Werror was being set for platforms other than Linux, since none of the makefiles for those platforms was including the Werror.mk file, or referencing WARNING_CFLAGS anywhere.

And even if they did, NSS_NO_GCC48 is defined if -Werror isn't, which would mean that the pragma isn't hit.  Have you made changes to the build configuration?  Or have you set any environment variables that might alter it?

Comment 10

[Landry Breuil (:gaston)]

(In reply to Martin Thomson [:mt:] from comment #9)
> Created attachment 8703500 [details] [diff] [review]
> bug1226179.patch
> 
> 
> And even if they did, NSS_NO_GCC48 is defined if -Werror isn't, which would
> mean that the pragma isn't hit.  Have you made changes to the build
> configuration?  Or have you set any environment variables that might alter
> it?

The changes to the build config are unrelated imo, see the few patches in http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/security/nss/patches/ - and the env vars set urelated too ,see http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/security/nss/Makefile?rev=1.56&content-type=text/x-cvsweb-markup

Comment 11

[Landry Breuil (:gaston)]

Comment on attachment 8703500 [details] [diff] [review]
bug1226179.patch

With your patch, using

gmake NSPR_INCLUDE_DIR=/usr/local/include/nspr NSPR_LIB_DIR=/usr/local/lib/ USE_64=1 NSS_DISABLE_GTESTS=1

in nss tip, the build goes past verref.h, only one warning remaining on it:
In file included from secoid.c:1987:
verref.h: In function 'SECOID_Init':
verref.h:31: warning: unused variable '_nss_version_c'

-DNSS_NO_GCC48 is present in the build commands - the build is successful.
(i still need NSS_DISABLE_GTESTS=1 otherwise it fails because those require stdc++11)

Note that i'm not an nspr peer, so i can only f+ this...

Comment 12

[Martin Thomson [:mt:]]

Thanks for checking.  The warning is expected in these circumstances.

Comment 13

[Jan Beich]

Comment on attachment 8703500 [details] [diff] [review]
bug1226179.patch

This fixes build on FreeBSD 9.3-RELEASE. Like OpenBSD we're going to pass NSS_DISABLE_GTESTS before switching to system gtest. 1.7.0 doesn't require C++11 (or C++0x) by itself.

https://www.freebsd.org/doc/en/books/porters-handbook/bundled-libs.html
https://svnweb.freebsd.org/ports/head/devel/googletest/files/patch-bsd-defines?revision=373421&view=co

Comment 14

[Martin Thomson [:mt:]]

Thanks for checking.  We certainly intended for the C++ code to be disabled in builds like that.  Firefox builds disable it as well.

Comment 15

[Landry Breuil (:gaston)]

ted, ping ?

Comment 16

[(not currently active) Ted Mielczarek]

Er, this is an NSS patch, and I'm not an NSS peer.

Comment 17

[Martin Thomson [:mt:]]

Comment on attachment 8703500 [details] [diff] [review]
bug1226179.patch

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

Well, I'm fairly sure that I'm allowed to check this in, so I'm going to.  Sorry ted, not paying enough attention.

Comment 18

[Martin Thomson [:mt:]]

https://hg.mozilla.org/projects/nss/rev/7bbcda0719dc