Bug 242523

Summary: security/nss 3.48 doesn't build on PowerPC
Product: Ports & Packages Reporter: Luciano Mannucci <luciano>
Component: Individual Port(s)Assignee: freebsd-gecko (Nobody) <gecko>
Status: In Progress ---    
Severity: Affects Some People CC: canardo909, gecko, jhibbits, marklmi26-fbsd, mikael, pkubaj, powerpc
Priority: --- Flags: bugzilla: maintainer-feedback? (gecko)
Version: Latest   
Hardware: powerpc   
OS: Any   
See Also: https://bugzilla.mozilla.org/show_bug.cgi?id=1608151
Attachments:
Description Flags
patch
jbeich: maintainer-approval-
v2 pkubaj: maintainer-approval? (gecko)

Description Luciano Mannucci 2019-12-09 13:22:35 UTC
Unlike previous version, 3.48 doesn't build on powerPC. It says:

blinit.c:398:22: error: sys/auxv.h: No such file or directory
blinit.c: In function 'CheckPPCSupport':
blinit.c:410: warning: implicit declaration of function 'getauxval'
blinit.c:410: error: 'AT_HWCAP2' undeclared (first use in this function)
blinit.c:410: error: (Each undeclared identifier is reported only once
blinit.c:410: error: for each function it appears in.)
gmake[5]: *** [../../coreconf/rules.mk:393: FreeBSD11.3_OPT.OBJ/FreeBSD_SINGLE_SHLIB/blinit.o] Error 1
gmake[5]: Leaving directory '/usr/ports/security/nss/work/nss-3.48/nss/lib/freebl'
gmake[4]: *** [Makefile:654: libs] Error 2
gmake[4]: Leaving directory '/usr/ports/security/nss/work/nss-3.48/nss/lib/freebl'
gmake[3]: *** [../coreconf/rules.mk:101: libs] Error 2
gmake[3]: Leaving directory '/usr/ports/security/nss/work/nss-3.48/nss/lib'
gmake[2]: *** [coreconf/rules.mk:101: libs] Error 2
gmake[2]: Leaving directory '/usr/ports/security/nss/work/nss-3.48/nss'
*** Error code 1

Stop.
make[1]: stopped in /usr/ports/security/nss
*** Error code 1

