Bug 908920 - Don't assume suspended generator activations have a script. r=jorendorff

This patch makes legacy (JS 1.7) and star (ES6) generators have
different classes, avoiding the need to poke through to the frame's
script to see which kind they are.  This steps around a failure to get
a frame's script after GC, when the script was thrown away, while also
preparing for bug 907744.

To detect errors in the future, also make accesses to gen->fp->script()
cause a null pointer deref in debug mode.  This caught another bug in
generator_close_impl(), in which with GGC it could be possible to
reference a return value from the recently dead frame.  This was easily
fixed because that value is always undefined.
This commit is contained in:
Ryan VanderMeulen
2013-08-27 14:01:11 -04:00
parent 4c07dce2a7
commit def3c6d947
3 changed files with 90 additions and 36 deletions

View File

@@ -1000,27 +1000,21 @@ js::ValueToIterator(JSContext *cx, unsigned flags, MutableHandleValue vp)
}
bool
IsGenerator(HandleValue v)
IsStarGeneratorObject(HandleValue v)
{
return v.isObject() && v.toObject().is<GeneratorObject>();
}
static bool
IsLegacyGenerator(HandleObject obj)
{
if (!obj->is<GeneratorObject>())
return false;
JSGenerator *gen = obj->as<GeneratorObject>().getGenerator();
return gen->regs.fp()->script()->isLegacyGenerator();
return v.isObject() && v.toObject().is<StarGeneratorObject>();
}
bool
IsLegacyGenerator(HandleValue v)
IsLegacyGeneratorObject(HandleValue v)
{
if (!IsGenerator(v))
return false;
JSGenerator *gen = v.toObject().as<GeneratorObject>().getGenerator();
return gen->regs.fp()->script()->isLegacyGenerator();
return v.isObject() && v.toObject().is<LegacyGeneratorObject>();
}
bool
IsGeneratorObject(HandleValue v)
{
return IsLegacyGeneratorObject(v) || IsStarGeneratorObject(v);
}
bool
@@ -1044,7 +1038,7 @@ js::CloseIterator(JSContext *cx, HandleObject obj)
*/
ni->props_cursor = ni->props_array;
}
} else if (IsLegacyGenerator(obj)) {
} else if (obj->is<LegacyGeneratorObject>()) {
return CloseLegacyGenerator(cx, obj);
}
return true;
@@ -1337,10 +1331,19 @@ Class StopIterationObject::class_ = {
/*** Generators **********************************************************************************/
static JSGenerator*
GetGenerator(JSObject *obj)
{
if (obj->is<LegacyGeneratorObject>())
return obj->as<LegacyGeneratorObject>().getGenerator();
JS_ASSERT(obj->is<StarGeneratorObject>());
return obj->as<StarGeneratorObject>().getGenerator();
}
static void
generator_finalize(FreeOp *fop, JSObject *obj)
{
JSGenerator *gen = obj->as<GeneratorObject>().getGenerator();
JSGenerator *gen = GetGenerator(obj);
if (!gen)
return;
@@ -1351,7 +1354,9 @@ generator_finalize(FreeOp *fop, JSObject *obj)
JS_ASSERT(gen->state == JSGEN_NEWBORN ||
gen->state == JSGEN_CLOSED ||
gen->state == JSGEN_OPEN);
JS_POISON(gen->fp, JS_FREE_PATTERN, sizeof(StackFrame));
// If gen->state is JSGEN_CLOSED, gen->fp may be NULL.
if (gen->fp)
JS_POISON(gen->fp, JS_FREE_PATTERN, sizeof(StackFrame));
JS_POISON(gen, JS_FREE_PATTERN, sizeof(JSGenerator));
fop->free_(gen);
}
@@ -1408,6 +1413,15 @@ SetGeneratorClosed(JSContext *cx, JSGenerator *gen)
if (GeneratorHasMarkableFrame(gen))
GeneratorWriteBarrierPre(cx, gen);
gen->state = JSGEN_CLOSED;
#ifdef DEBUG
MakeRangeGCSafe(gen->fp->generatorArgsSnapshotBegin(),
gen->fp->generatorArgsSnapshotEnd());
MakeRangeGCSafe(gen->fp->generatorSlotsSnapshotBegin(),
gen->regs.sp);
PodZero(&gen->regs, 1);
gen->fp = NULL;
#endif
}
GeneratorState::GeneratorState(JSContext *cx, JSGenerator *gen, JSGeneratorState futureState)
@@ -1453,7 +1467,7 @@ GeneratorState::pushInterpreterFrame(JSContext *cx, FrameGuard *)
static void
generator_trace(JSTracer *trc, JSObject *obj)
{
JSGenerator *gen = obj->as<GeneratorObject>().getGenerator();
JSGenerator *gen = GetGenerator(obj);
if (!gen)
return;
@@ -1461,7 +1475,30 @@ generator_trace(JSTracer *trc, JSObject *obj)
MarkGeneratorFrame(trc, gen);
}
Class GeneratorObject::class_ = {
Class LegacyGeneratorObject::class_ = {
"Generator",
JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS,
JS_PropertyStub, /* addProperty */
JS_DeletePropertyStub, /* delProperty */
JS_PropertyStub, /* getProperty */
JS_StrictPropertyStub, /* setProperty */
JS_EnumerateStub,
JS_ResolveStub,
JS_ConvertStub,
generator_finalize,
NULL, /* checkAccess */
NULL, /* call */
NULL, /* hasInstance */
NULL, /* construct */
generator_trace,
{
NULL, /* outerObject */
NULL, /* innerObject */
iterator_iteratorObject,
}
};
Class StarGeneratorObject::class_ = {
"Generator",
JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS,
JS_PropertyStub, /* addProperty */
@@ -1515,13 +1552,13 @@ js_NewGenerator(JSContext *cx, const FrameRegs &stackRegs)
if (!proto)
return NULL;
}
obj = NewObjectWithGivenProto(cx, &GeneratorObject::class_, proto, global);
obj = NewObjectWithGivenProto(cx, &StarGeneratorObject::class_, proto, global);
} else {
JS_ASSERT(stackfp->script()->isLegacyGenerator());
JSObject *proto = global->getOrCreateLegacyGeneratorObjectPrototype(cx);
if (!proto)
return NULL;
obj = NewObjectWithGivenProto(cx, &GeneratorObject::class_, proto, global);
obj = NewObjectWithGivenProto(cx, &LegacyGeneratorObject::class_, proto, global);
}
if (!obj)
return NULL;
@@ -1658,9 +1695,9 @@ SendToGenerator(JSContext *cx, JSGeneratorOp op, HandleObject obj,
static bool
CloseLegacyGenerator(JSContext *cx, HandleObject obj)
{
JS_ASSERT(IsLegacyGenerator(obj));
JS_ASSERT(obj->is<LegacyGeneratorObject>());
JSGenerator *gen = obj->as<GeneratorObject>().getGenerator();
JSGenerator *gen = GetGenerator(obj);
if (gen->state == JSGEN_CLOSED)
return true;
@@ -1670,11 +1707,11 @@ CloseLegacyGenerator(JSContext *cx, HandleObject obj)
JS_ALWAYS_INLINE bool
generator_next_impl(JSContext *cx, CallArgs args)
{
JS_ASSERT(IsGenerator(args.thisv()));
JS_ASSERT(IsGeneratorObject(args.thisv()));
RootedObject thisObj(cx, &args.thisv().toObject());
JSGenerator *gen = thisObj->as<GeneratorObject>().getGenerator();
JSGenerator *gen = GetGenerator(thisObj);
if (gen->state == JSGEN_CLOSED)
return js_ThrowStopIteration(cx);
@@ -1696,17 +1733,17 @@ bool
generator_next(JSContext *cx, unsigned argc, Value *vp)
{
CallArgs args = CallArgsFromVp(argc, vp);
return CallNonGenericMethod<IsGenerator, generator_next_impl>(cx, args);
return CallNonGenericMethod<IsGeneratorObject, generator_next_impl>(cx, args);
}
JS_ALWAYS_INLINE bool
generator_throw_impl(JSContext *cx, CallArgs args)
{
JS_ASSERT(IsGenerator(args.thisv()));
JS_ASSERT(IsGeneratorObject(args.thisv()));
RootedObject thisObj(cx, &args.thisv().toObject());
JSGenerator *gen = thisObj->as<GeneratorObject>().getGenerator();
JSGenerator *gen = GetGenerator(thisObj);
if (gen->state == JSGEN_CLOSED) {
cx->setPendingException(args.length() >= 1 ? args[0] : UndefinedValue());
return false;
@@ -1723,17 +1760,17 @@ bool
generator_throw(JSContext *cx, unsigned argc, Value *vp)
{
CallArgs args = CallArgsFromVp(argc, vp);
return CallNonGenericMethod<IsGenerator, generator_throw_impl>(cx, args);
return CallNonGenericMethod<IsGeneratorObject, generator_throw_impl>(cx, args);
}
JS_ALWAYS_INLINE bool
generator_close_impl(JSContext *cx, CallArgs args)
{
JS_ASSERT(IsLegacyGenerator(args.thisv()));
JS_ASSERT(IsLegacyGeneratorObject(args.thisv()));
RootedObject thisObj(cx, &args.thisv().toObject());
JSGenerator *gen = thisObj->as<GeneratorObject>().getGenerator();
JSGenerator *gen = GetGenerator(thisObj);
if (gen->state == JSGEN_CLOSED) {
args.rval().setUndefined();
return true;
@@ -1748,7 +1785,7 @@ generator_close_impl(JSContext *cx, CallArgs args)
if (!SendToGenerator(cx, JSGENOP_CLOSE, thisObj, gen, JS::UndefinedHandleValue))
return false;
args.rval().set(gen->fp->returnValue());
args.rval().setUndefined();
return true;
}
@@ -1756,7 +1793,7 @@ bool
generator_close(JSContext *cx, unsigned argc, Value *vp)
{
CallArgs args = CallArgsFromVp(argc, vp);
return CallNonGenericMethod<IsLegacyGenerator, generator_close_impl>(cx, args);
return CallNonGenericMethod<IsLegacyGeneratorObject, generator_close_impl>(cx, args);
}
#define JSPROP_ROPERM (JSPROP_READONLY | JSPROP_PERMANENT)