333925 - NSS Solaris build compiled with Forte 6 update 2 fails the "AES ECB Decrypt" and "AES CBC Decrypt" tests in cipher.sh; and may not run on Ultrasparc I and II

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Wan-Teh Chang]

NSS trunk (3.12) and NSS_3_11_BRANCH (3.11.1) builds compiled
with Forte 6 update 2 on Solaris 9 fail the "AES ECB Decrypt"
and "AES CBC Decrypt" tests in cipher.sh.

Compiler info:
cc: Sun WorkShop 6 update 2 C 5.3 2001/05/15

System info ("uname -a" output):
SunOS cyclone 5.9 Generic_112233-05 sun4u sparc SUNW,Ultra-5_10

Comment 1

[Wan-Teh Chang]

Attachment: results.html

Comment 2

[Wan-Teh Chang]

Attachment: Subset of output.log generated by cipher.sh

Comment 3

[Julien Pierre]

Wan-Teh,

I don't think we are using this compiler anymore for anything. I'm not sure if it's still supported. Copying Christophe.

Comment 4

[Wan-Teh Chang]

The problem is in the Solaris compiler flags in the
USE_ABI32_FPU section of lib/freebl/Makefile:

        ifdef USE_ABI32_FPU
            # this builds for Sparc v8+a ABI32_FPU architecture, 64-bit registers,
            # 32-bit ABI, it uses FPU code, and 32-bit word size.
            # these flags were determined by running cc -### -fast and copying
            # the generated flag settings
            SOL_CFLAGS += -D__MATHERR_ERRNO_DONTCARE -fns -fsimple=2 -fsingle
            SOL_CFLAGS += -xalias_level=basic -xbuiltin=%all
            SOL_CFLAGS += -xcache=64/32/4:1024/64/4 -xchip=ultra3 -xdepend
            SOL_CFLAGS += -xlibmil -xmemalign=8s -xO5
            ARCHFLAG = -xarch=v8plusa
            SOLARIS_AS_FLAGS = -xarch=v8plusa -K PIC
        endif

The problematic compiler flag seems to be -xchip=ultra3.  If I
change that to -xchip=ultra2 and only recompile lib/rijndael.c,
the two AES tests pass.  I don't know why.

One possible fix is to simply set SOL_CFLAGS to -fast or
-xchip=ultra2 (which is what NSS 3.10 or older uses) if we
detect that the Solaris compiler version is Forte 6 (or older
than Sun Studio 10).

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Neil, you're the Makefile guy for freebl.
Please plan to resolve this for 3.11.1.

Comment 6

[Julien Pierre]

Wan-Teh,

Do you absolutely need to keep building with this old compiler ?
I think when Neil did the work last summer to find the best flags for studio, the implication was that support for the forte 6 update 2 compiler was dropped in NSS 3.11 .

On the other hand, we did not drop support for pre-Ultrasparc III CPUs. The libfreebl3_abi32_fpu.so library gets used on any Ultrasparc CPU that supports VIS, including an Ultrasparc I and II. It's possible that -xchip=ultra3 will cause the compiler to generate code that won't run properly or at all on the earlier chips. So, we shouldn't use that option. I think we should use -xchip=ultra . That should kill two birds with one stone - hopefully binaries made with Forte 6 will work, and support for Ultra I and II will be ensured.

Comment 7

[Julien Pierre]

Wan-Teh,

Re: comment 4 :
Using -fast is bad because it optimizes for the local CPU of the build machine. Unless the build machine is the lowest common denominator that we officially support (for this library, an Ultrasparc I), the binaries may not run on other systems. So, we intentionally got rid of -fast in 3.11 . Unfortunately the replacement is incorrect.

Comment 8

[Julien Pierre]

Attachment: switch to -xchip=ultra

Comment 9

[Wan-Teh Chang]

Julien,

It is -xarch that selects the instruction set.
-xchip affects instruction scheduling only, so
I believe the code generated with -xchip=ultra3
will run on UltraSPARC I and II processors.

We have Forte 6 update 2 and Sun Studio 11, but
we haven't compiled any binary components with
Sun Studio 11 yet.  So it's easier for us to
continue to use Forte 6 update 2 for NSS 3.11.x.

Can you ask Sun's compiler team if Forte 6 update
2 has any bug with -xchip=ultra3?  Thanks.

Comment 10

[Wan-Teh Chang]

Attachment: Solution 1: use -xchip=ultra2 to compile lib/freebl/rijndael.c

This is the minimal patch.  My experiments showed that we
can pass all the tests in all.sh with Forte 6 update 2 by
just compiling one file, lib/freebl/rijndael.c, with
-xchip=ultra2 instead of -xchip=ultra3.  I don't know why.

I will attach another patch that compile the whole lib/freebl
directory with -xchip=ultra2 instead of -xchip=ultra3.  That
patch might be safer.

Comment 11

[Wan-Teh Chang]

Attachment: Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl

I examined all the compiler optimization flags we are using
for the ABI32_FPU and ABI64_FPU architectures, and identified
the two flags (-xchip and -xcache) that specify (properties of)
the target processor.

I define a new variable FPU_TARGET_OPTIMIZER to hold these
compiler flags, and set FPU_TARGET_OPTIMIZER to what we were
using in NSS 3.10 and earlier (-xchip=ultra2, with no -xcache
flag) if we are compiling with Forte 6.

