520367 - xpcom/PowerPC lnx: fix wrong argument in padding in a rare case

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Sebastian A. Siewior]

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0)
Build Identifier: "Mozilla/5.0 (X11; U; Linux ppc  en-US; rv:1.9.0.14) Gecko/2009100300                   Iceweasel/3.0.6 (Debian-3.0.6-3)"

lets assume the following call:
 
void func(void *this, long long l1, long long l2, int i1, long long l3,
                int i2)
 
which should be translated to
r3 this
r4 <pad>
r5 upper l1
r6 lower l1
r7 upper l2
r8 lower l2
r9 i1
r10 <pad>
stack +8 upper l3
stack +12 lower l3
stack +16 i2
 
Right now gpr is first tested if the long long fits and then incremented
if a pad is required. That means l3 does not fit in r10+r11 and gets
saved on stack. However i2 gets saved in r10 because the gpr still
allows one argument to be passed via gprs.



Reproducible: Always

Steps to Reproduce:
1. look at the code until it see it _or_ get a ppc
2. patch a the testcase
3. execute  xpcom/reflect/xptcall/tests/TestXPTCInvoke
Actual Results:  
 calling direct:
 P1 : 3
 P2 : 5
 P3 : 7
 P4 : 11
 P5 : 13
 P6 : 17
 P7 : 19
 P8 : 23
 P9 : 29
 P10: 31
 ret: 0xbf8f3b48
         3 + 5 + 7 + 11 + 13 + 17 + 19 + 23 + 29 + 31 = 158
 
 calling via invoke:
 backp: 0xbf8f3bf0
 P1 : 3
 P2 : 5
 P3 : 7
 P4 : 11
 P5 : 17
 P6 : 269303452
 P7 : 19
 P8 : 23
 P9 : 29
 P10: 31
 ret: 0xbf8f3bf0
         3 + 5 + 7 + 11 + 13 + 17 + 19 + 23 + 29+ 31 = 269303597


Expected Results:  
 calling direct:
 P1 : 3
 P2 : 5
 P3 : 7
 P4 : 11
 P5 : 13
 P6 : 17
 P7 : 19
 P8 : 23
 P9 : 29
 P10: 31
 ret: 0xbf8f3b48
         3 + 5 + 7 + 11 + 13 + 17 + 19 + 23 + 29 + 31 = 158
 
 calling via invoke:
 backp: 0xbf8f3bf0
 P1 : 3
 P2 : 5
 P3 : 7
 P4 : 11
 P5 : 13
 P6 : 17
 P7 : 19
 P8 : 23
 P9 : 29
 P10: 31
 ret: 0xbf8f3bf0
         3 + 5 + 7 + 11 + 13 + 17 + 19 + 23 + 29+ 31 = 158


I checked mozilla-central tree and the bug is still available.

Comment 1

[Sebastian A. Siewior]

Attachment: bug fix

Comment 2

[Sebastian A. Siewior]

Attachment: the test case addon

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

--> NEW since it has a testcase

jst, sicking: shaver's obviously the wrong person to review this. Who's better?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

cc'ing same group of people here to see if someone can review ppc xptcall code.

Also, can we get some tests here?

Comment 5

[Sebastian A. Siewior]

The output I added is from https://bugzilla.mozilla.org/attachment.cgi?id=404420. Isn't this enough for a test or do you think about something else?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Doh! Yes, assuming that test fails before your patch, and passes with it, that should do it.

Comment 7

[Benjamin Smedberg]

Comment on attachment 404419 [details] [diff] [review]
bug fix

I hate being the reviewer of last resort for xptcall, because I don't know anything about PPC, but I can't find anyone else who does either, so I'm going on trust and very basic code reading.

Please make sure this lands with its test.

Comment 8

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/425726d54dc9 (fix and test together)

Comment 9

[Mike Hommey [:glandium]]

FYI, this fixes nsNavHistory on powerpc (there are a couple functions using long long args or return values, and they failed)