Last Comment Bug 10061 - xptcall not quite right on sgi irix 6.5
: xptcall not quite right on sgi irix 6.5
Status: VERIFIED FIXED
: helpwanted
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: SGI IRIX
: P3 normal with 1 vote (vote)
: Future
Assigned To: Mike Shaver (:shaver -- probably not reading bugmail closely)
: Robert Ginda
Mentors:
: 11420 (view as bug list)
Depends on:
Blocks: 59219
  Show dependency treegraph
 
Reported: 1999-07-17 09:31 PDT by Tomi Leppikangas
Modified: 2001-01-08 15:10 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch: fix for Irix float handling in xptcinvoke_irix.cpp. (1.05 KB, patch)
2000-12-10 15:20 PST, Robert Low
no flags Details | Diff | Splinter Review

Description Tomi Leppikangas 1999-07-17 09:31:31 PDT
TestXPTCInvoke fails on SGI Irix 6.5. Float test gives wrong result,
also apprunner dies. Compiled with native compiler, viewer work ok.

Output from test:

indy1:(/var/tmp/tomi/mozilla/obj-sgi-cc/xpcom/reflect/xptcall/tests)(181)%
./TestXPTCInvoke
calling direct:
        1 + 1 = 2
        1L + 1L = 2
        2 * 2 = 4
        2L * 2L = 4
        1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 = 55.000000
        1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 = 55.000000f
calling via invoke:
        1 + 1 = 2
        1L + 1L = 2
        2 * 2 = 4
        2L * 2L = 4
        1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 = 55.000000
        1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 = 7.593750f

impl == 1001b798
foo  == 1001b798
bar  == 1001b7a8
Calling Foo...
direct calls:
        FooImpl::FooMethod1 called with i == 1, FooImpl part of a FooBarImpl
        FooImpl::FooMethod2 called with i == 2, FooImpl part of a FooBarImpl
invoke calls:
        FooImpl::FooMethod1 called with i == 1, FooImpl part of a FooBarImpl
        FooImpl::FooMethod2 called with i == 2, FooImpl part of a FooBarImpl

Calling Bar...
direct calls:
        BarImpl::BarMethod1 called with i == 1, BarImpl part of a FooBarImpl
        BarImpl::BarMethod2 called with i == 2, BarImpl part of a FooBarImpl
invoke calls:
        BarImpl::BarMethod1 called with i == 1, BarImpl part of a FooBarImpl
        BarImpl::BarMethod2 called with i == 2, BarImpl part of a FooBarImpl


impl == 10019a18
foo  == 10019a18
bar  == 10019a1c
Calling Foo...
direct calls:
        FooBarImpl2::FooMethod1 called with i == 1, local value = 12345678
        FooBarImpl2::FooMethod2 called with i == 2, local value = 12345678
invoke calls:
        FooBarImpl2::FooMethod1 called with i == 1, local value = 12345678
        FooBarImpl2::FooMethod2 called with i == 2, local value = 12345678

Calling Bar...
direct calls:
        FooBarImpl2::BarMethod1 called with i == 1, local value = 12345678
        FooBarImpl2::BarMethod2 called with i == 2, local value = 12345678
invoke calls:
        FooBarImpl2::BarMethod1 called with i == 1, local value = 12345678
        FooBarImpl2::BarMethod2 called with i == 2, local value = 12345678



Machine info:
> uname -a
IRIX indy1 6.5 04151556 IP22
> cc -version
MIPSpro Compilers: Version 7.2.1.3m
Comment 1 John Bandhauer 1999-07-25 02:12:59 PDT
setting as assigned...
This looks like a problem specific to the handling of floats. I've been
exchanging email with the guy who actually wrote the code - Jason Heirtzler
<jasonh@cthulhu.engr.sgi.com>. He is going to look into it.
Comment 2 John Bandhauer 1999-10-18 13:10:59 PDT
I send another email to Jason.

Let's see if shaver really wants to own all the xptcall Unix bugs?
Comment 3 John Bandhauer 1999-10-18 13:40:59 PDT
*BEEP*  *BEEP*  *BEEP*

This is an automatic reply to your email.

Your email message to jasonh@sgi.com has
been sent to my old address.

For personal email only, please use

        jheirtzler@excite.com

Sorry, I will not be reading email at SGI, so
you must manually forward it.

