Last Comment Bug 915336 - Assertion failure: JSVAL_IS_DOUBLE_IMPL(data), at ./dist/include/js/Value.h:1159 with OOM
: Assertion failure: JSVAL_IS_DOUBLE_IMPL(data), at ./dist/include/js/Value.h:1...
Status: RESOLVED WORKSFORME
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
-- critical (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: general
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: langfuzz 912928
  Show dependency treegraph
 
Reported: 2013-09-11 12:23 PDT by Christian Holler (:decoder)
Modified: 2015-10-06 03:58 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
[crash-signature] Machine-readable crash signature (353 bytes, text/plain)
2013-10-29 05:58 PDT, Christian Holler (:decoder)
no flags Details

Description User image Christian Holler (:decoder) 2013-09-11 12:23:41 PDT
The following testcase asserts on mozilla-central revision 9e9f74116749 (run with --fuzzing-safe):


typeof document != "object" || !document.location.href.match(/jsreftest.html/)
try {
new ArrayBuffer( 
  eval("\
    var valof=String.prototype.valueOf;\
    astring=new Number();\
    astring.valueOf = valof;\
    astring.valueOf()\
  ") 
);
} catch(exc3) {}
var month = 1000 * 30;
for (var step = month; step > 0; step = Math.floor(step / 3)) {};
new Date(2000, 11, 20, 0, 0, 0, 0);
Boolean(Number.NEGATIVE_INFINITY) 
oomAfterAllocations(286);
for(i in this){}
Comment 1 User image Christian Holler (:decoder) 2013-10-28 06:11:56 PDT
This doesn't reproduce anymore on tip but I'm still seeing it. We should debug this on the original revision and get rid of it because this assertion can typically also indicate a security problem. I'll attach a signature soon.
Comment 2 User image Christian Holler (:decoder) 2013-10-29 05:58:17 PDT
Created attachment 823971 [details]
[crash-signature] Machine-readable crash signature
Comment 3 User image Jan de Mooij [:jandem] 2013-10-29 07:54:39 PDT
Waldo, this is an Intl problem. We're triggering OOM under createBlankPrototype here:

    RootedObject proto(cx, global->createBlankPrototype(cx, &CollatorClass));
    if (!proto)
        return false;
    proto->setReservedSlot(UCOLLATOR_SLOT, PrivateValue(NULL));

However, at the point of the OOM we already allocated the JSObject so when we "return false" here we don't properly initialize its UCOLLATOR_SLOT. GC then crashes in collator_finalize because UCOLLATOR_SLOT is UndefinedValue, not PrivateValue.

static void
collator_finalize(FreeOp *fop, JSObject *obj)
{
    UCollator *coll = static_cast<UCollator*>(obj->getReservedSlot(UCOLLATOR_SLOT).toPrivate());
Comment 4 User image Jeff Walden [:Waldo] (remove +bmo to email) 2013-10-29 10:43:58 PDT
Not an Intl problem.  It is a long-standing idiom to be able to create an object of a given class, then initialize its reserved slots.  If this is busted, then GlobalObject::createBlankPrototype is busted, and whatever's underneath it.  That happens to be js::NewObjectWithGivenProto and js::NewObject, specifically this bit of it:

    if (newKind == SingletonObject) {
        RootedObject nobj(cx, obj);
        if (!JSObject::setSingletonType(cx, nobj))
            return nullptr;
        obj = nobj;
    }

So basically the problem is we don't indicate the object has singleton type when we create it.  We have to create the object, and give it a singleton type, as one atomic operation -- no intermediate states should ever be visible.

I imagine this isn't too difficult to fix, just requires a little plumbing to get singleton-ness set at the start on the initial object shape.
Comment 5 User image Jason Orendorff [:jorendorff] 2013-12-21 19:47:20 PST
Waldo, if you can't take this, I think we need to fix each one of these the whack-a-mole way. :-\
Comment 6 User image Jon Coppeard (:jonco) 2015-10-06 03:58:55 PDT
This no longer reproduces.

I can't find the bug but I remember the issue described in comment 4 being fixed in another bug.  Certainly that code isn't there anymore.

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