Closed Bug 550428 Opened 11 years ago Closed 11 years ago

assembler errors on hppa-hpux (32bit) in os_HPUX.s using gcc with GNU-as

Categories

(NSPR :: NSPR, defect, P2)

Product:
NSPR
Component:
NSPR
Platform:
HP
HP-UX
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: michael.haubenwallner, Assigned: wtc)

References

Details

Attachments

(5 files, 2 obsolete files)

Pass -x assembler-with-cpp to gcc
781 bytes, patch
Details | Diff | Splinter Review
no need for "+I PR_HPUX10xInit" linker flag with gcc
410 bytes, patch
Details | Diff | Splinter Review
use .ENTRY/.EXIT to support GNU-as
517 bytes, patch
Details | Diff | Splinter Review
when using gcc, use gcc for linking too instead of ld directly
1.89 KB, patch
Details | Diff | Splinter Review
Patch for os_HPUX.s by Dennis Handly
770 bytes, patch
nelson
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.6) Gecko/20100112 Gentoo Shiretoko/3.5.6
Build Identifier: hppa2.0w-hp-hpux11.31 (32bit), gcc with gnu-as, nspr-4.8.3

Compiling os_HPUX.s with gcc (4.2.4) using GNU-as (2.19) for hppa2.0w-hp-hpux11.31 (32bit) does not work due to GNU-assembler not understanding pseudo-ops used.


Reproducible: Always

