Last Comment Bug 728086 - Assertion failure: (ptrBits & 0x7) == 0, at ../../jsval.h:796 or Crash [@ js::gc::ArenaHeader::allocated]
: Assertion failure: (ptrBits & 0x7) == 0, at ../../jsval.h:796 or Crash [@ js:...
Status: VERIFIED FIXED
[sg:critical] js-triage-needed [advis...
: assertion, crash, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla13
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on: 732087
Blocks: langfuzz 723313
  Show dependency treegraph
 
Reported: 2012-02-16 16:42 PST by Christian Holler (:decoder)
Modified: 2013-01-14 08:07 PST (History)
8 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
unaffected
-
unaffected
+
unaffected
+
fixed
unaffected


Attachments
Test case for shell (run with -n -m) (1.91 KB, application/javascript)
2012-02-16 16:42 PST, Christian Holler (:decoder)
no flags Details
patch (6.75 KB, patch)
2012-02-17 15:46 PST, Bill McCloskey (:billm)
luke: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-02-16 16:42:55 PST
Created attachment 598060 [details]
Test case for shell (run with -n -m)

The attached test asserts/crashes on mozilla-central revision 78fde7e54d92 (options -m -n).

Stepping through the assert crashes:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000412598 in js::gc::ArenaHeader::allocated (this=0x7fff00000000) at ../../jsgc.h:447
447             JS_ASSERT(allocKind <= size_t(FINALIZE_LIMIT));
(gdb) bt 8
#0  0x0000000000412598 in js::gc::ArenaHeader::allocated (this=0x7fff00000000) at ../../jsgc.h:447
#1  0x00000000004125e0 in js::gc::ArenaHeader::getAllocKind (this=0x7fff00000000) at ../../jsgc.h:478
#2  0x00000000004126f6 in js::gc::Cell::getAllocKind (this=0x7fff00000001) at ../../jsgc.h:869
#3  0x000000000047da08 in js::gc::GetGCThingTraceKind (thing=0x7fff00000001) at ../jsgcinlines.h:63
#4  0x00000000004c654a in js::gc::MarkKind (trc=0x7fffffffb230, thing=0x7fff00000001, kind=JSTRACE_STRING) at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:219
#5  0x00000000004c6930 in js::gc::MarkValueInternal (trc=0x7fffffffb230, v=...) at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:306
#6  0x00000000004c6ac2 in js::gc::MarkValueRootRange (trc=0x7fffffffb230, len=9, vec=0x7ffff63fb270, name=0x7d37e6 "vm_stack")
    at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:338
#7  0x000000000061e893 in js::gc::MarkValueRootRange (trc=0x7fffffffb230, begin=0x7ffff63fb270, end=0x7ffff63fb2b8, name=0x7d37e6 "vm_stack") at ../jsgcmark.h:114
(More stack frames follow...)
(gdb) x /4i $pc
=> 0x412598 <js::gc::ArenaHeader::allocated() const+16>:        movzbl 0x18(%rax),%eax
   0x41259c <js::gc::ArenaHeader::allocated() const+20>:        cmp    $0x14,%al
   0x41259e <js::gc::ArenaHeader::allocated() const+22>:        jbe    0x4125b8 <js::gc::ArenaHeader::allocated() const+48>
   0x4125a0 <js::gc::ArenaHeader::allocated() const+24>:        mov    $0x1bf,%edx
(gdb) info register rax
rax            0x7fff00000000   140733193388032


S-s and sg:critical due to GC related crash.
Comment 1 Bill McCloskey (:billm) 2012-02-16 18:11:27 PST
This is a regression from the non-conservative VM stack scanning. Here's what seems to be the problem. We call a native. It pushes space for some native CallArgs via ContextStack::pushInvokeArgs. Those args never get initialized with anything. Then we run a GC. It will scan the stack up to StackSpace::firstUnused(), which includes the uninitialized CallArgs. We crash.

I don't know this code well, but I'm kinda thinking that we don't need to scan those native CallArgs. I thought they were only there for the debugger. Luke? If we do have to scan them, is there a way to tell whether they've been initialized or not? If there's no way, how much would it cost us to just initialize them no matter what?
Comment 2 Luke Wagner [:luke] 2012-02-16 18:34:44 PST
The native args definitely need to be marked (they are assumed to be rooted memory from within the native).  I think pushInvokeArgs should simply MakeValueRangeGCSafe on the pushed range (again... I definitely used to do this but I guess it was removed b/c of the conservative stack scanner...).
Comment 3 Bill McCloskey (:billm) 2012-02-17 15:46:42 PST
Created attachment 598407 [details] [diff] [review]
patch

This patch adds the MakeRangeGCSafe. I benchmarked SunSpider, V8, and Kraken. I didn't observe any effect.

I also added some stack poisoning. It turned up another problem where we were reading outside of seg_->end(). I think it was safe, because we wouldn't have GCed in between the pop and the read. But still worth fixing.

Luke, since I'm not so familiar with the stack invariants, it may be possible to strengthen the poisoning stuff by putting it in more places or making it cover more memory. Any suggestions?
Comment 4 Luke Wagner [:luke] 2012-02-18 18:20:15 PST
Comment on attachment 598407 [details] [diff] [review]
patch

Nice debug additions.
Comment 6 Bill McCloskey (:billm) 2012-02-22 13:41:13 PST
Backed out because it turned Windows builds purple (purple??):
https://hg.mozilla.org/integration/mozilla-inbound/rev/3446a5f91c2b
Comment 7 Daniel Veditz [:dveditz] 2012-02-23 13:37:44 PST
(In reply to Bill McCloskey (:billm) from comment #1)
> This is a regression from the non-conservative VM stack scanning.

What's the bug for that, or the affected versions?
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2012-02-23 13:43:35 PST
Tracking for 12 onwards. Bill, if this is not a problem in 12 let me know and I'll untrack this.
Comment 9 Bill McCloskey (:billm) 2012-02-23 13:47:18 PST
This is a regression from bug 723313. 12 is unaffected.
Comment 10 Bill McCloskey (:billm) 2012-02-23 14:57:12 PST
I'm trying again without the assertions. Hopefully that should avoid the terrifying purpleness.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fe3483946f9
Comment 11 Bill McCloskey (:billm) 2012-02-24 08:11:22 PST
https://hg.mozilla.org/mozilla-central/rev/0fe3483946f9
Comment 12 Christian Holler (:decoder) 2012-03-23 16:56:02 PDT
Test committed with fix, marking verified based on that.
Comment 13 Christian Holler (:decoder) 2013-01-14 08:07:40 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug728086.js.

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