125819 - unable to compile using gcc on OSF/1

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P4)

Description

[tilps]

When using gcc 3.0.3 - with OSF/1 4.0d
Compilation fails at 
nsprpub/pr/src/md/unix/uxrng.c line 91

use of <c_asm.h> - in a gcc compiler seems to be a problem.  gcc's 'asm' 
appears to overide the declared asm in c_asm.h
use of <machine/builtins.h> and replacing
t = asm("rpcc %v0");
with
t = __RPCC();

works fine.  

Other fixes probably exist.

Comment 1

[Wan-Teh Chang]

Chris, could you look into this?

If we can confirm that __RPCC() is equivalent
to asm("rpcc %v0"), we should use __RPCC().
It looks like it's equivalent.  If not, we
will need to do #ifdef __GNUC__ and use the
gcc equivalent (__asm__ or __asm?).

Comment 2

[hacker formerly known as [e-mail]]

I have no idea if __RPCC exists.  And I can't log into dec1.  shanmu?

Comment 3

[Shanmugavelu Shanmuganathan (gone)]

Chris,
   __RPCC  exists in dec1, which is a Tru64 UNIX 5.0A.

Comment 4

[tilps]

hmm - 'works fine' - wasnt quite actually correct.  somehow gcc is defineing
DECC or DECCXX - since otherwise it shouldnt have access to the headers in
machine/builtins.h - they are builtins for the dec compiler - result in
unresolved symbols on GCC.

This happens to also apply to use of machine/builtins.h in (from memory) _osf1.h
- where its used to define atomic incriment type functions.  Also resulting in
undefined symbols on gcc.

Comment 5

[hacker formerly known as [e-mail]]

Ok, I managed to get into dec1 and compiled gcc 3.0.4.  I ran into some other
problems that appear to be 5.0/gcc specific.  I had to define __V40_OBJ_COMPAT
in order for md/unix/unix.c to compile.

Changing asm(..) to __asm(..) doesn't fix the problem.

OSF1_HAVE_MACHINE_BUILTINS_H comes from configure.in.  It seems to only be
conditional upon the presence of the header.

Substituting __RPCC(); for asm("rpcc %v0"); makes the compile finish (even
tests) but I can't verify the correctness of such a change.

Comment 6

[hacker formerly known as [e-mail]]

Attachment: Use V40_OBJ format for V5.0 builds & replace asm() with __RPCC

Comment 7

[tilps]

my 4.0d box had a licence expiration problem - and some hard drive fullness 
problems - so its been in limbo for a bit, but things are comming round again, 
will see if i still get unresolved symbols using __RPCC with your patch, once i 
grab the source and start reapplying my patches again. (I will be using gcc cvs 
HEAD (3.2) - since I compiled that trying to fix a non mozilla related problem 
with my system - but hopefully that wont affect anything significantly.)

Comment 8

[tilps]

Okay - my system is back up and running, your patch compiles, but when I try to
run mozilla which is compiled with it, 
31094:./viewer: /sbin/loader: Error: Unresolved symbol in ./libnspr4.so: __RPCC
31094:./viewer: /sbin/loader: Error: Unresolved symbol in ./libnspr4.so:
__ATOMI_INCREMENT_LONG
31094:./viewer: /sbin/loader: Error: Unresolved symbol in ./libnspr4.so:
__ATOMI_EXCH_LONG
31094:./viewer: /sbin/loader: Error: Unresolved symbol in ./libnspr4.so:
__ATOMI_DECREMENT_LONG
31094:./viewer: /sbin/loader: Error: Unresolved symbol in ./libnspr4.so:
__ATOMI_ADD_LONG

is the result. (Also theres a list of other unresolveds from libxpcom.so dueto
gcc3 stuff, but that isnt relevant).
I get the idea that these are supported natively internally with the cc compiler
on OSF/1 4.0d - possibly 5.1 puts them in the c library instead?

Comment 9

[tilps]

umm - scratch that - I forgot to regenerate configure from configure.in

Comment 10

[tilps]

Okay ... back again - since my autoconf likes to stuff things up nastily - so
everything defaults to cxx and cc instead of g++ and gcc (and then misinterprets
its output as well)
I edited autoconf.mk after configure to remove the OSF1_HAVE_MACHINE_BUILTINS_H
define.  Which I assume is equivelent to the configure.in change.

This fixes the _ATOMIC_INCRIMENT_LONG etc being undefined.
__RPCC however is not fixed. and is still undefined.
not sure what to do instead, no experience with __asm__ here.

