Last Comment Bug 755564 - IonMonkey: Crash [@ js::HeapPtr<js::BaseShape, unsigned long>::operator] with ParallelArray
: IonMonkey: Crash [@ js::HeapPtr<js::BaseShape, unsigned long>::operator] with...
Status: VERIFIED FIXED
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- major (vote)
: ---
Assigned To: David Anderson [:dvander]
:
Mentors:
: 737298 (view as bug list)
Depends on:
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-05-15 16:11 PDT by Christian Holler (:decoder)
Modified: 2013-02-07 05:14 PST (History)
9 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected


Attachments
make ParallelArray moving GC safe (21.80 KB, patch)
2012-05-17 12:12 PDT, David Anderson [:dvander]
wmccloskey: review+
Details | Diff | Review
part 2: fix (1.03 KB, patch)
2012-05-17 13:34 PDT, David Anderson [:dvander]
wmccloskey: review+
Details | Diff | Review
part 2: better fix (1.70 KB, patch)
2012-05-18 01:18 PDT, David Anderson [:dvander]
wmccloskey: review+
Details | Diff | Review

Description Christian Holler (:decoder) 2012-05-15 16:11:09 PDT
The following testcase crashes on ionmonkey revision 50177d59c0e1 (run with --ion -n -m):


var p = new ParallelArray([1,2,3,4,5]);
var r = p.scatter([0,1,0,3,4], 9, function (a,b) { return a+b; });
assertEq(r.toString( 5 ? r : 0, gc()) ,[4,2,9,4,5].join(","));


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004145a8 in js::HeapPtr<js::BaseShape, unsigned long>::operator js::BaseShape* (this=0xdadadadadadadada) at ../../gc/Barrier.h:212
212         operator T*() const { return value; }
(gdb) bt 8
#0  0x00000000004145a8 in js::HeapPtr<js::BaseShape, unsigned long>::operator js::BaseShape* (this=0xdadadadadadadada) at ../../gc/Barrier.h:212
#1  0x0000000000405fe0 in js::Shape::base (this=0xdadadadadadadada) at ../../jsscope.h:708
#2  0x0000000000405eea in js::Shape::getObjectClass (this=0xdadadadadadadada) at ../../jsscope.h:607
#3  0x000000000040714e in js::ObjectImpl::getClass (this=0x7ffff09099a0) at ../../vm/ObjectImpl-inl.h:245
#4  0x000000000040718e in js::ObjectImpl::getOps (this=0x7ffff09099a0) at ../../vm/ObjectImpl-inl.h:263
#5  0x0000000000551738 in js_GetMethod (cx=0xd955a0, obj=..., id=..., getHow=0, vp=0x7fffffffc530) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsobj.cpp:5166
#6  0x00000000006a0cd8 in ParallelArray_forward_method (cx=0xd955a0, argc=2, vp=0x7ffff0beb0b0, native=0x6a0d90 <ParallelArray_toString(JSContext*, unsigned int, JS::Value*)>, id=...)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/builtin/ParallelArray.cpp:587
#7  0x00000000006a0ddc in ParallelArray_toString (cx=0xd955a0, argc=2, vp=0x7ffff0beb0b0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/builtin/ParallelArray.cpp:602
(More stack frames follow...)
(gdb) x /i $pc
=> 0x4145a8 <js::HeapPtr<js::BaseShape, unsigned long>::operator js::BaseShape*() const+12>:    mov    (%rax),%rax
(gdb) info reg rax
rax            0xdadadadadadadada       -2676586395008836902
Comment 1 Bill McCloskey (:billm) 2012-05-15 16:17:51 PDT
Oops. IonMonkey bug. Never mind.
Comment 2 David Anderson [:dvander] 2012-05-16 19:18:23 PDT
This is very likely to be bug 729812 so let's wait until that's fixed and try this again.
Comment 3 David Anderson [:dvander] 2012-05-17 12:12:07 PDT
Created attachment 624831 [details] [diff] [review]
make ParallelArray moving GC safe

This doesn't fix the bug, but I noticed ParallelArray is not moving GC safe so this patch fixes that.
Comment 4 Bill McCloskey (:billm) 2012-05-17 12:33:17 PDT
Comment on attachment 624831 [details] [diff] [review]
make ParallelArray moving GC safe

Review of attachment 624831 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/ParallelArray.cpp
@@ +320,2 @@
>      if (!obj)
> +        return false;

I think we need to |return ok| here.

::: js/src/ion/MIR.cpp
@@ +492,5 @@
>  
>  void
>  MBitNot::infer(const TypeOracle::Unary &u)
>  {
> +    JS_ASSERT(u.ival != MIRType_Value);

Not sure if you intended for this to be here...
Comment 5 David Anderson [:dvander] 2012-05-17 13:34:24 PDT
Created attachment 624865 [details] [diff] [review]
part 2: fix

Bug is, it's non-native but has no trace hook. Bill pointed out it can just be native.
Comment 6 David Anderson [:dvander] 2012-05-17 14:20:37 PDT
Thanks for the quick reviews!

http://hg.mozilla.org/projects/ionmonkey/rev/fe35715a3f01
http://hg.mozilla.org/projects/ionmonkey/rev/b15990ffc15d
Comment 7 David Anderson [:dvander] 2012-05-18 01:01:21 PDT
Backing out part 2: http://hg.mozilla.org/projects/ionmonkey/rev/ec1aca662dbe

It turns out these objects have some non-native behavior, so it'll be easier to just implement a trace hook.
Comment 8 David Anderson [:dvander] 2012-05-18 01:18:19 PDT
Created attachment 625033 [details] [diff] [review]
part 2: better fix
Comment 9 David Anderson [:dvander] 2012-05-18 11:03:03 PDT
http://hg.mozilla.org/projects/ionmonkey/rev/eeae414fa07a
Comment 10 David Anderson [:dvander] 2012-05-18 11:39:15 PDT
*** Bug 737298 has been marked as a duplicate of this bug. ***
Comment 11 Christian Holler (:decoder) 2012-05-18 11:40:05 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 12 Christian Holler (:decoder) 2013-02-07 05:14:42 PST
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397

Note You need to log in before you can comment on or make changes to this bug.