Closed
Bug 520367
Opened 13 years ago
Closed 13 years ago
xpcom/PowerPC lnx: fix wrong argument in padding in a rare case
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: mozbug, Assigned: mozbug)
Details
Attachments
(2 files)
2.11 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
5.84 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee | ||
Comment 2•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #404419 -
Flags: review?(shaver)
Comment 3•13 years ago
|
||
--> 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?
Assignee | ||
Comment 5•13 years ago
|
||
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 7•13 years ago
|
||
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+
Updated•13 years ago
|
Keywords: checkin-needed
Updated•13 years ago
|
Assignee: nobody → mozbug
Comment 8•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/425726d54dc9 (fix and test together)
Status: NEW → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Comment 9•13 years ago
|
||
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.
Description
•