268524 - mozilla is unusable in solaris x86 using Sun SOS10 compiler.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Leon Sha]

We try to build mozilla using Sun SOS10.
But the mozilla is unusable.
We can only see the ui but can do nothing.
You can find the Compiler information here:
http://dptwiki.sfbay/twiki/bin/view/Vulcan/SunStudio10FAQ#What_is_in_Sun_Studio_10_and_why

Also you can download the test build here:
ftp://ftp.mozilla.org/pub/mozilla.org/mozilla/releases/mozilla1.8a4/contrib/mozilla-i386-pc-solaris2.8-mozilla1.8a4-testonlyforsunSOS10.tar.bz2

Comment 1

[Leon Sha]

The problem happens here:
source/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_solaris.cpp

[e-mail]:

If the function return value is needed, then we have found the problem, it is a
user error:

int foo()
{
   asm(...
       call bar
       ...);
          if (0) return 0;
}

Since asm() is not a call to a function but inlined assmebly, the compiler would
not know the return value of bar to be in %eax. SOS8 works just by luck, because
the compiler does not return anything for the function, so the register %eax
retains the return value of bar.  SOS10 on the otherhand, returns an undefined
allocated value on the stack, since the source said "int foo()", but nothing is
really returned.

One way to make this really work is by putting the content of asm() into a .il
inline template file, give it a name ".inline XX", then "call" XX from foo(). 
The compiler will inline it.

extern "C" { int XX() }

int foo()
{
   return XX();
}

where XX() is in an asm.il file   .inline XX,0
  <your assembly>
  .end
  Compile your file with
   CC -c asm.il file.c

Comment 2

[Leon Sha]

Attachment: Patch for xptcinvoke_x86_solaris.cpp

Comment 3

[Leon Sha]

Attachment: template file

Comment 4

[Leon Sha]

I still do not know how to change the make file yet.

Comment 5

[Leon Sha]

Attachment: Patch with timeless's coments

Comment 6

[Leon Sha]

Attachment: Asm file

Comment 7

[Darin Fisher]

Comment on attachment 165255 [details] [diff] [review]
Patch with timeless's coments

>Index: xptcinvoke_x86_solaris.cpp

>-static PRUint32
>+PRUint32

>-static void
>+void

perhaps these should remain static in the other case.  static
function calls are typically much cheaper than non-static 
function calls.  at least, i'm pretty sure that is true of x86.


otherwise, i'm fine with this patch, but i don't really profess
to know much about this code.  have you looked through the cvs
history to see who created this file in the first place?  might
be good to get their input.

Comment 8

[Leon Sha]

(In reply to comment #7)
> (From update of attachment 165255 [details] [diff] [review])
> >Index: xptcinvoke_x86_solaris.cpp
> 
> >-static PRUint32
> >+PRUint32
> 
> >-static void
> >+void
> 
> perhaps these should remain static in the other case.  static
> function calls are typically much cheaper than non-static 
> function calls.  at least, i'm pretty sure that is true of x86.
> 
> 

Static functions are only visable to other functions in the same file.

> otherwise, i'm fine with this patch, but i don't really profess
> to know much about this code.  have you looked through the cvs
> history to see who created this file in the first place?  might
> be good to get their input.
> 
This file has been unchanged since 2001. I'll try to contact the person.

Comment 9

[Darin Fisher]

> Static functions are only visable to other functions in the same file.

Indeed, and they are also faster to call.  So, it might make sense to use an
#ifdef to only remove the static keyword when you are linking against the
standalone assembly file.

Comment 10

[Leon Sha]

(In reply to comment #9)
> > Static functions are only visable to other functions in the same file.
> 
> Indeed, and they are also faster to call.  So, it might make sense to use an
> #ifdef to only remove the static keyword when you are linking against the
> standalone assembly file.
Yes, you are correct.
I'll add this.

I have asked the original author rich.burridge%sun.com for input.
But I doubt that he can help.
This file has been unchanged for more than 3 years.

Comment 11

[Leon Sha]

Attachment: Patch v2

Comment 12

[Leon Sha]

Comment on attachment 165510 [details] [diff] [review]
Patch v2

I got response from [e-mail](The orignin author). He point me to
our compiler team, where I got the solution at the beginning. I have tried this
on both solaris 8,9,10. It works. Also I have tried build this with SOS8 and
SOS10, there is no problem.

Comment 13

[Darin Fisher]

Comment on attachment 165510 [details] [diff] [review]
Patch v2

>Index: xptcinvoke_x86_solaris.cpp

>+#if defined(__SUNPRO_CC)               /* Sun Workshop Compiler. */
>+PRUint32
>+#else
> static PRUint32
>+#endif
> invoke_count_words(PRUint32 paramCount, nsXPTCVariant* s)

Sorry to be so nit-picky, but could you write this like this instead?

#if !defined(__SUNPRO_CC)		/* Sun Workshop Compiler. */
static
#endif
PRUint32
invoke_count_words(PRUint32 paramCount, nsXPTCVariant* s)


>+#if defined(__SUNPRO_CC)               /* Sun Workshop Compiler. */
>+#else

This should also be "#if !defined(__SUNPRO_CC)"


r=darin with those changes.  Thanks!

Comment 14

[Leon Sha]

Attachment: Patch with darin's coments

Comment 15

[Henry Jia]

Comment on attachment 165517 [details] [diff] [review]
Patch with darin's coments

sr=Henry

Pls make the following change according to darin's comment.

>+#if defined(__SUNPRO_CC)               /* Sun Workshop Compiler. */
>+#else

This should also be "#if !defined(__SUNPRO_CC)"

Comment 16

[Leon Sha]

Attachment: Patch with henry's comment. Ready to check in.

Comment 17

[Ginn Chen]

Checking in Makefile.in;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/Makefile.in,v  <--  Makefile.in
new revision: 1.78; previous revision: 1.77
done
RCS file:
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_x86_solaris_SUNW.s,v
done
Checking in xptcinvoke_asm_x86_solaris_SUNW.s;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_x86_solaris_SUNW.s,v
 <--  xptcinvoke_asm_x86_solaris_SUNW.s
initial revision: 1.1
done
Checking in xptcinvoke_x86_solaris.cpp;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_solaris.cpp,v
 <--  xptcinvoke_x86_solaris.cpp
new revision: 1.3; previous revision: 1.2
done