10061 - xptcall not quite right on sgi irix 6.5

Status: VERIFIED FIXED

(Core :: XPConnect, defect, P3)

Description

[Tomi Leppikangas]

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]

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
<[e-mail]>. He is going to look into it.

Comment 2

[John Bandhauer]

I send another email to Jason.

Let's see if shaver really wants to own all the xptcall Unix bugs?

Comment 3

[John Bandhauer]

*BEEP*  *BEEP*  *BEEP*

This is an automatic reply to your email.

Your email message to [e-mail] has
been sent to my old address.

For personal email only, please use

        [e-mail]

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

*BEEP*  *BEEP*  *BEEP*

Comment 4

[leger]

Moving all XPConnect QA contact to rginda

Comment 5

[Mike Shaver (:shaver emeritus)]

*** Bug 11420 has been marked as a duplicate of this bug. ***

Comment 6

[Mike Shaver (:shaver emeritus)]

I'd love some help with this.

Comment 7

[Robert Low]

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]

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]

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]

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 emeritus)]

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]

Attachment: Patch: fix for Irix float handling in xptcinvoke_irix.cpp.

Comment 13

[John Bandhauer]

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)]

rubber stamp sr=blizzard on this.  It looks OK as far as I can tell.

Comment 15

[Blake Ross]

Fix checked in.

Comment 16

[Tomi Leppikangas]

TestXPTCInvoke works ok now, marking as verified.