Bug 674998 - Remove InvokeSessionGuard; it's bug-prone and we should self-host instead (r=bhackett)

This commit is contained in:
Luke Wagner 2011-09-06 09:06:07 -07:00
parent 0038e2e573
commit 54a297e878
7 changed files with 55 additions and 278 deletions

View File

@ -1836,10 +1836,11 @@ js_MergeSort(void *src, size_t nel, size_t elsize,
struct CompareArgs
{
JSContext *context;
InvokeSessionGuard session;
InvokeArgsGuard args;
Value fval;
CompareArgs(JSContext *cx)
: context(cx)
CompareArgs(JSContext *cx, Value fval)
: context(cx), fval(fval)
{}
};
@ -1860,15 +1861,21 @@ sort_compare(void *arg, const void *a, const void *b, int *result)
if (!JS_CHECK_OPERATION_LIMIT(cx))
return JS_FALSE;
InvokeSessionGuard &session = ca->session;
session[0] = *av;
session[1] = *bv;
InvokeArgsGuard &args = ca->args;
if (!args.pushed() && !cx->stack.pushInvokeArgs(cx, 2, &args))
return JS_FALSE;
args.calleeHasBeenReset();
args.calleev() = ca->fval;
args.thisv() = UndefinedValue();
args[0] = *av;
args[1] = *bv;
if (!session.invoke(cx))
if (!Invoke(cx, args))
return JS_FALSE;
jsdouble cmp;
if (!ToNumber(cx, session.rval(), &cmp))
if (!ToNumber(cx, args.rval(), &cmp))
return JS_FALSE;
/* Clamp cmp to -1, 0, 1. */
@ -2105,10 +2112,7 @@ js::array_sort(JSContext *cx, uintN argc, Value *vp)
} while (++i != newlen);
}
} else {
CompareArgs ca(cx);
if (!ca.session.start(cx, fval, UndefinedValue(), 2))
return false;
CompareArgs ca(cx, fval);
if (!js_MergeSort(vec, size_t(newlen), sizeof(Value),
comparator_stack_cast(sort_compare),
&ca, mergesort_tmp,
@ -2962,16 +2966,13 @@ array_extra(JSContext *cx, ArrayExtraMode mode, uintN argc, Value *vp)
*/
argc = 3 + REDUCE_MODE(mode);
InvokeSessionGuard session;
if (!session.start(cx, ObjectValue(*callable), thisv, argc))
return JS_FALSE;
MUST_FLOW_THROUGH("out");
JSBool ok = JS_TRUE;
JSBool cond;
Value objv = ObjectValue(*obj);
AutoValueRooter tvr(cx);
InvokeArgsGuard args;
for (jsuint i = start; i != end; i += step) {
JSBool hole;
ok = JS_CHECK_OPERATION_LIMIT(cx) &&
@ -2981,23 +2982,29 @@ array_extra(JSContext *cx, ArrayExtraMode mode, uintN argc, Value *vp)
if (hole)
continue;
if (!args.pushed() && !cx->stack.pushInvokeArgs(cx, argc, &args))
return false;
/*
* Push callable and 'this', then args. We must do this for every
* iteration around the loop since Invoke clobbers its arguments.
*/
args.calleeHasBeenReset();
args.calleev() = ObjectValue(*callable);
args.thisv() = thisv;
uintN argi = 0;
if (REDUCE_MODE(mode))
session[argi++] = *vp;
session[argi++] = tvr.value();
session[argi++] = Int32Value(i);
session[argi] = objv;
args[argi++] = *vp;
args[argi++] = tvr.value();
args[argi++] = Int32Value(i);
args[argi] = objv;
/* Do the call. */
ok = session.invoke(cx);
ok = Invoke(cx, args);
if (!ok)
break;
const Value &rval = session.rval();
const Value &rval = args.rval();
if (mode > MAP)
cond = js_ValueToBoolean(rval);

View File

@ -623,8 +623,6 @@ js::RunScript(JSContext *cx, JSScript *script, StackFrame *fp)
bool
js::InvokeKernel(JSContext *cx, const CallArgs &argsRef, MaybeConstruct construct)
{
/* N.B. Must be kept in sync with InvokeSessionGuard::start/invoke */
CallArgs args = argsRef;
JS_ASSERT(args.argc() <= StackSpace::ARGS_LENGTH_MAX);
@ -661,6 +659,12 @@ js::InvokeKernel(JSContext *cx, const CallArgs &argsRef, MaybeConstruct construc
if (fun->isNative())
return CallJSNative(cx, fun->u.n.native, args);
#ifdef JS_TRACER
if (TRACE_RECORDER(cx))
AbortRecording(cx, "attempt to reenter VM while recording");
LeaveTrace(cx);
#endif
TypeMonitorCall(cx, args, construct);
/* Get pointer to new frame/slots, prepare arguments. */
@ -685,104 +689,6 @@ js::InvokeKernel(JSContext *cx, const CallArgs &argsRef, MaybeConstruct construc
return ok;
}
bool
InvokeSessionGuard::start(JSContext *cx, const Value &calleev, const Value &thisv, uintN argc)
{
#ifdef JS_TRACER
if (TRACE_RECORDER(cx))
AbortRecording(cx, "attempt to reenter VM while recording");
LeaveTrace(cx);
#endif
/* Always push arguments, regardless of optimized/normal invoke. */
ContextStack &stack = cx->stack;
if (!stack.pushInvokeArgs(cx, argc, &args_))
return false;
/* Callees may clobber 'this' or 'callee'. */
savedCallee_ = args_.calleev() = calleev;
savedThis_ = args_.thisv() = thisv;
/* If anyone (through jsdbgapi) finds this frame, make it safe. */
MakeRangeGCSafe(args_.argv(), args_.argc());
do {
/* Hoist dynamic checks from scripted Invoke. */
if (!calleev.isObject())
break;
JSObject &callee = calleev.toObject();
if (callee.getClass() != &FunctionClass)
break;
JSFunction *fun = callee.getFunctionPrivate();
if (fun->isNative())
break;
script_ = fun->script();
if (fun->isHeavyweight())
break;
/*
* The frame will remain pushed even when the callee isn't active which
* will affect the observable current global, so avoid any change.
*/
if (callee.getGlobal() != GetGlobalForScopeChain(cx))
break;
/* Push the stack frame once for the session. */
if (!stack.pushInvokeFrame(cx, args_, INITIAL_NONE, &ifg_))
return false;
/*
* Update the 'this' type of the callee according to the value given,
* along with the types of any missing arguments. These will be the
* same across all calls.
*/
TypeScript::SetThis(cx, script_, thisv);
for (unsigned i = argc; i < fun->nargs; i++)
TypeScript::SetArgument(cx, script_, i, types::Type::UndefinedType());
StackFrame *fp = ifg_.fp();
#ifdef JS_METHODJIT
/* Hoist dynamic checks from RunScript. */
mjit::CompileStatus status = mjit::CanMethodJIT(cx, script_, false,
mjit::CompileRequest_JIT);
if (status == mjit::Compile_Error)
return false;
if (status != mjit::Compile_Okay)
break;
/* Cannot also cache the raw code pointer; it can change. */
/* Hoist dynamic checks from CheckStackAndEnterMethodJIT. */
JS_CHECK_RECURSION(cx, return false);
stackLimit_ = stack.space().getStackLimit(cx, REPORT_ERROR);
if (!stackLimit_)
return false;
stop_ = script_->code + script_->length - 1;
JS_ASSERT(*stop_ == JSOP_STOP);
#endif
/* Cached to avoid canonicalActualArg in InvokeSessionGuard::operator[]. */
nformals_ = fp->numFormalArgs();
formals_ = fp->formalArgs();
actuals_ = args_.argv();
JS_ASSERT(actuals_ == fp->actualArgs());
return true;
} while (0);
/*
* Use the normal invoke path.
*
* The callee slot gets overwritten during an unoptimized Invoke, so we
* cache it here and restore it before every Invoke call. The 'this' value
* does not get overwritten, so we can fill it here once.
*/
if (ifg_.pushed())
ifg_.pop();
formals_ = actuals_ = args_.argv();
nformals_ = (unsigned)-1;
return true;
}
bool
js::Invoke(JSContext *cx, const Value &thisv, const Value &fval, uintN argc, Value *argv,
Value *rval)

View File

@ -187,32 +187,6 @@ extern bool
InvokeGetterOrSetter(JSContext *cx, JSObject *obj, const Value &fval, uintN argc, Value *argv,
Value *rval);
/*
* Natives like sort/forEach/replace call Invoke repeatedly with the same
* callee, this, and number of arguments. To optimize this, such natives can
* start an "invoke session" to factor out much of the dynamic setup logic
* required by a normal Invoke. Usage is:
*
* InvokeSessionGuard session(cx);
* if (!session.start(cx, callee, thisp, argc, &session))
* ...
*
* while (...) {
* // write actual args (not callee, this)
* session[0] = ...
* ...
* session[argc - 1] = ...
*
* if (!session.invoke(cx, session))
* ...
*
* ... = session.rval();
* }
*
* // session ended by ~InvokeSessionGuard
*/
class InvokeSessionGuard;
/*
* InvokeConstructor* implement a function call from a constructor context
* (e.g. 'new') handling the the creation of the new 'this' object.

View File

@ -71,108 +71,6 @@ class AutoPreserveEnumerators {
}
};
class InvokeSessionGuard
{
InvokeArgsGuard args_;
InvokeFrameGuard ifg_;
Value savedCallee_, savedThis_;
Value *formals_, *actuals_;
unsigned nformals_;
JSScript *script_;
Value *stackLimit_;
jsbytecode *stop_;
bool optimized() const { return ifg_.pushed(); }
public:
InvokeSessionGuard() : args_(), ifg_() {}
~InvokeSessionGuard() {}
bool start(JSContext *cx, const Value &callee, const Value &thisv, uintN argc);
bool invoke(JSContext *cx);
bool started() const {
return args_.pushed();
}
Value &operator[](unsigned i) const {
JS_ASSERT(i < argc());
Value &arg = i < nformals_ ? formals_[i] : actuals_[i];
JS_ASSERT_IF(optimized(), &arg == &ifg_.fp()->canonicalActualArg(i));
JS_ASSERT_IF(!optimized(), &arg == &args_[i]);
return arg;
}
uintN argc() const {
return args_.argc();
}
const Value &rval() const {
return optimized() ? ifg_.fp()->returnValue() : args_.rval();
}
};
inline bool
InvokeSessionGuard::invoke(JSContext *cx)
{
/* N.B. Must be kept in sync with Invoke */
/* Refer to canonical (callee, this) for optimized() sessions. */
formals_[-2] = savedCallee_;
formals_[-1] = savedThis_;
/* Prevent spurious accessing-callee-after-rval assert. */
args_.calleeHasBeenReset();
if (!optimized())
return Invoke(cx, args_);
/*
* Update the types of each argument. The 'this' type and missing argument
* types were handled when the invoke session was created.
*/
for (unsigned i = 0; i < Min(argc(), nformals_); i++)
types::TypeScript::SetArgument(cx, script_, i, (*this)[i]);
#ifdef JS_METHODJIT
mjit::JITScript *jit = script_->getJIT(false /* !constructing */);
if (!jit) {
/* Watch in case the code was thrown away due a recompile. */
mjit::CompileStatus status = mjit::TryCompile(cx, script_, false);
if (status == mjit::Compile_Error)
return false;
JS_ASSERT(status == mjit::Compile_Okay);
jit = script_->getJIT(false);
}
void *code;
if (!(code = jit->invokeEntry))
return Invoke(cx, args_);
#endif
/* Clear any garbage left from the last Invoke. */
StackFrame *fp = ifg_.fp();
fp->resetCallFrame(script_);
JSBool ok;
{
AutoPreserveEnumerators preserve(cx);
args_.setActive(); /* From js::Invoke(InvokeArgsGuard) overload. */
Probes::enterJSFun(cx, fp->fun(), script_);
#ifdef JS_METHODJIT
ok = mjit::EnterMethodJIT(cx, fp, code, stackLimit_, /* partial = */ false);
cx->regs().pc = stop_;
#else
cx->regs().pc = script_->code;
ok = Interpret(cx, cx->fp());
#endif
Probes::exitJSFun(cx, fp->fun(), script_);
args_.setInactive();
}
/* Don't clobber callee with rval; rval gets read from fp->rval. */
return ok;
}
namespace detail {
template<typename T> class PrimitiveBehavior { };

View File

@ -134,7 +134,6 @@ class AutoStringRooter;
class ExecuteArgsGuard;
class InvokeFrameGuard;
class InvokeArgsGuard;
class InvokeSessionGuard;
class StringBuffer;
class TraceRecorder;
struct TraceMonitor;

View File

@ -1622,8 +1622,7 @@ struct ReplaceData
jsint leftIndex; /* left context index in str->chars */
JSSubString dollarStr; /* for "$$" InterpretDollar result */
bool calledBack; /* record whether callback has been called */
InvokeSessionGuard session; /* arguments for repeated lambda Invoke call */
InvokeArgsGuard singleShot; /* arguments for single lambda Invoke call */
InvokeArgsGuard args; /* arguments for lambda call */
StringBuffer sb; /* buffer built during DoMatch */
};
@ -1750,6 +1749,10 @@ FindReplaceLength(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, size_t
JSObject *lambda = rdata.lambda;
if (lambda) {
PreserveRegExpStatics staticsGuard(res);
if (!staticsGuard.init(cx))
return false;
/*
* In the lambda case, not only do we find the replacement string's
* length, we compute repstr and return it via rdata for use within
@ -1761,36 +1764,33 @@ FindReplaceLength(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, size_t
uintN p = res->parenCount();
uintN argc = 1 + p + 2;
InvokeSessionGuard &session = rdata.session;
if (!session.started()) {
Value lambdav = ObjectValue(*lambda);
if (!session.start(cx, lambdav, UndefinedValue(), argc))
return false;
}
PreserveRegExpStatics staticsGuard(res);
if (!staticsGuard.init(cx))
InvokeArgsGuard &args = rdata.args;
if (!args.pushed() && !cx->stack.pushInvokeArgs(cx, argc, &args))
return false;
args.calleeHasBeenReset();
args.calleev() = ObjectValue(*lambda);
args.thisv() = UndefinedValue();
/* Push $&, $1, $2, ... */
uintN argi = 0;
if (!res->createLastMatch(cx, &session[argi++]))
if (!res->createLastMatch(cx, &args[argi++]))
return false;
for (size_t i = 0; i < res->parenCount(); ++i) {
if (!res->createParen(cx, i + 1, &session[argi++]))
if (!res->createParen(cx, i + 1, &args[argi++]))
return false;
}
/* Push match index and input string. */
session[argi++].setInt32(res->matchStart());
session[argi].setString(rdata.str);
args[argi++].setInt32(res->matchStart());
args[argi].setString(rdata.str);
if (!session.invoke(cx))
if (!Invoke(cx, args))
return false;
/* root repstr: rdata is on the stack, so scanned by conservative gc. */
JSString *repstr = ValueToString_TestForStringInline(cx, session.rval());
JSString *repstr = ValueToString_TestForStringInline(cx, args.rval());
if (!repstr)
return false;
rdata.repstr = repstr->ensureLinear(cx);
@ -2082,10 +2082,10 @@ str_replace_flat_lambda(JSContext *cx, uintN argc, Value *vp, ReplaceData &rdata
/* lambda(matchStr, matchStart, textstr) */
static const uint32 lambdaArgc = 3;
if (!cx->stack.pushInvokeArgs(cx, lambdaArgc, &rdata.singleShot))
if (!cx->stack.pushInvokeArgs(cx, lambdaArgc, &rdata.args))
return false;
CallArgs &args = rdata.singleShot;
CallArgs &args = rdata.args;
args.calleev().setObject(*rdata.lambda);
args.thisv().setUndefined();
@ -2094,7 +2094,7 @@ str_replace_flat_lambda(JSContext *cx, uintN argc, Value *vp, ReplaceData &rdata
sp[1].setInt32(fm.match());
sp[2].setString(rdata.str);
if (!Invoke(cx, rdata.singleShot))
if (!Invoke(cx, rdata.args))
return false;
JSString *repstr = js_ValueToString(cx, args.rval());

View File

@ -1018,13 +1018,6 @@ StackIter::settleOnNewState()
continue;
}
/* Censor pushed-but-not-active frames from InvokeSessionGuard. */
if (containsCall && !calls_->active() && fp_->hasArgs() &&
calls_->argv() == fp_->actualArgs()) {
popFrame();
continue;
}
/*
* As an optimization, there is no CallArgsList element pushed for
* natives called directly by a script (compiled or interpreted).