Bob, please pick one of these two patches.  Thanks.

Comment 12

[Wan-Teh Chang]

Comment on attachment 219671 [details] [diff] [review]
Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl

I should point out that the -xcache=64/32/4:1024/64/4
flag does not cause problems.  The reason I don't use
it with Forte 6 is that we weren't using it in NSS 3.10
or earlier.

Comment 13

[Robert Relyea]

Comment on attachment 219671 [details] [diff] [review]
Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl

r+ = relyea

I prefer this patch, though I would like the sun team to comment since it's in Solaris part of the code and effects their products more than ours.

bob

Comment 14

[Wan-Teh Chang]

Comment on attachment 219671 [details] [diff] [review]
Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl

Julien, Neil,

Please see comment 11 and comment 12 for a description
of this patch.

Sun Studio 11 C compiler man page
(http://docs.sun.com/source/819-3688/cc_ops.app.html)
says:

  B.2.77 -xchip[=c]

  Specifies the target processor for use by the optimizer.

  [...]
 
  This option specifies timing properties by specifying
  the target processor.

  Some effects are:

      * The ordering of instructions, that is, scheduling

      * The way the compiler uses branches

      * The instructions to use in cases where
        semantically equivalent alternatives are available 

I am a little worried about the third (last) bullet
item.  Perhaps that's why Julien thinks -xchip=ultra3
may generate code that won't run on UltraSPARC I and II
CPUs?  But in this bug, the generated code doesn't crash;
it's just incorrect.

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

Reassigning this bug to Christophe.  This bug is a stopper for NSS 3.11.1.

Comment 16

[Wan-Teh Chang]

Attachment: Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl, v2

The only change from v1 is that I improved the comments.

Please see comment 11 and comment 12 for a description
of this patch.

Comment 17

[Neil Williams]

Sun Studio 11 cc -V returns "cc: Sun C 5.8 2005/10/13" which, I think, means this patch would use the 3.10 flags when built with Studio 11. Most of us Sun Solaris developers use Studio 11. Should we change this to check for Forte and make the else case set xcache and xchip?

Comment 18

[Wan-Teh Chang]

Comment on attachment 219688 [details] [diff] [review]
Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl, v2

>+	ifeq (,$(findstring Sun WorkShop 6,$(shell $(CC) -V 2>&1)))
             ^^^

Neil,

This if statement means "if the output of $(CC) -V does NOT
contain Sun WorkShop 6".  Note the comma right after the
first left parenthesis.  We are comparing the value of the
findstring function with the empty string.  If findstring
returns an empty string, it means the string is not found.

I can use ifneq instead, or add a comment to paraphrase the
if statement.  Do you think that will help?

Comment 19

[Julien Pierre]

In response to comment 14, yes, the third bullet in man page for cc -xchip was my concern about the compiler generating code that may not run on older CPUs. We need to get someone in the compiler team to answer about that. In the meantime, we can check binaries.

However, I don't think the code would ever crash, it would simply not run, because Solaris marks binaries for the minimum instruction set required. So the loader would simply fail if it selected a library incompatible with the machine. You can check the binaries in Solaris 10 with the "file" command :

% file libfreebl_32fpu_3.so
libfreebl_32fpu_3.so:   ELF 32-bit MSB dynamic lib SPARC32PLUS Version 1, V8+ Required, UltraSPARC1 Extensions Required, dynamically linked, not stripped

% file libfreebl_32int_3.so
libfreebl_32int_3.so:   ELF 32-bit MSB dynamic lib SPARC Version 1, dynamically linked, not stripped

% file libfreebl_32int64_3.so
libfreebl_32int64_3.so: ELF 32-bit MSB dynamic lib SPARC32PLUS Version 1, V8+ Required, dynamically linked, not stripped

I think "Ultrasparc I extensions" is synonymous with VIS.

Comment 20

[Wan-Teh Chang]

The file command produced the same output on the binaries compiled
with Forte 6 update 2:

% file libfreebl_32fpu_3.so
libfreebl_32fpu_3.so:   ELF 32-bit MSB dynamic lib SPARC32PLUS Version 1, V8+
Required, UltraSPARC1 Extensions Required, dynamically linked, not stripped

% file libfreebl_32int_3.so
libfreebl_32int_3.so:   ELF 32-bit MSB dynamic lib SPARC Version 1, dynamically
linked, not stripped

% file libfreebl_32int64_3.so
libfreebl_32int64_3.so: ELF 32-bit MSB dynamic lib SPARC32PLUS Version 1, V8+
Required, dynamically linked, not stripped

Comment 21

[Wan-Teh Chang]

I checked in my patch "Solution 2" on the NSS trunk (3.12) and
the NSS_3_11_BRANCH (3.11.2).

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

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

We have installed Sun Studio 11 on our Solaris build machines at
Red Hat, but we can't switch to it right away.  When we start to
use Sun Studio 11 to build NSS at Red Hat, we can back this patch
out.

I don't have time to research whether code generated with the
-xchip=ultra3 flag will run on UltraSPARC I and II processors.

Comment 22

[Martin Schröder [:mschroeder]]

Comment on attachment 219688 [details] [diff] [review]
Solution 2: use -xchip=ultra2 and no -xcache flag to compile lib/freebl, v2

Removing review requests. Patch was already checked in a year ago.