244915 - freebl doesn't build correctly on Solaris 10 Sparc

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Julien Pierre]

mozilla/security/nss/lib/freebl/Makefile includes the following test :

ifeq (5.5.1,$(firstword $(sort 5.5.1 $(OS_RELEASE))))
    SYSV_SPARC = 1
endif

On Solaris 10, the OS_RELEASE is 5.10 . The sort function in gmake thinks that
5.10 comes before 5.5.1 . This causes SYSV_SPARC not to be set, and the
resulting binaries don't work, just like in bug 237068 .

This means we need to find an alternate test. Since we don't build on 5.5.1
anyway - the oldest we support is 2.6, I question the value of doing the test at
all - we should probably do it by default.

We need to fix this since we have a requirement to build our NSS bits on Solaris 10.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Does Solaris 10 run on Sparc (non-Ultra) CPUs?
If not, Maybe it's best not to build the old sparc code for Sol10.

Comment 2

[Wan-Teh Chang]

It is okay to drop support for Solaris 2.5.1 so we
can avoid that particular test.  But similar tests
are used in NSPR and possibly NSS, and they may compare
against 5.6 or 5.7.  So a solution is still needed.

(See mozilla/nsprpub/pr/src/Makefile.in; search for
"5.7".)

Comment 3

[Wan-Teh Chang]

I found that mozilla/nsprpub/pr/src/Makefile.in is
the only other place where this construct is used.

Comment 4

[Roland Mainz]

Nelson Bolyard wrote:
> Does Solaris 10 run on Sparc (non-Ultra) CPUs?
> If not, Maybe it's best not to build the old sparc code for Sol10.

Solaris 10/SPARC only runs on UltraSPARC and compatible CPUs (and only on Sun
CPUs >= 200MHz unless you know a secret switch... :), however care should be
taken that the code works on SPARC64 (=Fujitsu's version of the SPARC CPUs),
too.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

First, some nomenclature (as I use it):
v8 instruction set artchitecure (ISA): an instruction set that runs on 
   32-bit Sparc CPUs, which have only 32-bit registers.
v8+ ISA: an superset of the v8 ISA, uses 64-bit registers, but 32-bit pointers.
   runs only on UltraSparc CPUs (which have 64-bit registers).  
v9 ISA: a superset of v8+, uses 64-bit registers and 64-bit pointers.  
   also known as LP64 model.

v8 and v8+ code can be linked together into a v8+ executable.  
They both use ilp32 model, with long long for 64-bits.

v9 code cannot be linked with any v8 or v8+ code.  

I call the machines that can only run v8, not v8+, not v9 "Sparc" CPUs.
They have only 32-bit registers.

I call the machines that have 64-bit registers "UltraSparc".  They run 
v8+ and/or v9 code.  They typically run v8 code also (backwards compatible).

Today, NSS can be built two ways for Solaris (ignoring Solaris x86 here):
a) "32-bit", produces code that is mostly v8, but includes TWO different
   shared libs for freebl, a v8 DSO, and a v8+ DSO.  softoken picks one and 
   loads it at run time, according to the registers available on the machine.
b) "64-bit", produces pure v9 code, IIRC.  

We build the "32-bit" build of NSS so that it can run on 32-bit Sparc and
64-bit UltraSparc CPUs.  But AFAIK, Sun has not manufactured a 32-bit sparc
CPU in this decade.  So, I expect that eventually, some version of Solaris
will stop supporting the old 32-bit Sparc CPUs.