Stop.
make: stopped in /usr/ports/security/nss
Comment 1 Mark Millard 2019-12-09 20:05:56 UTC
(In reply to Luciano Mannucci from comment #0)

sys/auxv.h and getauxval and AT_HWCAP2 is an area where
some ports have presumed that having sys/auxv.h implied
linux was the context in detail.

Quoting from the FreeBSD head sys/sys/auxv.h that turns
into /usr/include/sys/auxv.h (at least in head):

QUOTE
Revision 324815 . . .
Added Sat Oct 21 12:06:18 2017 UTC (2 years, 1 month ago) by mmel 
. . .
Make elf_aux_info() as public libc function.
- Teach elf aux vector functions about newly added AT_HWCAP and AT_HWCAP2
  vectors.
- Export _elf_aux_info() as new public libc function elf_aux_info(3)

The elf_aux_info(3) should be considered as FreeBSD counterpart of glibc
getauxval() with more robust interface.

Note:
We cannot name this new function as getauxval(), with glibc compatible
interface. Some ports autodetect its existence and then expects that all
Linux specific AT_<*> vectors are defined and implemented.
END QUOTE

multimedia/libvpx went through a round of being adjusted back in early
2017-Dec. A quote from back then, with a / vs. . typo fixed:

QUOTE (of Michal Meloun)
Having linux compatible getauxval() was my original plan.
But during testing, I found that some ports autodetect getauxval() presence and if exists, then full linux implementation is required.
My bad is that I didn't realize that the presence of ["sys/auxv.h"] causes the same troubles.
END QUOTE

So it may be that the port has to be updated to have FreeBSD specifics
in a place where it assumes linux currently.
Comment 2 Piotr Kubaj freebsd_committer 2019-12-11 18:58:32 UTC
Please try again after svn up at least to r519827.
Comment 3 Mark Millard 2019-12-11 19:23:23 UTC
(In reply to Piotr Kubaj from comment #2)

Use of -mvsx would prevent use of PowerMacs and such as far as I can tell:

Quoting https://en.wikipedia.org/wiki/AltiVec#VSX_(Vector_Scalar_Extension) about VSX:

Power ISA v2.06 introduces the new VSX vector-scalar instructions[5] which extend SIMD processing for the Power ISA to support up to 64 registers, with support for regular floating point, decimal floating point and vector execution. POWER7 is the first Power ISA processor to implement Power ISA v2.06.

End quote.

Old PowerMacs and such predate POWER7 and predate v2.06 of the Power ISA.

FreeBSD has not cut off such old machines from support (yet?).

So I'm unsure what should be done relative to PowerPC vintages.
Comment 4 Mark Millard 2019-12-11 19:29:40 UTC
(In reply to Mark Millard from comment #3)

Hmm. Looks like -mcrypto has a similar status (gnu docuumetnation):

-mcrypto
-mno-crypto
Enable the use (disable) of the built-in functions that allow direct access to the cryptographic instructions that were added in version 2.07 of the PowerPC ISA.


Hopefully nss is figuring out to avoid using such instructions on
old PowerPC hardware.
Comment 5 Piotr Kubaj freebsd_committer 2019-12-11 19:40:19 UTC
(In reply to Mark Millard from comment #4)
I only have access to POWER9, so I can't check.
Comment 6 Mark Millard 2019-12-11 20:13:37 UTC
(In reply to Piotr Kubaj from comment #5)

I may be able to check this weekend or some such. (No promises.)

Looking at the code in the patch, it may be that both 32-bit processor
and 64-bit processor contexts should be checked.

The patch shows a check assigning something called ppc_crypto_support_
but I do not see anything analogous in the patch for the older VSX.
Comment 7 Piotr Kubaj freebsd_committer 2019-12-12 12:50:12 UTC
I tried to build in 32-bit jail on my POWER9 box.
It fails to build gcm.c with:
cc1: error: unrecognized command line option "-mcrypto"
cc1: error: unrecognized command line option "-mvsx"

-mvsx was added by me, but -mcrypto is from upstream. I'm afraid they may simply not support 32-bit PPC anymore.
Comment 8 Piotr Kubaj freebsd_committer 2019-12-12 13:47:56 UTC
(In reply to Piotr Kubaj from comment #7)
In coreconf/FreeBSD.mk, there is CPU_ARCH variable being assigned. All powerpc architectures have ppc. Then, in lib/freebl/Makefile, there's a check for CPU_ARCH==ppc and that's when -maltivec -mcrypto -mvsx are appended. We would need to probably change differentiate powerpc64 from powerpc and powerpcspe, but I will be able to work on it next week (certainly not now).

If you want to build nss now, in lib/freebl/Makefile just remove the lines at the end containing -maltivec -mcrypto -mvsx.
Comment 9 Luciano Mannucci 2019-12-12 15:12:50 UTC
(In reply to Piotr Kubaj from comment #2)

Works now on my PowerPC8.

Many Thanks,

Luciano.
Comment 10 canardo 2019-12-14 09:50:00 UTC
(In reply to Piotr Kubaj from comment #8)

Workaround proposed in comment #8 works fine, security/nss is now built successfully on Powerbook G4
Comment 11 Piotr Kubaj freebsd_committer 2019-12-15 13:10:27 UTC
Created attachment 209971 [details]
patch

This patch fixes build on 32-bit PPC, since it makes sure to use PPC64-only CPU features on only PPC64.
Comment 12 Jan Beich freebsd_committer 2019-12-15 13:59:01 UTC
Comment on attachment 209971 [details]
patch

Did you try adjusting USES like on powerpc64? comment 7 indicates powerpc uses base GCC but powerpc64 uses lang/gcc9. According to the comment in lib/freebl/gcm.h the crypto code needs at least GCC 5.
Comment 13 Jan Beich freebsd_committer 2019-12-15 14:21:29 UTC
Comment on attachment 209971 [details]
patch

ifdef USE_64 here is arbitrary given USE_PPC_CRYPTO is disabled on big-endian out of caution (see comment in lib/freebl/gcm.h). Either USE_PPC_CRYPTO should be enabled or -mcrypto -maltivec -mvsx limited to little-endian.
Comment 14 Jan Beich freebsd_committer 2019-12-15 14:22:22 UTC
Also, if you limit -mcrypto -maltivec -mvsx to little-endian then USES=compiler:c++14-lang can be dropped from powerpc64.
Comment 15 Jan Beich freebsd_committer 2019-12-15 14:52:02 UTC
(In reply to Piotr Kubaj from comment #8)
> We would need to probably change differentiate powerpc64
> from powerpc and powerpcspe

powerpcspe doesn't support -maltivec according to base r307761.

(In reply to Mark Millard from comment #3)
> Use of -mvsx would prevent use of PowerMacs and such as far as I can tell:

Shouldn't be a problem if the file built with -mvsx is never called on a CPU that lacks VSX. Looking at FreeBSD kernel code, PPC_FEATURE_HAS_VSX is always available when PPC_FEATURE2_HAS_VEC_CRYPTO is.

> Old PowerMacs and such predate POWER7 and predate v2.06 of the Power ISA.

Is it possible to run FreeBSD powerpc (or only userland) on modern powerpc64-capable hardware akin to running FreeBSD i386 (or only userland) on modern x86_64-capable hardware? If so then -mcrypto -maltivec usage is correct even on 32-bit powerpc.

At least lib/freebl/gcm-ppc.c (with USE_PPC_CRYPTO defined) builds fine with Clang -target powerpc-unknown-freebsd13.0.
Comment 16 Mark Millard 2019-12-15 17:41:04 UTC
(In reply to Jan Beich from comment #15)

Context FYI: old powerpc64 PowerMacs also predate what both -mvsx
and -mcypto enable (predate POWER7 and its 2.06 ISA). It is not
just a 32-bit powerpc issue for old PowerMacs.

I had written in comment 6:

QUOTE
The patch shows a check assigning something called ppc_crypto_support_
but I do not see anything analogous in the patch for the older VSX.
END QUOTE

So, as it was, adding -mvsx appeared to block use of old PowerMacs
(always enabled), even for powerc64. In essence, requiring POWER7
(2.06 ISA) or later as written.

After the old PowerMacs, POWER7 (2.06) has what is needed for -mvsx but
-mcrypto comes even later (2.07). So having crypto may imply VSX is
present, but having VSX present does not imply crypto is present.
Supporting -mvsx use on POWER7 would require testing PPC_FEATURE_HAS_VSX
explicitly and avoiding VSX code when not present.
Comment 17 Piotr Kubaj freebsd_committer 2020-01-08 21:18:21 UTC
Created attachment 210548 [details]
v2

- Fix build without AltiVec, Crypto and VSX by moving altivec-types.h include to USE_PPC_CRYPTO's ifdef -this header is not necessary without USE_PPC_CRYPTO enabled,
- Enable USE_PPC_CRYPTO on BE as well if VSX and Crypto are supported,
- Add non-default VSX option for adding -mvsx -mcrypto to set USE_PPC_CRYPTO,
- since we don't need new GCC now, don't set USES=compiler:c11 by default.

This fixes powerpc and powerpc64 build without AltiVec, Crypto and VSX and makes it possible to use USE_PPC_CRYPTO which should accelerate things.

make test output without this patch:
Passed:             14415
Failed:             58
Failed with core:   0
ASan failures:      0
Unknown status:     52
TinderboxPrint:Unknown: 52

And with:
Passed:             14471
Failed:             2
Failed with core:   0
ASan failures:      0
Unknown status:     52
TinderboxPrint:Unknown: 52
Comment 18 Jan Beich freebsd_committer 2020-01-08 23:09:30 UTC
Comment on attachment 210548 [details]
v2

> VSX_CFLAGS=	-mcrypto -mvsx

When every file is built with SIMD runtime detection is pointless. Essentially, you're disabling USE_PPC_CRYPTO code for binary package users.

> +    defined(__VSX__) && defined(__CRYPTO__)

Can you check if this works with per-file SIMD flags?
Comment 19 Piotr Kubaj freebsd_committer 2020-01-09 09:31:24 UTC
(In reply to Jan Beich from comment #18)
The problem is that feature detection doesn't currently work for package users, jhibbits complains that he can't use nss on his Amiga X5000 (which doesn't even have AltiVec, let alone Crypto or VSX).
Comment 20 Justin Hibbits freebsd_committer 2020-01-30 15:55:36 UTC
(In reply to Jan Beich from comment #18)

I can't say right now if the build system even supports per-file SIMD flags but I can say that right now the port is broken for anything pre-POWER8.  That means all 32-bit powerpc, powerpcspe, and G5.  Right now I'm pretty sure G5 owners make up the majority of powerpc64 users, but that may change in the not too distant future.

It very well could be a simpler patch to only set the flags on the gcm-ppc.c, not eliminate the block altogether, since the code is already guarded behind an external platform support check in blinit.c.
Comment 21 Mikael Urankar freebsd_committer 2020-01-30 16:51:56 UTC
The ports is broken for me, I have a sigill on my g5:

Thread 1 received signal SIGILL, Illegal instruction.
0x000000081c1bf468 in ?? () from /usr/local/lib/nss/libfreeblpriv3.so
(gdb) disassemble 0x000000081c1bf468,-8 
Dump of assembler code from 0x81c1bf468 to 0xfffffffffffffff8:
=> 0x000000081c1bf468:  xxlxor  vs0,vs0,vs0

Piotr's patch fixes the issue.
Comment 22 commit-hook freebsd_committer 2020-02-10 23:51:58 UTC
A commit references this bug:

Author: jbeich
Date: Mon Feb 10 23:51:09 UTC 2020
New revision: 525766
URL: https://svnweb.freebsd.org/changeset/ports/525766

Log:
  security/nss: disable AltiVec on 32-bit powerpc

  Crypto acceleration is only implemented for powerpc64 but build flags
  leak to other powerpc targets. Disable via a variable introduced in 3.50.

  PR:		242523
  Reported by:	many

Changes:
  head/security/nss/Makefile
Comment 23 commit-hook freebsd_committer 2020-02-10 23:53:00 UTC
A commit references this bug:

Author: jbeich
Date: Mon Feb 10 23:52:09 UTC 2020
New revision: 525767
URL: https://svnweb.freebsd.org/changeset/ports/525767

Log:
  MFH: r525766

  security/nss: disable AltiVec on 32-bit powerpc

  Crypto acceleration is only implemented for powerpc64 but build flags
  leak to other powerpc targets. Disable via a variable introduced in 3.50.

  PR:		242523
  Reported by:	many
  Approved by:	ports-secteam blanket

Changes:
_U  branches/2020Q1/
  branches/2020Q1/security/nss/Makefile
Comment 24 Mark Millard 2020-02-11 01:00:29 UTC
(In reply to commit-hook from comment #23)

The below is not claiming that the handling is an
inappropriate way to deal with a messy 32-bit
context. It is just an FYI about the messy
context.

The 32-bit "G4" PowerMacs have processors
that handle alitvec. The "G3" 32-bit PowerMacs
do not.

(Alitvec = VMX = Velocity Engine, with
distinct naming due to trademark ownerships.
The later VMX128 is somewhat distinct.)
Comment 25 canardo 2020-05-01 18:12:53 UTC
Error when building security/nss on Powerbook G4 powerpc 32 bit : 

FreeBSD 13.0-CURRENT (GENERIC) #0 r360211: Thu Apr 23 05:00:07 UTC 2020

/usr/ports/security/nss # make -DBATCH install clean
===>  Building for nss-3.51.1_1
....
pqg.c:345:16: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
    if (addend < MP_DIGIT_MAX) {
        ~~~~~~ ^ ~~~~~~~~~~~~
1 error generated.
gmake[4]: *** [../../coreconf/rules.mk:393: FreeBSD13.0_OPT.OBJ/FreeBSD_SINGLE_SHLIB/pqg.o] Error 1
gmake[4]: Leaving directory '/usr/ports/security/nss/work/nss-3.51.1/nss/lib/freebl'
gmake[3]: *** [Makefile:639: libs] Error 2


Is it related to the problem above ?
(If not, I can open a new PR)

Does the error message help ?
Comment 26 commit-hook freebsd_committer 2020-05-01 19:45:54 UTC
A commit references this bug:

Author: jbeich
Date: Fri May  1 19:45:37 UTC 2020
New revision: 533584
URL: https://svnweb.freebsd.org/changeset/ports/533584

Log:
  security/nss: disable -Werror due to -Wall mine

  blinit.c:122:24: error: unused variable 'getauxval' [-Werror,-Wunused-variable]
  static unsigned long (*getauxval)(unsigned long) = NULL;
                         ^

  pqg.c:345:16: error: comparison of constant 18446744073709551615 with expression of type 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
      if (addend < MP_DIGIT_MAX) {
          ~~~~~~ ^ ~~~~~~~~~~~~

  PR:		242523

Changes:
  head/security/nss/Makefile
Comment 27 commit-hook freebsd_committer 2020-05-01 19:46:58 UTC
A commit references this bug:

Author: jbeich
Date: Fri May  1 19:46:12 UTC 2020
New revision: 533585
URL: https://svnweb.freebsd.org/changeset/ports/533585

Log:
  MFH: r533584

  security/nss: disable -Werror due to -Wall mine

  blinit.c:122:24: error: unused variable 'getauxval' [-Werror,-Wunused-variable]
  static unsigned long (*getauxval)(unsigned long) = NULL;
                         ^

  pqg.c:345:16: error: comparison of constant 18446744073709551615 with expression of type 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
      if (addend < MP_DIGIT_MAX) {
          ~~~~~~ ^ ~~~~~~~~~~~~

  PR:		242523
  Approved by:	ports-secteam blanket

Changes:
_U  branches/2020Q2/
  branches/2020Q2/security/nss/Makefile
Comment 28 Jan Beich freebsd_committer 2020-05-01 19:48:07 UTC
(In reply to canardo from comment #25)
Fixed. -Wall -Werror by default is recipe for wasting time due to -Wall changing between compiler vendors and compiler versions.
https://bugzilla.mozilla.org/show_bug.cgi?id=1384827