Steps to Reproduce:
1. Build some gcc using the GNU-as too, not just GNU-ld.
2. compile nspr with 'gmake CC=gcc'
3.
Actual Results:  
gcc -o os_HPUX.o      -pthread -g -fPIC  -UNDEBUG -DDEBUG_wie  -DDEBUG=1 -DXP_UNIX=1 -DHPUX=1 -D_HPUX_SOURCE=1 -D_PR_POLL_WITH_SELECT=1 -D_USE_BIG_FDS=1 -Dhppa=1 -DHAVE_POINTER_LOCALTIME_R=1 -DHPUX10=1 -DHPUX11=1 -D_LARGEFILE64_SOURCE=1 -D_PR_HAVE_OFF64_T=1 -DHAVE_FCNTL_FILE_LOCKING=1 -DHAVE_LCHOWN=1 -DHAVE_STRERROR=1 -D_POSIX_C_SOURCE=199506L -D_PR_HAVE_THREADSAFE_GETHOST=1  -DFORCE_PR_LOG -D_PR_PTHREADS -UHAVE_CVAR_BUILT_ON_SEM -D_PR_INET6 -D_NSPR_BUILD_ -I../../../../dist/include/nspr -I/buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/include -I/buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/include/private  -c /buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/src/md/unix/os_HPUX.s
/buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/src/md/unix/os_HPUX.s: Assembler messages:
/buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/src/md/unix/os_HPUX.s:39: Error: junk at end of line, first unrecognized character is `W'
/buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/src/md/unix/os_HPUX.s:52: Error: The .ENTER pseudo-op is not supported
/buildroot/nspr/nspr-4.8.3/mozilla/nsprpub/pr/src/md/unix/os_HPUX.s:56: Error: The .LEAVE pseudo-op is not supported
gmake[2]: *** [os_HPUX.o] Error 1
gmake[2]: Leaving directory `/buildroot/nspr/sasrvc.hppa2.0w-hp-hpux11.31/pr/src/md/unix'
gmake[1]: *** [export] Error 2
gmake[1]: Leaving directory `/buildroot/nspr/sasrvc.hppa2.0w-hp-hpux11.31/pr/src/md'


Expected Results:  
successful compilation.

The minor problem (line 39) is that gcc does not call the preprocessor first. Using '-xassembler-with-cpp' just before the filename would help here.

The major problem is that GNU-as does not support those pseudo-ops.

The reason I cannot (don't want to) use the native assembler is that I do have some cross-compilation setup, where I'm bound to GNU-as, even if this one compilation is done natively.

Any idea how to write this assembler file to be understood by GNU-as?

Thank you!
For the minor problem:
When the file ends with uppercase S (os_HPUX.S), then gcc runs the preprocessor first.

For the major problem:
When I do s/.ENTER/.ENTRY/ and s/.LEAVE/.EXIT/, both GNU and native assembler can compile this file.
However, there's a small difference in disassembly output then:
--- os_HPUX.o.orig
+++ os_HPUX.o
 00000000 <ret_cr16>:
    0:   e8 40 c0 00     bv r0(rp)
    4:   02 00 08 bc     mfctl itmr,ret0
-   8:   e8 40 c0 00     bv r0(rp)
-   c:   08 00 02 40     nop

IMVHO, 'bv' does the return, and 'mfctl' the work, so this might still be ok?

Thank you!
Have been able to build successfully using this patch.
> Have been able to build successfully using this patch.
What about run successfully?
Severity: enhancement → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Other → HP-UX
Hardware: Other → HP
There are two copies of this file in the source tree.  
The other is seen at
http://mxr.mozilla.org/security/source/security/nss/lib/freebl/ret_cr16.s
If you build/use NSS, then I believe you're actually using the copy in nss.

Wan-Teh, is HP PA-Risc 2.0 a FIPS platform?
Besides ret_cr16.s, in NSS-freebl there is also mpi/hpma512.s and mpi/hppa20.s.
Although I'm able to build them after similar tweaking, GNU 'objdump -d' shows interesting differences between HP-cc/as and gcc/GNU-as, like:
--- hpma512.cc.disasm
+++ hpma512.gcc.disasm
-   c4: 08 c4 06 1a     add r4,r6,r26
+   c4: 08 c4 06 3a     add r4,r6,r26

Different values but same mnemonics: feels like I get lost here - and really should figure out how to run the tests of both nspr and nss...

Thank you!
Comment on attachment 430600 [details] [diff] [review]
patch for nspr-4.8.3 to work with gcc (using GNU-as) on hpux

r=wtc.
Attachment #430600 - Flags: review+
Michael, please file a separate bug for the NSS-freebl patch. Thanks.
I checked in the -x assembler-with-cpp change on the
NSPR trunk (NSPR 4.8.5).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.270; previous revision: 1.269
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.274; previous revision: 1.273
done

I found the documentation of .ENTER/.LEAVE and .ENTRY/.EXIT,
and they don't seem to be exactly equivalent:

http://docs.hp.com/en/92432-90012/ch04s14.html
http://docs.hp.com/en/92432-90012/ch04s15.html

In particular, the doc says:

  When the Assembler encounters an .ENTER pseudo-operation,
  it generates an entry code sequence according to the
  parameters in the .CALLINFO directive for that procedure.
  Similarly, when the Assembler encounters a .LEAVE
  pseudo-operation, it generates an exit code sequence
  according to the parameters in the .CALLINFO directive
  for that procedure.

So, the entry code sequence generated by the assmebler may
be more than .ENTRY.  Similarly, the exit code sequence
generated by the assembler may be more than .EXIT.

So I didn't check in the os_HPUX.s change.

If you really can't get gcc to accept .ENTER/.LEAVE, I
guess you can use the HP-UX native compiler/assembler to
assemble os_HPUX.s and check in the generated assembly
code, along the line of what you did in comment 1.
Attachment #430600 - Attachment is obsolete: true
See also the documentation of .CALLINFO:
http://docs.hp.com/en/92432-90012/ch04s07.html

which says:

  The parameters in the .CALLINFO directive govern the generation
  of the Entry/Exit code sequence (except for SAVE_SP). However,
  if you use the .ENTRY and .EXIT directives, your code must
  provide the necessary Entry/Exit code sequences.

So, perhaps the difference you observed in comment 1 is the
"necessary Exit code sequence".
(In reply to comment #9)
> http://docs.hp.com/en/92432-90012/ch04s07.html

Ohw, thank you for the pointers!

> So, perhaps the difference you observed in comment 1 is the
> "necessary Exit code sequence".

Exactly - but my humble thought was:
There is the first (handwritten) "bv r0(rp)" already,
so the second (generated) one is useless.
As I'm not a pa-risc asm expert: is this wrong thought?

Reading the .ENTRY/.EXIT docs, and still under the impression that ret_cr16() does not contain much more than an exit code sequence (because the generated exit code sequence contains the same branch-command with a NOP instead), this is my fixed proposed patch for os_HPUX.s

Do you know by chance if shlibsign (built and executed within NSS) does call ret_cr16()?
While at it: another minor one not worth another bug IMO is that gcc supports constructors/destructors and therefore does not need PR_HPUX10xInit().
It does make a difference in the created sharedlib indeed when not adding the .LEAVE-generated commands when using .EXIT, even if they look much like the function itself.
Attachment #431324 - Attachment is obsolete: true
Michael, please elaborate on your comment 12.
Comment on attachment 431377 [details] [diff] [review]
use .ENTRY/.EXIT to support GNU-as

Michael, could you use the HP-UX native compiler to compile
os_HPUX.s with the command-line option -S, and attach the
output .s file here?

http://docs.hp.com/en/4310/options.htm#option-capS

You need to manually modify the generated
mozilla/nsprpub/pr/src/md/unix/Makefile to do that, and
you may need to add a -o option to specify the name of the
output file, because I'm afraid the default output file name
is also os_HPUX.s and will cause the compiler to overwrite
the input os_HPUX.s.

It may make more sense to spend your time on HP-UX for Itanium.
I may not check in all of your patches for HP-UX PA-RISC GCC
because some of them, such as this one, need a lot of time
to verify their correctness.

It would be nice to file a bug report for GCC to support
HP-UX's .ENTER/.LEAVE pseudo operations, but I'm afraid that
it won't be the best use of the GCC developers' time if
.ENTER/.LEAVE are only used for PA-RISC assembly.
Ehm, doing '/usr/bin/cc -S' when the input already is an assembler file does not generate any output.

Instead, I've compiled the original os_HPUX.s with "/usr/bin/cc -c" to os_HPUX.orig.o and disassembled this using GNU-objdump:
$ objdump -d os_HPUX.orig.o

os_HPUX.orig.o:     file format som


Disassembly of section $CODE$:

00000000 <ret_cr16>:
   0:   e8 40 c0 00     bv r0(rp)
   4:   02 00 08 bc     mfctl itmr,ret0
   8:   e8 40 c0 00     bv r0(rp)
   c:   08 00 02 40     nop

So .LEAVE adds "8:" and "c:", while .ENTER does not add anything here.

My first thought (comment#1, comment#10) was:
"0:" and "8:" are identical, so "0:" is the return-command already, while "4:" (because of parisc-pipe still executed after "0:") is the only valuable command (as "c:" is NOP). And because of "0:" doing the return already, "8:" and "c:" aren't executed anyway, so it shouldn't make a difference to omit them when using .ENTRY/.EXIT instead of .ENTER/.LEAVE.

But later in NSS, I disassembled libfreebl_32fpu_3.sl, and ret_cr16 was:
0006bfe8 <ret_cr16>:
   6bfe8:   e8 40 c0 00     bv r0(rp)
   6bfec:   02 00 08 bc     mfctl itmr,ret0
   ...
Interesting here is the literal "...", the "difference" mentioned as the reason for comment#12.

Strange enough, disassembling libfreebl_32fpu_3.sl now (didn't recheck before comment#12 unfortunately) shows:
0004e7a8 <ret_cr16>:
   4e7a8:   e8 40 c0 00     bv r0(rp)
   4e7ac:   02 00 08 bc     mfctl itmr,ret0
   4e7b0:   e8 40 c0 00     bv r0(rp)
   4e7b4:   08 00 02 40     nop
    ...
Even more strange: disassembling libnspr4.sl does not show the '...', neither with nor without the additional "bv+nop".
Ohw, libnspr4.sl is still created using "/usr/bin/ld -b" instead of "gcc -shared" (have to create another patch) - but this doesn't make any difference, the '...' is never shown when disassembling libnspr4.sl.
So basically my first thought may still be valid - especially because when creating both ret_cr16.o and libfreebl_32fpu_3.sl with HP-cc, the '...' is shown too (haven't checked that before either).

Anyway, throwing away my first thought for optimization: With patch from comment#12, ret_cr16 is created with identical opcodes by both native- and GNU-as.

Huh (wth?) - there's this line missing in that patch, sorry:
@@ -49,10 +49,12 @@

And yes, I'll come to Itanium HP-UX once that machine does work again here.

Thank you for your patience and attention!
FWIW, I agree that the second pair of instructions (bv, nop) is useless to
the CPU, which executes only the first two.  It MIGHT be that the OS or some
debugger expects to see a particular sequence of instructions, and so leaving
the extra instructions there serves some purpose OTHER THAN having the CPU 
itself execute properly.  But I'd be happy for you to remove those extra two 
until some need for them can be demonstrated.
(In reply to comment #15)
> Ohw, libnspr4.sl is still created using "/usr/bin/ld -b" instead of "gcc
> -shared" (have to create another patch)
Michael: you should consider using GCC inline assembly code.
I found two examples of GCC inline assembly code for reading
the CR registers.

1. The mfctl() function in Linux source code:
http://lxr.linux.no/linux+v2.6.33/arch/parisc/include/asm/system.h

mfctl(16) reads cr16.

2. Code to read cr27:
http://wiki.parisc-linux.org/ThreadLocalStorage

3. http://www.ecrypt.eu.org/stream/svn/viewcvs.cgi/ecrypt/trunk/test/timers.h?rev=147&view=auto
Compiling this c-file containing inline assembly:
    $ cat > retcr16.c <<-EOF
    unsigned long ret_cr16(void)
    {
        unsigned long cr;
        __asm__ __volatile__(
                "mfctl 16,%0" :
                "=r" (cr)
        );
        return cr;
    }
    EOF
using gcc creates this assembler output (needs optimization turned on):
    $ gcc -S -O1 retcr16.c -o -
        .LEVEL 1.1
        .SPACE $PRIVATE$
        .SUBSPA $DATA$,QUAD=1,ALIGN=8,ACCESS=31
        .SUBSPA $BSS$,QUAD=1,ALIGN=8,ACCESS=31,ZERO,SORT=82
        .SPACE $TEXT$
        .SUBSPA $LIT$,QUAD=0,ALIGN=8,ACCESS=44
        .SUBSPA $CODE$,QUAD=0,ALIGN=8,ACCESS=44,CODE_ONLY
        .IMPORT $global$,DATA
        .IMPORT $$dyncall,MILLICODE
        .SPACE $TEXT$
        .NSUBSPA $CODE$
        .align 4
        .EXPORT ret_cr16,ENTRY,PRIV_LEV=3,RTNVAL=GR
    ret_cr16:
        .PROC
        .CALLINFO FRAME=0,NO_CALLS
        .ENTRY
        mfctl 16,%r28
        bv,n %r0(%r2)
        .EXIT
        .PROCEND
So there seems to be no need for the extra "bv+nop" indeed.
Thank you!
Michael,

Actually I meant that with GCC, you can skip the ret_cr16 function
and just use inline assembly in the GetHighResClock function:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/src/md/unix/uxrng.c&rev=1.23&mark=85,88#81

But you still need to solve the .ENTER/.LEAVE problem for the
large PA-RISC assembly code files in NSS.
Well, I don't like to use inline assembly for one compiler and an extra .s file for another compiler.

For the ".ENTER/.LEAVE problem" I'm not sure what exactly you mean:

For the "..." in the GNU-objdump disassembly output of the sharedlib: I'd like to ignore them, because this occurs with other functions generated from plain C code too...

For the binary difference wrt "add":
Either the difference can be ignored, or it is a GNU-as bug:
http://sourceware.org/bugzilla/show_bug.cgi?id=11395
So there's nothing we can do within NSS here.
I consulted Dennis Handly of HP, who helped me with HP-UX assembly
code issues before.  He told me to use this patch.  I know nothing
about PA-RISC assembly code, but I trust Dennis Handly.
Attachment #433112 - Flags: review?(nelson)
Comment on attachment 433112 [details] [diff] [review]
Patch for os_HPUX.s by Dennis Handly

Looks good to me.  If this satisfies Michael, let's do it.
Attachment #433112 - Flags: review?(nelson) → review+
Fine with me, thank you!
Blocks: 556252
Bug 550428: assembler errors on hppa-hpux (32bit) using gcc with GNU-as
Patch contributed by Dennis Handly. r=wtc, nelson

Checking in mozilla/nsprpub/pr/src/md/unix/os_HPUX.s;
new revision: 1.5; previous revision: 1.4
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.8.5
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.