My point is that when Sun produces a Solaris that is NOT compatible with the
old 32-bit sparc CPUs any more, and requires CPUs with 64-bit registers
(which I generically call "UltraSparcs), then we should STOP building NSS
with two shared libs (v8, and v8+), and switch to building ONLY the v8+ code.
And we should link freebl into softoken's DSO, rather than keeping it in a
separate DSO that is dynatmically loaded by softoken.  

So, my question was: is Solaris 10 the first version of Solaris that requires
CPUs with 64-bit registers?  or not?

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

To go one step further:  When the day comes that we no longer need to build
v8 code, then we should build ALL of NSS in v8+ mode, not just freebl.

Comment 7

[Julien Pierre]

Nelson,

To my knowledge, nothing has changed regarding the CPUs that Solaris 10 Sparc
supports vs the CPUs that Solaris 9 and earlier support.

The issue at hand here is that Solaris 10 was considered a pre-system V OS by
the gmake test, and therefore we decided not to build any of the Sparc assembly
code for any version of Sparc CPUs. The result was that when building under
Solaris 10, we used the C code instead. I don't know why that C code crashed at
runtime as it did, eating all the memory on the machine in bltest, but I suspect
it's the same problem as in bug 237068 comment 6.

Since nobody builds on Solaris 2.5.1 anymore, we can safely remove the bogus
Solaris OS version test. A patch is forthcoming.

Comment 8

[Julien Pierre]

Attachment: always set SYSV_SPARC to 1

I also attempted to clarify the Makefile with comments on endif .

Comment 9

[Julien Pierre]

Wan-Teh,

FYI, for NSPR, there is a link from /usr/lib/libposix4.so to /usr/lib/librt.so
on Solaris 10, so no fix is required.

Comment 10

[Wan-Teh Chang]

Comment on attachment 149772 [details] [diff] [review]
always set SYSV_SPARC to 1

r=wtc.

>+SYSV_SPARC=1
> ifeq ($(SYSV_SPARC),1)
> SOLARIS_AS = /usr/ccs/bin/as

You might want to eliminate the ifeq ($(SYSV_SPARC),1)
test here because it always evaluates to true.

NSPR should still be fixed because the test using 'sort'
is broken.  We should either fix the test, or not do the
test at all (and rely on the libposix4.so symlink).

Comment 11

[Julien Pierre]

Attachment: patch as checked in

Checked in to NSS_3_9_BRANCH :

Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.53.14.2; previous revision: 1.53.14.1
done

And to the tip :

Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.56; previous revision: 1.55
done

Comment 12

[Julien Pierre]

Wan-Teh,

I removed the SYSV_SPARC test in the patch I checked in .
As far as NSPR goes, I'm not sure if the symlink exists on every other version
of Solaris between 5.6 and 5.10 . In particular, I don't have access to a 5.7
machine. So, I don't know if we can always rely on the symlink. I tried to put
together a fix, but I don't know gmake well enough.

I think it can be done by separating the minor and the major in the OS version,
and testing them separately. In that case, 10 will be greater than 5.1 and
things will be OK. I don't know how to split the minor and major, however.

Comment 13

[Wan-Teh Chang]

Comment on attachment 149786 [details] [diff] [review]
patch as checked in 

Julien, I should have been more verbose.

You can only remove the SYSV_SPARC test
that I pointed out in comment 10.  You
should not remove the SYSV_SPARC=1
statement and the other SYSV_SPARC test
in the makefile, otherwise it will use
the SPARC assembly code on Solaris x86
systems (I think).

I will take care of fixing NSPR.  Since
Solaris 10 has the libposix4.so symlink,
fixing NSPR is less urgent.

Comment 14

[Wan-Teh Chang]

Comment on attachment 149786 [details] [diff] [review]
patch as checked in 

Looking at this patch closer, I think it won't break
Solaris x86.  The rules for the SPARC assembly code
files will be ignored on other platforms because the
targets of those rules don't need to be built.

There is a minor issue though: this dependency

  $(OBJDIR)/mpmontg.o: mpmontg.c montmulf.h

is only valid for Solaris SPARC.  However, the effect
is that mpmontg.c is unnecessarily recompiled on
non-Solaris SPARC platforms when montmulf.h changes.
Not a big deal.

In any case, I think it is safer to restore the
SYSV_SPARC = 1 statement and the second SYSV_SPARC
test.

Comment 15

[Julien Pierre]

Attachment: incremental patch

Comment 16

[Julien Pierre]

Wan-Teh,

I took care of your issues and checked in the fix with your suggested change. I
agree that the NSPR change is not urgent since nothing breaks. Also, it is a
different issue than in this bug, so I'm marking it fixed.

NSS_3_9_BRANCH :
Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.53.14.3; previous revision: 1.53.14.2
done

tip :

Checking in Makefile;
/cvsroot/mozilla/security/nss/lib/freebl/Makefile,v  <--  Makefile
new revision: 1.57; previous revision: 1.56
done

Comment 17

[Wan-Teh Chang]

Attachment: NSPR patch

Simply enumerate all the Solaris versions <= 2.6.