Last Comment Bug 732719 - Assertion failure: allocated(), at ../../jsgc.h:495 or Crash [@ markIfUnmarked]
: Assertion failure: allocated(), at ../../jsgc.h:495 or Crash [@ markIfUnmarked]
Status: VERIFIED FIXED
[sg:moderate] js-triage-needed [advis...
: assertion, crash, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: mozilla13
Assigned To: Bill McCloskey (:billm)
:
:
Mentors:
Depends on:
Blocks: langfuzz IncrementalGC
  Show dependency treegraph
 
Reported: 2012-03-03 11:05 PST by Christian Holler (:decoder)
Modified: 2013-01-19 14:10 PST (History)
7 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
unaffected
-
unaffected
-
unaffected
+
fixed
-
unaffected


Attachments
patch (5.01 KB, patch)
2012-03-05 16:31 PST, Bill McCloskey (:billm)
bhackett1024: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-03-03 11:05:08 PST
The following test asserts on mozilla-central revision 343ec916dfd5 (options -m -n -a):


function TestCase(n, d, e, a)
TestCase.prototype.dump = function () {};
function enterFunc (funcName)
function writeHeaderToLog( string ) {}
gczeal(2);
function f() {}
try {
var BUGNUMBER = 350621;
test();
} catch(exc1) {}
function test() {
  enterFunc ( summary =  this, test(BUGNUMBER));
  function gen1() {  }
  function test_it(RUNS) {  }
}
new TestCase (String([(1),'a22','a23','a24']), 
    String('a11\na22\na23\na24'.match(new RegExp('a..$','g'))));
test();


Stepping through the assertion in the debug build only gives me a division by zero:

Program received signal SIGFPE, Arithmetic exception.
0x080fa54c in js::gc::Arena::isAligned (thing=4149308864, thingSize=0) at /srv/repos/mozilla-central/js/src/jsgc.h:580
580             return tailOffset % thingSize == 0;

However, a less minimized version crashed my opt build:


Program received signal SIGSEGV, Segmentation fault.
markIfUnmarked (gcmarker=0x8321b98, thing=0xf73e5860) at /srv/repos/mozilla-central/js/src/jsgc.h:674
674             if (*word & mask)
(gdb) bt 8
#0  markIfUnmarked (gcmarker=0x8321b98, thing=0xf73e5860) at /srv/repos/mozilla-central/js/src/jsgc.h:674
#1  markIfUnmarked (gcmarker=0x8321b98, thing=0xf73e5860) at /srv/repos/mozilla-central/js/src/jsgc.h:972
#2  js::gc::PushMarkStack (gcmarker=0x8321b98, thing=0xf73e5860) at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:439
#3  0x080ac0a5 in js::gc::MarkInternal<JSObject> (trc=0x8321b98, thing=0xf73e5860) at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:107
#4  0x08185608 in js::StackSpace::markFrameSlots (this=0x8321af0, trc=0x8321b98, fp=0xf7910530, slotsEnd=0xf7910590, pc=0x83514fa "\212")
    at /srv/repos/mozilla-central/js/src/vm/Stack.cpp:488
#5  0x08185823 in js::StackSpace::mark (this=0x8321af0, trc=0x8321b98) at /srv/repos/mozilla-central/js/src/vm/Stack.cpp:521
#6  0x080a20d5 in js::MarkRuntime (trc=<value optimized out>, useSavedRoots=<value optimized out>) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2397
#7  0x080a270f in BeginMarkPhase (rt=0x8321ad0) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2990
(gdb) x /1i $pc
=> 0x80a9c04 <js::gc::PushMarkStack(js::GCMarker*, JSObject*)+68>:      mov    0x4(%edi,%eax,4),%ebp
(gdb) info reg edi eax ebp
edi            0xf7300000       -147849216
eax            0x3fe88  261768
ebp            0x1000   0x1000


Bisect:

The first bad revision is:
changeset:   87140:2a8ceeb27f7c
user:        Bill McCloskey
date:        Fri Feb 17 14:35:20 2012 -0800
summary:     Bug 641025 - Incremental GC (r=igor,smaug,roc,cdleary,gregor)


S-s and sg:critical due to GC related crash/memory hazard.
Comment 1 Bill McCloskey (:billm) 2012-03-05 16:31:28 PST
Created attachment 603103 [details] [diff] [review]
patch

As we discussed, the problem here is that the DEFLOCALFUN opcode doesn't fill in its result until the opcode finishes. However, the GC assumes that the value will be defined at the start of the opcode. So if we GC in the middle of the DefLocalFun stub, the GC will crash trying to mark an invalid value.

This patch (somewhat inelegantly) writes undefined into the slot location before the stub runs. Please let me know if there's a better way to do this. I'm worried about the fact that we're totally circumventing the FrameState here, although maybe that's what needs to happen.
Comment 2 Johnny Stenback (:jst, jst@mozilla.com) 2012-03-07 16:24:56 PST
Bill, do you think this is exploitable at all?
Comment 3 Bill McCloskey (:billm) 2012-03-07 16:37:11 PST
This causes us to mark an invalid pointer. So we'll be walking through memory and setting a bit for each bad pointer we encounter. The bit will be sort-of near the pointer we find. It seems possibly exploitable, but since we're only setting a single bit for each pointer, it would be difficult.
Comment 5 Ed Morley [:emorley] 2012-03-08 13:51:53 PST
https://hg.mozilla.org/mozilla-central/rev/45d1588c2c71
Comment 6 Christian Holler (:decoder) 2013-01-19 14:10:48 PST
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929

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