Comment 11

[tilps]

hmm teaching myself __asm__

how about 
#ifdef __GNUC__
__asm__("rpcc %0" : "=r" (t));
#else
t = __RPCC(); 
#endif
?
This seems to be working for me so far. 

Comment 12

[hacker formerly known as [e-mail]]

That compiles for me as well.  No idea if it works though.

Comment 13

[hacker formerly known as [e-mail]]

Attachment: Updated patch

This patch includes the __asm__ usage (silly me only trying _asm()) but I left
the existing asm() call for the non-gcc case as I don't know enough to
authoritatively say __RPCC is the way to go.

Comment 14

[Wan-Teh Chang]

Comment on attachment 78317 [details] [diff] [review]
Updated patch

1. It is a shame that we can't use <machine/builtins.h> with
any compilers other than DEC C or C++.	This means gcc-compiled
binaries will not take advantage of Alpha's atomic instructions.
Ideally we should implement the atomic routines in Alpha assembly
code for gcc.

2. Defining __V40_OBJ_COMPAT is the wrong solution to the stat
and fstat problem.  I must say this is a difficult problem to
solve and I only came up with one solution after studying
the <sys/stat.h> header file for a long time.

The idea of my solution is outlined in the patch file below.
It needs to be polished.

Index: unix.c
===================================================================
RCS file: /cvsroot/mozilla/nsprpub/pr/src/md/unix/unix.c,v
retrieving revision 3.46
diff -u -r3.46 unix.c
--- unix.c	11 Apr 2002 18:34:17 -0000	3.46
+++ unix.c	16 Apr 2002 01:25:34 -0000
@@ -2727,6 +2727,16 @@
 }  /* _MD_Unix_mmap64 */
 #endif /* defined(_PR_NO_LARGE_FILES) || defined(SOLARIS2_5) */

+static int my_stat(const char *path, struct stat *buffer)
+{
+    return stat(path, buffer);
+}
+
+static int my_fstat(int filedes, struct stat *buffer)
+{
+    return fstat(filedes, buffer);
+}
+
 static void _PR_InitIOV(void)
 {
 #if defined(SOLARIS2_5)
@@ -2771,8 +2781,8 @@
 #elif defined(_PR_HAVE_LARGE_OFF_T)
     _md_iovector._open64 = open;
     _md_iovector._mmap64 = mmap;
-    _md_iovector._fstat64 = fstat;
-    _md_iovector._stat64 = stat;
+    _md_iovector._fstat64 = my_fstat;
+    _md_iovector._stat64 = my_stat;
     _md_iovector._lseek64 = lseek;
 #else
 #error "I don't know yet"

The stat and fstat problem comes about because <sys/stat.h>
declares _F64_stat and _F64_fstat and define stat(a, b) and
fstat(a, b) as macros for gcc.	So if you call stat or fstat,
you will be fine.  On the other hand, if you simply mention
stat or stat, without the parentheses and function arguments,
macro expansion won't happen, and you end up with stat and
fstat undeclared.

My solution is to implement two functions that call stat and
fstat.	Chris, I hope you agree that defining __V40_OBJ_COMPAT
is not the right way to solve this problem.

3. The change to uxrng.c is good.

Comment 15

[Wan-Teh Chang]

Comment on attachment 78317 [details] [diff] [review]
Updated patch

Sorry, I checked the wrong box.  This patch needs work.

Comment 16

[Wan-Teh Chang]

Attachment: Updated patch v3

This patch has a different solution for the stat
and fstat problem (see comment #14).

Comment 17

[Wan-Teh Chang]

Patch checked into the tip and NSPRPUB_PRE_4_2_CLIENT_BRANCH
of NSPR.

Future Work

1. I ran the debug build through the NSPR test suite.  All the
tests passed except dtoa: it gets SIGFPE when trying to divide
by 0 (bug 151711).

2. With this patch, we are not taking advantage of Alpha's
atomic instructions when compiling with gcc.  I filed
a RFE for Alpha assembly implementation of the NSPR atomic
routines (bug 151709).

Comment 18

[tilps]

Thought i might point out that I believe i noticed some noise on the gcc
mailinglist about adding some alpha builtins to its support. Dont remember if it
was only alpha/linux though.

From what i saw, the syntax reminded me of the builtins in machine/builtins.h in
OSF.

However since i worked out that i could upgrade the dec c++ compiler on this
machine for free, and have been able to build mozilla using it, gcc issues arent
an itch i am going to be scratching much anymore. (well at least not for a while :P)