Bug 242521 - security/nss: fix build on powerpc*
Summary: security/nss: fix build on powerpc*
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: powerpc Any
: --- Affects Many People
Assignee: freebsd-gecko (Nobody)
URL: https://bugzilla.mozilla.org/show_bug...
Depends on:
Reported: 2019-12-09 10:27 UTC by Piotr Kubaj
Modified: 2019-12-13 12:03 UTC (History)
2 users (show)

See Also:
jbeich: maintainer-feedback+

patch (1.11 KB, patch)
2019-12-09 10:27 UTC, Piotr Kubaj
no flags Details | Diff
patch (2.65 KB, patch)
2019-12-11 11:43 UTC, Piotr Kubaj
no flags Details | Diff
v3 (3.14 KB, patch)
2019-12-11 16:37 UTC, Piotr Kubaj
no flags Details | Diff
patch (2.56 KB, patch)
2019-12-11 17:14 UTC, Piotr Kubaj
jbeich: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Kubaj freebsd_committer 2019-12-09 10:27:14 UTC
Created attachment 209799 [details]

https://bugzilla.mozilla.org/show_bug.cgi?id=1602386Upstream report: 

NSS 3.48 uses getauxinfo function on all ppc architectures unconditionally, which breaks compilation on systems other than Linux distributions.
Comment 1 Jan Beich freebsd_committer 2019-12-09 21:17:50 UTC
Comment on attachment 209799 [details]

After addressing upstream review comments can you put the patch into separate file e.g., files/patch-bug1602386.

files/patch-lib_freebl_blinit.c will be removed once bug 240037 is fixed.
Comment 2 Piotr Kubaj freebsd_committer 2019-12-11 00:06:26 UTC
It's another, unrelated issue, but there's a problem with building on elfv1.
Newer GCC is now necessary than base has, due to the usage of -mcrypto. However, GCC9 complains:
certdb.c: In function 'CERT_UnlockCertRefCount':
../../../dist/public/nss/secport.h:127:24: error: implicit declaration of function 'PR_ASSERT_ARG'; did you mean 'PR_ASSERT'? [-Werror=implicit-function-declaration]
  127 | #define PORT_AssertArg PR_ASSERT_ARG
      |                        ^~~~~~~~~~~~~

It seems to be in the code that was introduced recently in https://bugzilla.mozilla.org/show_bug.cgi?id=1589073. Strangely, clang on elfv2 can compile it.

I tried to build with -Wno-error=implicit-function-declaration, but then there's an issue about PR_ASSERT_ARG being unfound symbol.

Still investigating.
Comment 3 Piotr Kubaj freebsd_committer 2019-12-11 00:32:02 UTC
(In reply to Piotr Kubaj from comment #2)
Nevermind, NSPR 4.23 was too old.
Comment 4 Piotr Kubaj freebsd_committer 2019-12-11 11:43:14 UTC
Created attachment 209861 [details]

Is that ok?

New compiler is needed due to usage of -mcrypto and -mvsx. -mcrypto is added in nss's code and -mvsx is added by me because nss won't build otherwise:
altivec-types.h:19:1: error: use of 'long long' in AltiVec types is invalid without '-mvsx'

I also removed i386 section since it has C++14 capable compiler on all versions already.
Comment 5 Jan Beich freebsd_committer 2019-12-11 12:43:49 UTC
Comment on attachment 209861 [details]

> altivec-types.h:19:1: error: use of 'long long' in AltiVec types is invalid without '-mvsx'

Hmm, Clang 9.0.0 doesn't complain. Tested via -target powerpc64-unknown-freebsd13.0. Given 

> +.if ${ARCH} == powerpc64
> +CFLAGS+=	-mvsx

Adding -mvsx globally (for all files) would defeat runtime detection and make the binary package unusable without VSX support (assuming auto-vectorization). Can you try adding -mvsx after -maltivec? If it works submit the fix upstream.

# To build via GYP use build.sh in root directory
$ rg -- -maltivec
162:        '-maltivec'
166:        '-maltivec'

793:$(OBJDIR)/$(PROG_PREFIX)gcm-ppc$(OBJ_SUFFIX): CFLAGS += -mcrypto -maltivec

> +    sysctlbyname("hw.cpu_features", &hwcaps, &len, NULL, 0);

hw.cpu_features (unlike hw.cpu_features2) will never return PPC_FEATURE2_*. Also, won't build on FreeBSD 11 due to incorrect includes. See upstream review comment.
Comment 6 Piotr Kubaj freebsd_committer 2019-12-11 16:37:54 UTC
Created attachment 209871 [details]

Is it ok?

Fixes things without adding -mvsx globally. You don't experience a problem with clang, because the problem is only with gcc.
Comment 7 Piotr Kubaj freebsd_committer 2019-12-11 16:39:14 UTC
(In reply to Piotr Kubaj from comment #6)
Wait, blinit.c needs the fix from DR.
Comment 8 Piotr Kubaj freebsd_committer 2019-12-11 17:14:12 UTC
Created attachment 209872 [details]

This should be ok, I guess. Builds on 12.1-RELEASE.
Comment 9 Jan Beich freebsd_committer 2019-12-11 18:07:46 UTC
Comment on attachment 209872 [details]

Good enough. CPU feature detection is OK. AltiVec usage may need more work but let's unblock build and see if anyone complains about runtime.
Comment 10 commit-hook freebsd_committer 2019-12-11 18:58:33 UTC
A commit references this bug:

Author: pkubaj
Date: Wed Dec 11 18:57:48 UTC 2019
New revision: 519827
URL: https://svnweb.freebsd.org/changeset/ports/519827

  security/nss: fix build on powerpc platforms

  NSS 3.48 broke powerpc by using getauxval, which FreeBSD doesn't have. GCC9 is also required because of usage of VSX, which needs to be manually added for some files.

  PR:		242521
  Approved by:	jbeich (maintainer)