Closed Bug 520367 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: XPCOM, defect)

PowerPC
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: mozbug, Assigned: mozbug)

Details

Attachments

(2 files)

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.
Attached patch bug fixSplinter Review
Attachment #404419 - Flags: review?(shaver)
--> NEW since it has a testcase

jst, sicking: shaver's obviously the wrong person to review this. Who's better?
Status: UNCONFIRMED → NEW
Ever confirmed: true
cc'ing same group of people here to see if someone can review ppc xptcall code.

Also, can we get some tests here?
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?
Doh! Yes, assuming that test fails before your patch, and passes with it, that should do it.
Attachment #404419 - Flags: review?(shaver) → review?(benjamin)
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.
Attachment #404419 - Flags: review?(benjamin) → review+
Keywords: checkin-needed
Assignee: nobody → mozbug
http://hg.mozilla.org/mozilla-central/rev/425726d54dc9 (fix and test together)
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
FYI, this fixes nsNavHistory on powerpc (there are a couple functions using long long args or return values, and they failed)
You need to log in before you can comment on or make changes to this bug.