Closed Bug 268524 Opened 15 years ago Closed 15 years ago

mozilla is unusable in solaris x86 using Sun SOS10 compiler.

Categories

(Core :: XPCOM, defect)

x86
Solaris
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: leon.sha, Assigned: leon.sha)

References

()

Details

Attachments

(4 files, 3 obsolete files)

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
The problem happens here:
source/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_solaris.cpp

Vijay.Tatkar@Sun.COM:

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
Status: NEW → ASSIGNED
Attached file template file (obsolete) —
I still do not know how to change the make file yet.
Attached patch Patch with timeless's coments (obsolete) — Splinter Review
Attachment #165239 - Attachment is obsolete: true
Attachment #165240 - Attachment is obsolete: true
Attached file Asm file
Attachment #165255 - Flags: review?(darin)
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.
(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.
> 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.
(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.
Attached patch Patch v2Splinter Review
Attachment #165255 - Attachment is obsolete: true
Attachment #165255 - Flags: review?(darin)
Comment on attachment 165510 [details] [diff] [review]
Patch v2

I got response from Rich.Burridge@Sun.COM(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.
Attachment #165510 - Flags: review?(darin)
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!
Attachment #165510 - Flags: review?(darin) → review+
Attachment #165517 - Flags: superreview?(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)"
Attachment #165517 - Flags: superreview?(Henry.Jia) → superreview+
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.