4 of the JIT tests fail with flags amn. All tests pass with other flag combinations.
The browser can't stand up with JIT turned on as per default. Turning on to always jit and disabling type inference it can stand up but crashes soon after that.
Turning off jit altogether solves the issues.
In the backtrace I can see that it always crashes with java script objects allocated at addresses of something near to xefffa98c - does that mean that the stack is nearly empty or that it's nearly full?
Comment #1
Posted on Jun 12, 2012 by Massive RhinoWhich tests?
Comment #2
Posted on Jun 12, 2012 by Massive RhinoAlso, if you can compare an 'hg history .' in js/src with 14, we can see what landed that was different. Since JM+TI is supposed to be old and busted, it can't be something too complicated.
Comment #3
Posted on Jun 12, 2012 by Happy RabbitThe history isn't very useful because 14 and 15 come from different branches.
Excluding subdirectories "tests", "jit-test" and "yarr/pcre" results in a 3MB diff. Some part of the changes are just the changed licence headers, other big part is the new garbage collection and there were also some other general changes.
Comment #4
Posted on Jun 12, 2012 by Massive RhinoDifferent branches? But IGC could definitely do it. javascript.options.mem.gc_incremental
But what tests are failing?
Comment #5
Posted on Jun 12, 2012 by Happy RabbitAlready tried disabling incremental gc but nothing changed.
The failing tests are: arguments/argsx-1.js arguments/argsx-3.js arguments/argsx-3a.js jaeger/bug710780.js
When hg updating from one major version to another one mercurial warns that you are about to update crossing branches. And indeed there are some ten thousands of file changes when doing it. And the history is pretty useless also as mercurial seems to revert all changesets until the most recent common ancestor and then begins te apply the changesets from the other branch.
Comment #6
Posted on Jun 12, 2012 by Happy RabbitHere the JaegerSpew output when running args-1.js, one with flags "amd" which doesn't crash and one with flags "amnd" which does crash.
- argsx-1_amd.log 344.63KB
- argsx-1_amnd.log 602.03KB
Comment #7
Posted on Jun 12, 2012 by Happy RabbitAnd now InferSpew output.
- argsx-1_amnd_infer.log 644.69KB
Comment #8
Posted on Jun 12, 2012 by Massive RhinoI usually just have separate trees. I'll diff them at some point. Adding Ben to see if anything jumps out at him from the output
Comment #9
Posted on Jun 12, 2012 by Massive RhinoHowever, since -d doesn't make a difference, this probably isn't the stubs issue, since -d will fix that.
Comment #10
Posted on Jun 13, 2012 by Grumpy BirdIIRC, 0xEFFFA98C is way up near the top of the stack (i.e. almost empty), at least for the real browser. (We still put the stack top @ 0xF0000000, right?)
Can you post a backtrace? Does it still crash if you turn off the G4-style branching?
Comment #11
Posted on Jun 13, 2012 by Massive RhinoBrowser stack is at 0xf0000000, but js shell only has 0x10000000 of stack, so it doesn't need to be relocated. I assume the 0xefff.... was from the browser.
Comment #12
Posted on Jun 13, 2012 by Happy RabbitHere one backtrace of the js shell and one of the browser. Note that the cause for the crash is always dereferencing a null pointer.
- argsx-1_amn_backtrace.log 2.75KB
- browser_m_backtrace.log 9.68KB
- browser_mn_backtrace.log 7.79KB
Comment #13
Posted on Jun 13, 2012 by Happy RabbitStrangely now after rebuilding with optimization for G4 7400 the pointers now don't point to the top of the stack anymore. But that indicates that stack over-/underflow isn't causing the problem.
Comment #14
Posted on Jun 13, 2012 by Massive RhinoThe assert in the shell backtrace was added from bug 728411, according to the hg blame log. It seems like there is some unfinished work to do with that.
Comment #15
Posted on Jun 13, 2012 by Happy RabbitThe backtraces are from a debug build with optimization -O1. Here a backtrace from a release build (-O3).
- argsx-1_amn_rel.log 2.35KB
Comment #16
Posted on Jun 13, 2012 by Happy RabbitNow the instructions that lead to the crash. It happens directly after the branch in JaegerStubVeneer.
Comment #17
Posted on Jun 13, 2012 by Massive RhinoWhen you enter ArgGetter, can you dump the registers after the stwu r1,... executes? I want to see what argument is corresponding to which register. r4 came from r28 originally and that appears to be the bad register it can't dereference.
Comment #18
Posted on Jun 13, 2012 by Happy RabbitHere the reg dump.
- argsx-1_amnd_regdump.log 1.29KB
Comment #19
Posted on Jun 13, 2012 by Massive RhinoThe obj argument is the bad register. No surprise there. The assert is when it tries to get the shape of the object, and the object pointer is bogus, so that's all correct. So somehow a bad object pointer is getting in here. You don't have jemalloc on, do you?
Comment #20
Posted on Jun 13, 2012 by Happy RabbitThe browser is indeed built with jemalloc - but that doesn't affect the js shell in any way as far as I can see.
Comment #21
Posted on Jun 13, 2012 by Happy RabbitVerified that the js shell doesn't use jemalloc. In backtraces in other cases where gdb could directly display the object pointer I had seen that it was NULL (and in that cases it was always NULL).
Comment #22
Posted on Jun 22, 2012 by Massive RhinoWell, we have a bigger problem. I can't get 14 to pass tests either; we are failing a very weird one, tests/jaeger/recompile/exotic.js with -a -m -n. It did pass 13 and the test didn't change. I'm not sure if this is even related to this particular problem, but we need to solve this one first. 14 passed all tests for you?
The assertion is a ridiculous args.length(), which occurs just after args.length() goes to zero. My suspect is bug 733950.
Comment #23
Posted on Jun 22, 2012 by Massive RhinoThe part that actually fails (as minimized as I could get it):
// Recompilation out of SplatApplyArgs. weirdarray = [,,1,2,3]; Object.defineProperty(weirdarray, 0, {get: function() { vglobal = 'true'; }});
var vglobal = 3; function yfoo(x, y) { var q = x.apply(null, y); if (q != 10) assertEq(vglobal, 'true'); else assertEq(vglobal, 3); } yfoo(function(a) { return 10; }, [1]); yfoo(function() { return 0; }, weirdarray);
Comment #24
Posted on Jun 22, 2012 by Massive RhinoAnd it looks like this bugged for you in issue 150, but did not assert. What did you do to fix it?
Comment #25
Posted on Jun 22, 2012 by Massive RhinoSo, debugging a bit. Between 13 and 14 lazyArgsObj was removed from the VMFrame, so dynamicArgc sits at 24(r1) and dead space at 28(r1) where it used to be.
Upon entering JaegerInterpoline, 24(r1) shows an argc of 5, which is a reasonable value. It then calls methodjit/InvokeHelpers.cpp's js_InternalInterpret. js_InternalInterpret pulls down a 352 byte stack frame and puts sp into r30, then proceeds to store r4 into 380(r30), which overwrites our old 24(r1). Yuck.
Comment #26
Posted on Jun 22, 2012 by Massive RhinoProgram received signal SIGTRAP, Trace/breakpoint trap.
_JaegerInterpoline () at /Volumes/BruceDeuce/src/mozilla-14b/js/src/methodjit/TrampolinePPCOSX.s:285
285 stw r13, 52(r1)
(gdb) i reg r1
r1 0xbfffd430 3221214256
(gdb) x/128 0xbfffd430
0xbfffd430: 0xbfffd4b0 0x44024424 0x0060af14 0x01809fcc
0xbfffd440: 0x014084c0 0x02511260 0x00000005 0x00442a84
0xbfffd450: 0x00000000 0x00000000 0x02008108 0x0141052e
0xbfffd460: 0x00000000 0x02008080 0x014084c0 0x023e8000
0xbfffd470: 0x02008080 0xbfffed24 0x00000000 0x01410510
0xbfffd480: 0xbffff268 0x00000000 0x014084c0 0x007e0b00
0xbfffd490: 0x00000001 0x007e0b00 0x0000003a 0x00000001
0xbfffd4a0: 0x023e8000 0x010a9808 0x01408500 0x0060af28
0xbfffd4b0: 0xbfffd540 0x84024422 0x00442d38 0x007e0b00
0xbfffd4c0: 0xbfffd510 0x00000000 0x00000003 0xbfffecc4
0xbfffd4d0: 0x01410504 0x00000006 0x00000001 0x00000000
0xbfffd4e0: 0x02008000 0xbfffed24 0x014084c0 0x01809e00
0xbfffd4f0: 0x00000001 0x014084c0 0x0000ffff 0x00000001
0xbfffd500: 0x02008080 0x014111b8 0xbfffd510 0x00196b84
0xbfffd510: 0xbfffd5c0 0x00000001 0x02008080 0x00000000
0xbfffd520: 0x010a9808 0x014084c0 0xbfffed24 0x007e002c
0xbfffd530: 0x0000010a 0x020080c8 0xbfffd540 0x00443000
0xbfffd540: 0xbfffd5c0 0x44024424 0x0044377c 0x00000000
0xbfffd550: 0x00000000 0x00000000 0x00000000 0x00000000
0xbfffd560: 0xbfffd5c0 0x00000000 0x00000002 0x02500200
0xbfffd570: 0xbfffd5c0 0x00000077 0xbffff268 0x00000000
0xbfffd580: 0x014084c0 0x007e0b00 0x00000001 0x007e0b00
0xbfffd590: 0x0000003a 0x00000000 0x023e8000 0x00000001
0xbfffd5a0: 0xffffff81 0x00000000 0x00000000 0x00000002
0xbfffd5b0: 0x00000000 0x02507120 0xbfffd5c0 0x00170a88
0xbfffd5c0: 0xbffff040 0x44024482 0x00185114 0x00000014
0xbfffd5d0: 0x00000001 0xfffffffe 0x00000000 0x00000001
0xbfffd5e0: 0x020080d8 0x00000002 0xffffffff 0x014111e8
0xbfffd5f0: 0x00000014 0x00000001 0x00000000 0x00000000
0xbfffd600: 0xfffffffe 0x00000274 0x00663d00 0x02503040
0xbfffd610: 0x00000000 0x00000005 0x00000000 0xfffffffe
0xbfffd620: 0x00000000 0xfffffffe 0x000002b8 0x00000538
(gdb) si
285 stw r13, 52(r1)
(gdb)
285 stw r13, 52(r1)
(gdb)
285 stw r13, 52(r1)
(gdb) disp/i $pc
1: x/i $pc 0x60af28 <_JaegerInterpoline>: trap
(gdb) set $pc+=4
(gdb) si
297 mr r6, r1 ; VMFrame
1: x/i $pc 0x60af30 <_JaegerInterpoline+8>: mr r5,r3
(gdb)
298 mr r5, r3 ; returnReg (from MachineRegs.h)
1: x/i $pc 0x60af34 <_JaegerInterpoline+12>: mr r4,r8
(gdb)
299 mr r4, r8 ; returnType (from BaseAssembler.h)
1: x/i $pc 0x60af38 <_JaegerInterpoline+16>: mr r3,r7
(gdb)
300 mr r3, r7 ; returnData (from BaseAssembler.h)
1: x/i $pc 0x60af3c <_JaegerInterpoline+20>: bl 0x57b670
(gdb)
js_InternalInterpret (returnData=0x2512b50, returnType=0x0, returnReg=0x1, f=@0xbfffd430) at /Volumes/BruceDeuce/src/mozilla-14b/js/src/methodjit/InvokeHelpers.cpp:753
753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b670 : mflr r0
(gdb) x/8 0xbfffd430
0xbfffd430: 0xbfffd4b0 0x44024424 0x0060af14 0x01809fcc
0xbfffd440: 0x014084c0 0x02511260 0x00000005 0x00442a84
Current language: auto; currently c++
(gdb) si
0x0057b674 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b674 : stmw r13,-76(r1)
(gdb)
269 StackFrame *fp() { return regs.fp(); }
1: x/i $pc 0x57b678 : addi r26,r6,40
(gdb) x/8 0xbfffd430
0xbfffd430: 0xbfffd4b0 0x44024424 0x0060af14 0x01809fcc
0xbfffd440: 0x014084c0 0x02511260 0x00000005 0x00442a84
(gdb) si
753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b67c : mfcr r2
(gdb)
0x0057b680 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b680 : bcl- 20,4*cr7+so,0x57b684
(gdb)
0x0057b684 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b684 : stw r0,8(r1)
(gdb)
0x0057b688 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b688 : mr r20,r6
(gdb) x/8 0xbfffd430
0xbfffd430: 0xbfffd4b0 0x44024424 0x0060af40 0x01809fcc
0xbfffd440: 0x014084c0 0x02511260 0x00000005 0x00442a84
(gdb) si
0x0057b68c 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b68c : mr r24,r5
(gdb)
0x0057b690 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b690 : mflr r31
(gdb)
0x0057b694 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b694 : stw r2,4(r1)
(gdb)
0x0057b698 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b698 : stwu r1,-352(r1)
(gdb) x/8 0xbfffd430
0xbfffd430: 0xbfffd4b0 0x44024424 0x0060af40 0x01809fcc
0xbfffd440: 0x014084c0 0x02511260 0x00000005 0x00442a84 <<< OK!
(gdb) si
0x0057b69c 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b69c : mr r30,r1
(gdb)
753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b6a0 : stw r3,376(r30)
(gdb)
0x0057b6a4 753 js_InternalInterpret(void *returnData, void *returnType, void *returnReg, js::VMFrame &f)
1: x/i $pc 0x57b6a4 : stw r4,380(r30)
(gdb)
1249 StackFrame *fp() const { return fp_; }
1: x/i $pc 0x57b6a8 : lwz r3,12(r26)
(gdb) x/8 0xbfffd430
0xbfffd430: 0xbfffd4b0 0x44024424 0x0060af40 0x01809fcc
0xbfffd440: 0x014084c0 0x02511260 0x02512b50 0x00000000 <<< CRAP!
(gdb) i reg r4
r4 0x0 0
(gdb) i reg r30
r30 0xbfffd2d0 3221213904
(gdb)
Comment #27
Posted on Jun 22, 2012 by Massive RhinoThis works.
- ; Pull down a dummy argument area.
- addi r1, r1, -64 bl _js_InternalInterpret
- addi r1, r1, 64
If we keep getting problems like this, then I guess we have to bite the bullet and rework the VMFrame. Hopefully we can keep this glued together until IonMonkey.
Strictly speaking this probably also applies to 10. Tobias, does this fix your proble?
Comment #28
Posted on Jun 22, 2012 by Happy RabbitI currently don't have internet connection at home - because of that I didn't investigate any longer in this issue. The particular problem with tests/jaeger/recompile/exotic.js didn't show up anymore once I started building with gcc 4.6 . After that I didn't build with gcc 4.0/4.2 anymore. (So more different inlining in gcc 4.6 might have solved this?)
However I'll be able to apply the patch to the interpoline - but that might take some days.
Comment #29
Posted on Jun 22, 2012 by Massive RhinoNo worries -- I've been in that situation not too long ago :P . When I get 14 up and running, then I will pull down an early copy of 15 and see if this works (if you don't get to it first). Could definitely be changes in how 4.6 inlines functions, surely.
Comment #30
Posted on Jun 23, 2012 by Massive RhinoOkay, posting from 14. Had to do more hacks to it than I would like to get around compiler problems. I guess 4.6 will be coming soon to 10.4 as well.
Comment #31
Posted on Jun 23, 2012 by Happy RabbitMacports should have soon or already does have a port or gcc 4.6 for Tiger available. I used the port to build Aurora 14 and 15 on 10.5 - I had to do some installation steps manually though.
And I do have a patch available that allows contains all fixes I needed to build with gcc 4.6 .
Comment #32
Posted on Jul 2, 2012 by Happy RabbitThe patch from comment 27 doesn't fix the crashes here. If I understand correctly this will only change anything if type inference is used - but the crashes I also get with type inference off.
So may be there are other places where the dummy argument area is needed in order to get 15 to work?
Comment #33
Posted on Jul 5, 2012 by Massive RhinoComment deleted
Comment #34
Posted on Jul 5, 2012 by Massive RhinoWhoops, cut and pasted the wrong thing.
bruce:/home/spectre/src/bruce/mozilla-15a/js/src/jit-test/% gdb --args ../../../obj-ff-dbg/dist/bin/js -a -m -n -f lib/prolog.js -f /Volumes/BruceDeuce/src/mozilla-15a/js/src/jit-test/tests/arguments/argsx-1.js GNU gdb 6.3.50-20050815 (Apple version gdb-696) (Sat Oct 20 18:20:28 GMT 2007) Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "powerpc-apple-darwin"... warning: --arch option not supported in this gdb. Reading symbols for shared libraries .......... done
(gdb) run Starting program: /Volumes/BruceDeuce/src/mozilla-15a/obj-ff-dbg/dist/bin/js -a -m -n -f lib/prolog.js -f /Volumes/BruceDeuce/src/mozilla-15a/js/src/jit-test/tests/arguments/argsx-1.js Reading symbols for shared libraries ....................+++...............................................+ done
Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00000000 ArgGetter (cx=0x1408790, obj={ptr = 0x2008118}, id={ptr = 0x2008110}, vp=0x2008108) at gc/Barrier.h:172 172 operator T*() const { return value; }
Comment #35
Posted on Jul 5, 2012 by Massive RhinoAdding a dummy argument frame in JaegerStubVeneer made no difference.
Bugs in history diff for gc/Barrier.h between 14 (working) and 15 (not working): 730933 744579 753931 751003 750907 751003(?) 750476 745322 744880
Comment #36
Posted on Jul 5, 2012 by Massive RhinoHmm.
Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00000000 ArgGetter (cx=0x1408790, obj={ptr = 0x2008118}, id={ptr = 0x2008110}, vp=0x2008108) at gc/Barrier.h:172 172 operator T*() const { return value; } (gdb) bt
0 ArgGetter (cx=0x1408790, obj={ptr = 0x2008118}, id={ptr = 0x2008110}, vp=0x2008108) at gc/Barrier.h:172
1 0x0063ed48 in _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:259
[...] (gdb) x/128 0x2008108 0x2008108: 0xffffff82 0x00000000 0x00000000 0x02503040 0x2008118: 0x00000000 0x010ef9fc 0xffffff82 0x00000000
Notice the pointers are separated by 8 bytes and at the 4 byte offset, we appear to have valid obj and id pointers (and I believe vp should be zero also here):
(gdb) x/128 0x02503040 0x2503040: 0x02513c28 0x025000e0 0x01809a00 0x006bc3c4 0x2503050: 0xffffff82 0x00000000 0xffffff87 0x02506040
(gdb) x/128 0x010ef9fc 0x10ef9fc: 0x81ad0010 0x82ed004c 0x3800ffff 0x7ee0bc51 0x10efa0c: 0x7d8102a6 0x558c203e 0x7d801120 0x558ce0fe
Comment #37
Posted on Jul 5, 2012 by Massive RhinoIn 14, the ArgGetter was ArgGetter(JSContext *cx, JSObject *obj, jsid id, Value *vp). In 15, the ArgGetter is ArgGetter(JSContext *cx, HandleObject obj, HandleId id, Value *vp).
This is new with the new garbage collector, and I think our culprit is somewhere in gc/Root.h. I think this is Mozilla's bug, but we'll have to figure it out ourselves.
Comment #38
Posted on Jul 5, 2012 by Massive RhinoHere's my patches so far. Ben, do you have any ideas? I did a little fiddling with gc/Barrier.h to make EncapsulatedPtr return value+4 but this just seemed to make it worse (and is the wrong fix anyway, even if it wallpapered this issue).
- aurora-changesets.zip 1.79MB
Comment #39
Posted on Jul 5, 2012 by Massive RhinoUltra-minified test case with my notes. The requirement of running x amount of times suggests a polymorphic inline cache that is getting warmed up. If you unroll the loop, it doesn't crash. But -d doesn't fix this either.
function f() {
// must be 5 or higher for (var i = 0; i < 5; ++i) { // a[0] doesn't crash arguments.callee; }
/* unrolling the loop doesn't crash: arguments.callee; arguments.callee; arguments.callee; arguments.callee; arguments.callee; */ }
f();
Comment #40
Posted on Jul 5, 2012 by Massive RhinoWe get repatched to a different patch (the ArgGetter) right before the end. Before that it only calls ic::GetProp, which works fine. So this is indeed a problem with the ICs.
Program received signal SIGTRAP, Trace/breakpoint trap. _exitthejaeger () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:160 160 li r3, 1 (gdb) set $pc+=4 (gdb) cont Continuing.
Program received signal SIGTRAP, Trace/breakpoint trap. _JaegerTrampoline () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:138 138 mtctr r24 (gdb) set $pc+=4 (gdb) cont Continuing.
Program received signal SIGTRAP, Trace/breakpoint trap. _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:259 259 stw r0, 124(r1) (gdb) set $pc+=4 (gdb) si js::mjit::ic::GetProp (f=@0xbfffd020, pic=0x1411020) at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/PolyIC.cpp:1887 1887 ic::GetProp(VMFrame &f, ic::PICInfo *pic) (gdb) disp/i $pc 1: x/i $pc 0x586c30 <_ZN2js4mjit2ic7GetPropERNS_7VMFrameEPNS1_7PICInfoE>: mflr r0 Current language: auto; currently c++ (gdb) finish Run till exit from #0 js::mjit::ic::GetProp (f=@0xbfffd020, pic=0x1411020) at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/PolyIC.cpp:1887 _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:271 271 bctrl 1: x/i $pc 0x63ed40 <_JaegerStubVeneer+20>: lwz r0,124(r1) (gdb) cont Continuing. Current language: auto; currently asm
Program received signal SIGTRAP, Trace/breakpoint trap. _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:259 259 stw r0, 124(r1) 1: x/i $pc 0x63ed38 <_JaegerStubVeneer+12>: trap (gdb) set $pc+=4 (gdb) si js::mjit::ic::GetProp (f=@0xbfffd020, pic=0x1411020) at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/PolyIC.cpp:1887 1887 ic::GetProp(VMFrame &f, ic::PICInfo *pic) 1: x/i $pc 0x586c30 <_ZN2js4mjit2ic7GetPropERNS_7VMFrameEPNS1_7PICInfoE>: mflr r0 (gdb) finish Run till exit from #0 js::mjit::ic::GetProp (f=@0xbfffd020, pic=0x1411020) at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/PolyIC.cpp:1887 Current language: auto; currently c++ _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:271 271 bctrl 1: x/i $pc 0x63ed40 <_JaegerStubVeneer+20>: lwz r0,124(r1) (gdb) si Current language: auto; currently asm 274 lwz r0, 124(r1) 1: x/i $pc 0x63ed44 <_JaegerStubVeneer+24>: mtlr r0 (gdb) cont Continuing.
Program received signal SIGTRAP, Trace/breakpoint trap. _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:259 259 stw r0, 124(r1) 1: x/i $pc 0x63ed38 <_JaegerStubVeneer+12>: trap (gdb) set $pc+=4 (gdb) si ArgGetter (cx=0x1408710, obj={ptr = 0x20080d8}, id={ptr = 0x20080d0}, vp=0x20080c8) at /Volumes/BruceDeuce/src/mozilla-15a/js/src/vm/ArgumentsObject.cpp:169 169 ArgGetter(JSContext *cx, HandleObject obj, HandleId id, Value *vp) 1: x/i $pc 0x32f740 : mflr r0
Comment #41
Posted on Jul 5, 2012 by Massive Rhino[jaeger] PICs getprop ignored: first hit (test.js: 6)
Program received signal SIGTRAP, Trace/breakpoint trap. _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:259 259 stw r0, 124(r1) (gdb) disas 0x10ebe00 0x10ebe50 Dump of assembler code from 0x10ebe00 to 0x10ebe50: 0x010ebe00: li r0,340 0x010ebe04: mr r23,r22 0x010ebe08: lwz r20,72(r13) 0x010ebe0c: stw r20,80(r13) 0x010ebe10: stw r22,84(r13) 0x010ebe14: cmpwi r20,-121 0x010ebe18: bne- 0x10ec120 0x010ebe1c: li r0,324 0x010ebe20: lwz r21,0(r23) 0x010ebe24: lis r0,0 0x010ebe28: nop 0x010ebe2c: cmpw r21,r0 0x010ebe30: bne- 0x10ec128 0x010ebe34: li r0,312 0x010ebe38: lis r12,0 0x010ebe3c: ori r12,r12,8 0x010ebe40: lwzx r23,r23,r12 0x010ebe44: lis r12,0 0x010ebe48: ori r12,r12,0 0x010ebe4c: add r12,r23,r12 End of assembler dump. (gdb) set $pc+=4 (gdb) cont Continuing. [jaeger] PICs generated getprop stub at 0x10ec570 [jaeger] PICs Patching previous (0 stubs) (start 0x10ebe14) (offset 36) (second 0)
Program received signal SIGTRAP, Trace/breakpoint trap. _JaegerStubVeneer () at /Volumes/BruceDeuce/src/mozilla-15a/js/src/methodjit/TrampolinePPCOSX.s:259 259 stw r0, 124(r1) (gdb) disas 0x10ebe00 0x10ebe50 Dump of assembler code from 0x10ebe00 to 0x10ebe50: 0x010ebe00: li r0,340 0x010ebe04: mr r23,r22 0x010ebe08: lwz r20,72(r13) 0x010ebe0c: stw r20,80(r13) 0x010ebe10: stw r22,84(r13) 0x010ebe14: cmpwi r20,-121 0x010ebe18: bne- 0x10ec120 0x010ebe1c: li r0,324 0x010ebe20: lwz r21,0(r23) 0x010ebe24: lis r0,0 0x010ebe28: nop 0x010ebe2c: cmpw r21,r0 0x010ebe30: bne- 0x10ec570 0x010ebe34: li r0,312 0x010ebe38: lis r12,0 0x010ebe3c: ori r12,r12,8 0x010ebe40: lwzx r23,r23,r12 0x010ebe44: lis r12,0 0x010ebe48: ori r12,r12,0 0x010ebe4c: add r12,r23,r12 End of assembler dump. (gdb)
It looks like it patches the branch correctly, but it doesn't patch the guard (the patchable load is still 0).
Comment #42
Posted on Jul 5, 2012 by Massive RhinoNever mind. Here's the real problem.
The branch gets called to the new stub at 0x10ec570 as promised. Near the end of that, we get this:
0x010ec5e0 in ?? () 1: x/i $pc 0x10ec5e0: addi r22,r13,88 <<< r13 is JSFP, remember (gdb) 0x010ec5e4 in ?? () 1: x/i $pc 0x10ec5e4: mr r6,r22 (gdb) 0x010ec5e8 in ?? () 1: x/i $pc 0x10ec5e8: addi r22,r22,8 (gdb) 0x010ec5ec in ?? () 1: x/i $pc 0x10ec5ec: mr r5,r22 (gdb) 0x010ec5f0 in ?? () 1: x/i $pc 0x10ec5f0: addi r22,r22,8 (gdb) 0x010ec5f4 in ?? () 1: x/i $pc 0x10ec5f4: mr r4,r22
These are setting up the arguments for ArgGetter, as proven by:
(gdb) i reg r4 r4 0x20080d8 33587416 (gdb) i reg r5 r5 0x20080d0 33587408 (gdb) i reg r6 r6 0x20080c8 33587400 (gdb) cont [...] Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00000000 ArgGetter (cx=0x14086f0, obj={ptr = 0x20080d8}, id={ptr = 0x20080d0}, vp=0x20080c8) at gc/Barrier.h:172
So we need to look at how that stub is generated because it is clearly emitting the wrong offsets.
Comment #43
Posted on Jul 5, 2012 by Happy RabbitTrying to build js with JS_POLYIC undefined.
While looking for the correct place to do remove the definition I just recognized that by copying the JS config flags (in configure.in) from ARM we copied something that might be an oversight. ENABLE_POLYIC_TYPED_ARRAY doesn't do anything (anymore?) but I think it must be ENABLE_METHODJIT_TYPED_ARRAY instead (now?) as all other architectures use that. In case this really proves to be an oversight and changing the definition actually works this should be filed as a mozilla bug.
Comment #44
Posted on Jul 5, 2012 by Happy RabbitWhile the ARM port seems to have implemented the necessary functions for ENABLE_METHODJIT_TYPED_ARRAY to work our port actually doesn't...
Comment #45
Posted on Jul 5, 2012 by Happy RabbitSwitching off POLYIC doesn't seem to be supported anymore...
Comment #46
Posted on Jul 5, 2012 by Massive RhinoThanks for the catch. It's easy enough to implement METHODJIT_TYPED_ARRAY (basically we need to add the new methods in BaseAssembler.h), so we'll do that in a separate bug.
We really don't want to build with POLYIC off because it would be a big runtime hit, even if it did work. I think the problem is how PolyIC.cpp is computing vpOffset; they pull some new code in between 14 and 15.
Comment #47
Posted on Jul 5, 2012 by Massive Rhino(No comment was entered for this change.)
Comment #48
Posted on Jul 5, 2012 by Grumpy BirdThis chunk of code from methodjit/PolyIC.cpp looks suspicious to me:
1061 /* 1062 * Use three values above sp on the stack for use by the call to store 1063 * the object and id being passed into the call as handles and to store 1064 * the resulting value. Temporary slots are used by GETPROP for this, 1065 * plus there is extra room on the stack reserved for a callee frame. 1066 */ 1067 int32_t initialFrameDepth = f.regs.sp - f.fp()->slots() + 3; 1068 int32_t vpOffset = (char *) f.regs.sp - (char *) f.fp(); 1069 int32_t idHandleOffset = (char *) (f.regs.sp + 1) - (char *) f.fp(); 1070 int32_t objHandleOffset = (char *) (f.regs.sp + 2) - (char *) f.fp(); 1071 1072 masm.storePtr(holdObjReg, Address(JSFrameReg, objHandleOffset)); 1073 masm.storePtr(ImmPtr((void ) JSID_BITS(userid)), Address(JSFrameReg, idHandleOffset)); 1074 1075 / 1076 * On 32 bit platforms zero the upper portion of the values so that 1077 * the GC does not see a corrupt value in the handle slots. The two 1078 * slots will look like doubles, so won't be traced, but the objects 1079 * will be held live by the object value still in place on the stack. 1080 * This will need to be addressed once a moving GC can relocate the 1081 * objects, as the created handles will need to be properly registered. 1082 */ 1083 #if JS_BITS_PER_WORD == 32 1084 masm.storePtr(ImmPtr(NULL), masm.tagOf(Address(JSFrameReg, objHandleOffset))); 1085 masm.storePtr(ImmPtr(NULL), masm.tagOf(Address(JSFrameReg, idHandleOffset))); 1086 #endif
Specifically, it stores 0 to the tagOf(...), but it doesn't store the pointers to the payloadOf(...) -- meaning that the payloadOf(...) which gets used in the stub is just garbage sitting in the VM stack.
Comment #49
Posted on Jul 5, 2012 by Grumpy BirdOf course, if the stores on 1072/1073 are changed, the address calculations further down also need to be corrected by Assembler::PAYLOAD_OFFSET:
1106 /* Use a temporary for parameters. */ 1107 masm.addPtr(Imm32(vpOffset), JSFrameReg, t0); 1108 1109 masm.restoreStackBase(); 1110 masm.setupABICall(Registers::NormalCall, 4); 1111 masm.storeArg(3, t0); 1112 masm.addPtr(Imm32(idHandleOffset - vpOffset), t0); ^^^ + Assembler::PAYLOAD_OFFSET
1113 masm.storeArg(2, t0); 1114 masm.addPtr(Imm32(objHandleOffset - idHandleOffset), t0); 1115 masm.storeArg(1, t0); 1116 masm.storeArg(0, cxReg)
Unfortunately, Mozilla's x86-isms are showing through: it would be much clearer to write the addPtr()s all in 3-operand form based from JSFrameReg, but the code as written is more efficient in 2-operand x86-world.
Comment #50
Posted on Jul 5, 2012 by Massive RhinoWell, we could #ifdef it and just rewrite the whole stub section, but I'm surprised this hasn't bitten ARM or SPARC (or, for that matter, MIPS). I'm thinking we might have an endian problem here, because if I understand tagOf() correctly, it's the second byte on x86 based on NunboxAssembler.h. Mozilla admits they don't do any testing on anything but little endian anymore.
I don't think there's a way to fix this for everyone, unless you can think of one; I think there will have to be two stub paths here.
We should definitely upstream this in any case.
Comment #51
Posted on Jul 5, 2012 by Massive RhinoEr, second word, even.
Comment #52
Posted on Jul 5, 2012 by Grumpy BirdYup, the issue is endian-ness. I'm not somewhere that I can prepare a patch, but I think that if we just change the storePtr()s on 1071-1072 to go to the addressOf(...) (== ... + 0 on x86) and then change line 1112 to be addPtr(Imm32(idHandleOffset + Assembler::PAYLOAD_OFFSET - vpOffset), ...) the result will actually be correct for both big- and little-endian platforms.
(Different jsval boxing styles are a separate issue, and indeed there may not be a clean way of writing it without #ifdefs.)
Comment #53
Posted on Jul 5, 2012 by Massive RhinoFortunately, my G5 never sleeps. This passes all of Tobias' failing tests. I'm doing a full test run now, and if that passes, we will push this to Mozilla. Thanks, Ben! I knew you'd spot it faster than I would.
diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -1064,18 +1064,21 @@ class GetPropCompiler : public PICStubCo * the resulting value. Temporary slots are used by GETPROP for this, * plus there is extra room on the stack reserved for a callee frame. */ int32_t initialFrameDepth = f.regs.sp - f.fp()->slots() + 3; int32_t vpOffset = (char *) f.regs.sp - (char *) f.fp(); int32_t idHandleOffset = (char *) (f.regs.sp + 1) - (char *) f.fp(); int32_t objHandleOffset = (char *) (f.regs.sp + 2) - (char *) f.fp();
- masm.storePtr(holdObjReg, Address(JSFrameReg, objHandleOffset));
- masm.storePtr(ImmPtr((void *) JSID_BITS(userid)), Address(JSFrameReg, idHandleOffset));
- /*
- Make sure we handle endianness correctly.
- */
- masm.storePtr(holdObjReg, masm.payloadOf(Address(JSFrameReg, objHandleOffset)));
masm.storePtr(ImmPtr((void *) JSID_BITS(userid)), masm.payloadOf(Address(JSFrameReg, idHandleOffset)));
/* * On 32 bit platforms zero the upper portion of the values so that * the GC does not see a corrupt value in the handle slots. The two * slots will look like doubles, so won't be traced, but the objects * will be held live by the object value still in place on the stack. * This will need to be addressed once a moving GC can relocate the * objects, as the created handles will need to be properly registered.
@@ -1104,17 +1107,17 @@ class GetPropCompiler : public PICStubCo masm.loadPtr(FrameAddress(offsetof(VMFrame, cx)), cxReg);
/* Use a temporary for parameters. */ masm.addPtr(Imm32(vpOffset), JSFrameReg, t0); masm.restoreStackBase(); masm.setupABICall(Registers::NormalCall, 4); masm.storeArg(3, t0);
- masm.addPtr(Imm32(idHandleOffset - vpOffset), t0);
masm.addPtr(Imm32(idHandleOffset - vpOffset + Assembler::PAYLOAD_OFFSET), t0); masm.storeArg(2, t0); masm.addPtr(Imm32(objHandleOffset - idHandleOffset), t0); masm.storeArg(1, t0); masm.storeArg(0, cxReg);
masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, getter), false); NativeStubLinker::FinalJump done;
Comment #54
Posted on Jul 5, 2012 by Massive RhinoAll tests pass. Tobias, confirm when you get a chance that you're back in business.
Comment #55
Posted on Jul 5, 2012 by Happy RabbitFrom today on I do have internet connection at home. With the patch to PolyIC.cpp the tests pass here as well - great job!
Comment #56
Posted on Jul 12, 2012 by Massive RhinoLeaving open pending 15
Comment #57
Posted on Jul 16, 2012 by Happy Rabbitjit-tests in Aurora 16 are fine so far; powerpc methodjit patched applied without problems - just the fix from comment 53 was already applied.
Comment #58
Posted on Jul 17, 2012 by Massive RhinoThen we'll mark this Verified.
Status: Verified
Labels:
Type-Defect
Priority-Critical
Milestone-Unstable