*BEEP*  *BEEP*  *BEEP*
Comment 4 leger 2000-02-03 18:04:50 PST
Moving all XPConnect QA contact to rginda
Comment 5 Mike Shaver (:shaver -- probably not reading bugmail closely) 2000-05-16 09:45:04 PDT
*** Bug 11420 has been marked as a duplicate of this bug. ***
Comment 6 Mike Shaver (:shaver -- probably not reading bugmail closely) 2000-05-16 09:46:47 PDT
I'd love some help with this.
Comment 7 Robert Low 2000-07-10 00:32:40 PDT
On Irix 6.5 using MipsPro (7.30 & 7.3.1m) and GNU (gcc 2.8.1
& 2.95.1) compilers, the assembler output indicates loading
and arithmetic on 4 byte (float) data.  Not sure if it has
ever worked, or what the status is on older Irix's and
compilers (so I won't attach it as a patch) but the following
works for me.

Index: xptcinvoke_irix.cpp
===================================================================
RCS file: 
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_irix.cpp,v
retrieving revision 1.3
diff -r1.3 xptcinvoke_irix.cpp
109c109,110
<               ((double*)regs)[i] = s->val.f;
---
>               // Place a float in least significant bytes.
>               ((float*)regs)[(i<<1)+1] = s->val.f;
111c112,113
<               *((double*)d++) = s->val.f;
---
>               // Place a float at address d.
>               *((float*)d++) = s->val.f;
Comment 8 Robert Low 2000-07-10 01:10:16 PDT
The bugs marked as duplicates (11420 and 28801) regarding
unresolved symbol ...StubBase seem to be something different
to the float problem.  Getting rid of the Gv at the end of
the STUB_ENTRY macro in xptcstubs_asm_irix.s got me SEGVing
in _XPTC_InvokeByIndex.  The virtual function table entry
size/structure seems to be different for the MipsPro and
GNU compilers, and even gcc (with no -mips option) produced a
different result on a MIPS4 machine than -mips4 on the same.

Current virtual table assembler code assumes a 3 word (12 bytes)
entry (consisting of offset_to_this, ???, and function_address).
GNU compilers without -mips on a MIPS4 machine with Irix6.5
produce a entry with 2 half words and a word (ie. 8 bytes) and
appears to be same structure as above (ie. offset_to_this (half),
??? (half), func_addr (word).  The following code got the
test going (and main progs not SEGVing) for this half/half/word
structure, but a real fix needs some pre-processor work.

Index: xptcinvoke_asm_irix.s
===================================================================
RCS file: 
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_irix.s,v
retrieving revision 1.1
diff -r1.1 xptcinvoke_asm_irix.s
76,80c76,77
<       # t1 = methodIndex * 12
<       # (use shift and subtract trick instead of mult)
<       sll     t1, a1, 2
<       subu    t1, t1, a1
<       sll     t1, t1, 2
---
>       # t1 = methodIndex * 8
>       sll     t1, a1, 3
86c83
<       li      t2, 20
---
>       li      t2, 12
88c85
<       lw      t9, 0(t9)       # t9 = *(that+t1+20)
---
>       lw      t9, 0(t9)       # t9 = *(that+t1+12)
94c91
<       lw      t0, 12(t0)
---
>       lh      t0, 8(t0)
Comment 9 Robert Low 2000-09-20 19:41:28 PDT
The initial bug was reporting a problem with floating
point arithmetic for xptcall, but the bug was then expanded
to include some problems with gcc linking and runtime.

I believe bug 43024 adequately descibes and proposes
fixes for the gcc problems, so I would like this bug 10061
rescoped to only deal with the floating point problem
common to both gcc and MIPSpro compilation.

Following is a diff to fix the floating point bug
(slightly improved on my previous diff for the same file).
Plus also a couple of changes to dubious casts re double.

--------------% cvs diff xptcinvoke_irix.cpp
Index: xptcinvoke_irix.cpp
===================================================================
RCS file: 
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_irix.cpp,v
retrieving revision 1.3
diff -r1.3 xptcinvoke_irix.cpp
109c109,110
<               ((double*)regs)[i] = s->val.f;
---
>               // Place a float in least significant bytes.
>               *(float*)(((char*)&regs[i+1]) - sizeof(float)) = s->val.f;
111c112
<               *((double*)d++) = s->val.f;
---
>               *(float*)d++ = s->val.f;
115c116
<               ((double*)regs)[i] = s->val.d;
---
>               *(double*)&regs[i] = s->val.d;
117c118
<               *((double*)d++) = s->val.d;
---
>               *(double*)d++ = s->val.d;
Comment 10 Robert Low 2000-10-09 16:18:41 PDT
Mike, what is the chance of getting this checked in.
It only effects SGI Irix, and is a generic fix for
both MIPSpro and GNU compiler builds.
thanks
Rob
Comment 11 Mike Shaver (:shaver -- probably not reading bugmail closely) 2000-10-14 09:07:51 PDT
The chances are pretty high, if you get it reviewed (by someone other than me;
I'll be your sr=).

To do that, you probably want to:
 - create a unified diff
 - attach it to the bug (using ``Create a new attachment'')
 - cc: some likely reviewers
Comment 12 Robert Low 2000-12-10 15:20:23 PST
Created attachment 20452 [details] [diff] [review]
Patch: fix for Irix float handling in xptcinvoke_irix.cpp.
Comment 13 John Bandhauer 2000-12-19 01:07:57 PST
Spooky, I could *swear* I marked this one r=jband the first time you asked for
review. Looks good to me if you say this is right. Sorry for the delay.
Comment 14 Christopher Blizzard (:blizzard) 2001-01-04 13:21:28 PST
rubber stamp sr=blizzard on this.  It looks OK as far as I can tell.
Comment 15 Blake Ross 2001-01-05 15:44:34 PST
Fix checked in.
Comment 16 Tomi Leppikangas 2001-01-08 15:10:15 PST
TestXPTCInvoke works ok now, marking as verified.

Note You need to log in before you can comment on or make changes to this bug.