Assertion failure: JSVAL_IS_DOUBLE_IMPL(data), at ./dist/include/js/Value.h:1159 with OOM

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
--
critical
RESOLVED WORKSFORME
4 years ago
2 years ago

People

(Reporter: decoder, Unassigned)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Trunk
x86_64
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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){}
(Reporter)

Comment 1

4 years ago
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.
Blocks: 912928
(Reporter)

Comment 2

4 years ago
Created attachment 823971 [details]
[crash-signature] Machine-readable crash signature
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());
Flags: needinfo?(jwalden+bmo)
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.
Flags: needinfo?(jwalden+bmo)
Waldo, if you can't take this, I think we need to fix each one of these the whack-a-mole way. :-\
(Assignee)

Updated

3 years ago
Assignee: general → nobody
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.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.