From 327120e195eb09d5197f07278bbace57045851bc Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Thu, 8 May 2014 21:30:50 -0700 Subject: [PATCH] Bug 1007164 - Throw on touching sentinel values in DebugScopeProxy by default but allow Debugger.Environment.prototype.getVariable access. (r=jimb) --- js/src/jit-test/tests/debug/Frame-eval-12.js | 10 +- js/src/js.msg | 2 +- js/src/vm/CommonPropertyNames.h | 1 + js/src/vm/Debugger.cpp | 30 ++- js/src/vm/Debugger.h | 4 + js/src/vm/ScopeObject.cpp | 207 ++++++++++++++----- js/src/vm/ScopeObject.h | 4 + toolkit/devtools/server/actors/script.js | 19 +- toolkit/devtools/webconsole/utils.js | 9 +- 9 files changed, 202 insertions(+), 84 deletions(-) diff --git a/js/src/jit-test/tests/debug/Frame-eval-12.js b/js/src/jit-test/tests/debug/Frame-eval-12.js index e74e97658ad4..d0f3ffc2f4b8 100644 --- a/js/src/jit-test/tests/debug/Frame-eval-12.js +++ b/js/src/jit-test/tests/debug/Frame-eval-12.js @@ -4,16 +4,10 @@ var g = newGlobal(); var dbg = new Debugger(g); // capture arguments object and test function -var hits = 0; dbg.onDebuggerStatement = function (frame) { - try { - frame.older.environment.parent.getVariable('arguments') - } catch (e) { - assertEq(''+e, "Error: Debugger scope is not live"); - hits++; - } + var args = frame.older.environment.parent.getVariable('arguments'); + assertEq(args.missingArguments, true); }; g.eval("function h() { debugger; }"); g.eval("function f() { var x = 0; return function() { x++; h() } }"); g.eval("f('ponies')()"); -assertEq(hits, 1); diff --git a/js/src/js.msg b/js/src/js.msg index c8496394c163..469b1b24d5e0 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -356,7 +356,7 @@ MSG_DEF(JSMSG_INTRINSIC_NOT_DEFINED, 302, 1, JSEXN_REFERENCEERR, "no intrinsic MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA, 303, 2, JSEXN_ERR, "{0} is being assigned a {1}, but already has one") MSG_DEF(JSMSG_PAR_ARRAY_BAD_ARG, 304, 0, JSEXN_RANGEERR, "invalid parallel method argument") MSG_DEF(JSMSG_REGEXP_RUNTIME_ERROR, 305, 0, JSEXN_INTERNALERR, "an error occurred while executing regular expression") -MSG_DEF(JSMSG_UNUSED306, 306, 0, JSEXN_NONE, "") +MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT, 306, 0, JSEXN_ERR, "variable has been optimized out") MSG_DEF(JSMSG_UNUSED307, 307, 0, JSEXN_NONE, "") MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_CONFLICT, 308, 0, JSEXN_ERR, "no conflict resolution function provided") MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_BOUNDS, 309, 0, JSEXN_ERR, "index in scatter vector out of bounds") diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index 749ab66d0252..9bd593b4bf54 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -147,6 +147,7 @@ macro(of, of, "of") \ macro(offset, offset, "offset") \ macro(optimizedOut, optimizedOut, "optimizedOut") \ + macro(missingArguments, missingArguments, "missingArguments") \ macro(outOfMemory, outOfMemory, "out of memory") \ macro(parseFloat, parseFloat, "parseFloat") \ macro(parseInt, parseInt, "parseInt") \ diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index c442420b2b65..76cae7930ce3 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -755,15 +755,23 @@ Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp) vp.setObject(*dobj); } } else if (vp.isMagic()) { - // Other magic values should not have escaped. - MOZ_ASSERT(vp.whyMagic() == JS_OPTIMIZED_OUT); - RootedObject optObj(cx, NewBuiltinClassInstance(cx, &JSObject::class_)); if (!optObj) return false; + // We handle two sentinel values: missing arguments (overloading + // JS_OPTIMIZED_ARGUMENTS) and optimized out slots (JS_OPTIMIZED_OUT). + // Other magic values should not have escaped. + PropertyName *name; + if (vp.whyMagic() == JS_OPTIMIZED_ARGUMENTS) { + name = cx->names().missingArguments; + } else { + MOZ_ASSERT(vp.whyMagic() == JS_OPTIMIZED_OUT); + name = cx->names().optimizedOut; + } + RootedValue trueVal(cx, BooleanValue(true)); - if (!JSObject::defineProperty(cx, optObj, cx->names().optimizedOut, trueVal)) + if (!JSObject::defineProperty(cx, optObj, name, trueVal)) return false; vp.setObject(*optObj); @@ -5986,8 +5994,18 @@ DebuggerEnv_getVariable(JSContext *cx, unsigned argc, Value *vp) /* This can trigger getters. */ ErrorCopier ec(ac, dbg->toJSObject()); - if (!JSObject::getGeneric(cx, env, env, id, &v)) - return false; + + // For DebugScopeObjects, we get sentinel values for optimized out + // slots and arguments instead of throwing (the default behavior). + // + // See wrapDebuggeeValue for how the sentinel values are wrapped. + if (env->is()) { + if (!env->as().getMaybeSentinelValue(cx, id, &v)) + return false; + } else { + if (!JSObject::getGeneric(cx, env, env, id, &v)) + return false; + } } if (!dbg->wrapDebuggeeValue(cx, &v)) diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index 03863204be0f..c3a5a3835285 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -472,6 +472,10 @@ class Debugger : private mozilla::LinkedListElement * * If *vp is a magic JS_OPTIMIZED_OUT value, this produces a plain object * of the form { optimizedOut: true }. + * + * If *vp is a magic JS_OPTIMIZED_ARGUMENTS value signifying missing + * arguments, this produces a plain object of the form { missingArguments: + * true }. */ bool wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp); diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index 1e8305525e54..6a8a36da04e9 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/js/src/vm/ScopeObject.cpp @@ -29,6 +29,7 @@ using namespace js::types; using mozilla::PodZero; typedef Rooted RootedArgumentsObject; +typedef MutableHandle MutableHandleArgumentsObject; /*****************************************************************************/ @@ -1118,6 +1119,12 @@ class DebugScopeProxy : public BaseProxyHandler { enum Action { SET, GET }; + enum AccessResult { + ACCESS_UNALIASED, + ACCESS_GENERIC, + ACCESS_LOST + }; + /* * This function handles access to unaliased locals/formals. Since they are * unaliased, the values of these variables are not stored in the slots of @@ -1137,13 +1144,17 @@ class DebugScopeProxy : public BaseProxyHandler * - and there was not a DebugScopeObject yet associated with the * scope, then the unaliased values are lost and not recoverable. * - * handleUnaliasedAccess returns 'true' if the access was unaliased and - * completed by handleUnaliasedAccess. + * Callers should check accessResult for non-failure results: + * - ACCESS_UNALIASED if the access was unaliased and completed + * - ACCESS_GENERIC if the access was aliased or the property not found + * - ACCESS_LOST if the value has been lost to the debugger */ - bool handleUnaliasedAccess(JSContext *cx, Handle debugScope, Handle scope, - jsid id, Action action, MutableHandleValue vp) + bool handleUnaliasedAccess(JSContext *cx, Handle debugScope, + Handle scope, jsid id, Action action, + MutableHandleValue vp, AccessResult *accessResult) { JS_ASSERT(&debugScope->scope() == scope); + *accessResult = ACCESS_GENERIC; ScopeIterVal *maybeLiveScope = DebugScopes::hasLiveScope(*scope); /* Handle unaliased formals, vars, and consts at function scope. */ @@ -1158,12 +1169,12 @@ class DebugScopeProxy : public BaseProxyHandler while (bi && NameToId(bi->name()) != id) bi++; if (!bi) - return false; + return true; if (bi->kind() == Binding::VARIABLE || bi->kind() == Binding::CONSTANT) { uint32_t i = bi.frameIndex(); if (script->varIsAliased(i)) - return false; + return true; if (maybeLiveScope) { AbstractFramePtr frame = maybeLiveScope->frame(); @@ -1178,14 +1189,16 @@ class DebugScopeProxy : public BaseProxyHandler snapshot->setDenseElement(bindings.numArgs() + i, vp); } else { /* The unaliased value has been lost to the debugger. */ - if (action == GET) - vp.set(MagicValue(JS_OPTIMIZED_OUT)); + if (action == GET) { + *accessResult = ACCESS_LOST; + return true; + } } } else { JS_ASSERT(bi->kind() == Binding::ARGUMENT); unsigned i = bi.frameIndex(); if (script->formalIsAliased(i)) - return false; + return true; if (maybeLiveScope) { AbstractFramePtr frame = maybeLiveScope->frame(); @@ -1207,14 +1220,17 @@ class DebugScopeProxy : public BaseProxyHandler snapshot->setDenseElement(i, vp); } else { /* The unaliased value has been lost to the debugger. */ - if (action == GET) - vp.set(MagicValue(JS_OPTIMIZED_OUT)); + if (action == GET) { + *accessResult = ACCESS_LOST; + return true; + } } if (action == SET) TypeScript::SetArgument(cx, script, i, vp); } + *accessResult = ACCESS_UNALIASED; return true; } @@ -1223,11 +1239,11 @@ class DebugScopeProxy : public BaseProxyHandler Rooted block(cx, &scope->as()); Shape *shape = block->lastProperty()->search(cx, id); if (!shape) - return false; + return true; unsigned i = block->staticBlock().shapeToIndex(*shape); if (block->staticBlock().isAliased(i)) - return false; + return true; if (maybeLiveScope) { AbstractFramePtr frame = maybeLiveScope->frame(); @@ -1244,13 +1260,14 @@ class DebugScopeProxy : public BaseProxyHandler block->setVar(i, vp, DONT_CHECK_ALIASING); } + *accessResult = ACCESS_UNALIASED; return true; } /* The rest of the internal scopes do not have unaliased vars. */ JS_ASSERT(scope->is() || scope->is() || scope->as().isForEval()); - return false; + return true; } static bool isArguments(JSContext *cx, jsid id) @@ -1276,31 +1293,33 @@ class DebugScopeProxy : public BaseProxyHandler } /* - * This function creates an arguments object when the debugger requests - * 'arguments' for a function scope where the arguments object has been - * optimized away (either because the binding is missing altogether or - * because !ScriptAnalysis::needsArgsObj). + * This function checks if an arguments object needs to be created when + * the debugger requests 'arguments' for a function scope where the + * arguments object has been optimized away (either because the binding is + * missing altogether or because !ScriptAnalysis::needsArgsObj). */ - static bool checkForMissingArguments(JSContext *cx, jsid id, ScopeObject &scope, - ArgumentsObject **maybeArgsObj) + static bool isMissingArguments(JSContext *cx, jsid id, ScopeObject &scope) { - *maybeArgsObj = nullptr; + return isArguments(cx, id) && isFunctionScope(scope) && + !scope.as().callee().nonLazyScript()->needsArgsObj(); + } - if (!isArguments(cx, id) || !isFunctionScope(scope)) - return true; - - if (scope.as().callee().nonLazyScript()->needsArgsObj()) - return true; + /* + * Create a missing arguments object. If the function returns true but + * argsObj is null, it means the scope is dead. + */ + static bool createMissingArguments(JSContext *cx, jsid id, ScopeObject &scope, + MutableHandleArgumentsObject argsObj) + { + MOZ_ASSERT(isMissingArguments(cx, id, scope)); + argsObj.set(nullptr); ScopeIterVal *maybeScope = DebugScopes::hasLiveScope(scope); - if (!maybeScope) { - JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE, - "Debugger scope"); - return false; - } + if (!maybeScope) + return true; - *maybeArgsObj = ArgumentsObject::createUnexpected(cx, maybeScope->frame()); - return true; + argsObj.set(ArgumentsObject::createUnexpected(cx, maybeScope->frame())); + return !!argsObj; } public: @@ -1336,51 +1355,118 @@ class DebugScopeProxy : public BaseProxyHandler Rooted debugScope(cx, &proxy->as()); Rooted scope(cx, &debugScope->scope()); - RootedArgumentsObject maybeArgsObj(cx); - if (!checkForMissingArguments(cx, id, *scope, maybeArgsObj.address())) - return false; + if (isMissingArguments(cx, id, *scope)) { + RootedArgumentsObject argsObj(cx); + if (!createMissingArguments(cx, id, *scope, &argsObj)) + return false; + + if (!argsObj) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE, + "Debugger scope"); + return false; + } - if (maybeArgsObj) { desc.object().set(debugScope); desc.setAttributes(JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT); - desc.value().setObject(*maybeArgsObj); + desc.value().setObject(*argsObj); desc.setGetter(nullptr); desc.setSetter(nullptr); return true; } RootedValue v(cx); - if (handleUnaliasedAccess(cx, debugScope, scope, id, GET, &v)) { + AccessResult access; + if (!handleUnaliasedAccess(cx, debugScope, scope, id, GET, &v, &access)) + return false; + + switch (access) { + case ACCESS_UNALIASED: desc.object().set(debugScope); desc.setAttributes(JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT); desc.value().set(v); desc.setGetter(nullptr); desc.setSetter(nullptr); return true; + case ACCESS_GENERIC: + return JS_GetOwnPropertyDescriptorById(cx, scope, id, desc); + case ACCESS_LOST: + JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_OPTIMIZED_OUT); + return false; + default: + MOZ_ASSUME_UNREACHABLE("bad AccessResult"); } - - return JS_GetOwnPropertyDescriptorById(cx, scope, id, desc); } - bool get(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id, + bool get(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id, MutableHandleValue vp) MOZ_OVERRIDE { Rooted debugScope(cx, &proxy->as()); Rooted scope(cx, &proxy->as().scope()); - RootedArgumentsObject maybeArgsObj(cx); - if (!checkForMissingArguments(cx, id, *scope, maybeArgsObj.address())) - return false; + if (isMissingArguments(cx, id, *scope)) { + RootedArgumentsObject argsObj(cx); + if (!createMissingArguments(cx, id, *scope, &argsObj)) + return false; - if (maybeArgsObj) { - vp.set(ObjectValue(*maybeArgsObj)); + if (!argsObj) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE, + "Debugger scope"); + return false; + } + + vp.setObject(*argsObj); return true; } - if (handleUnaliasedAccess(cx, debugScope, scope, id, GET, vp)) - return true; + AccessResult access; + if (!handleUnaliasedAccess(cx, debugScope, scope, id, GET, vp, &access)) + return false; - return JSObject::getGeneric(cx, scope, scope, id, vp); + switch (access) { + case ACCESS_UNALIASED: + return true; + case ACCESS_GENERIC: + return JSObject::getGeneric(cx, scope, scope, id, vp); + case ACCESS_LOST: + JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_OPTIMIZED_OUT); + return false; + default: + MOZ_ASSUME_UNREACHABLE("bad AccessResult"); + } + } + + /* + * Like 'get', but returns sentinel values instead of throwing on + * exceptional cases. + */ + bool getMaybeSentinelValue(JSContext *cx, Handle debugScope, HandleId id, + MutableHandleValue vp) + { + Rooted scope(cx, &debugScope->scope()); + + if (isMissingArguments(cx, id, *scope)) { + RootedArgumentsObject argsObj(cx); + if (!createMissingArguments(cx, id, *scope, &argsObj)) + return false; + vp.set(argsObj ? ObjectValue(*argsObj) : MagicValue(JS_OPTIMIZED_ARGUMENTS)); + return true; + } + + AccessResult access; + if (!handleUnaliasedAccess(cx, debugScope, scope, id, GET, vp, &access)) + return false; + + switch (access) { + case ACCESS_UNALIASED: + return true; + case ACCESS_GENERIC: + return JSObject::getGeneric(cx, scope, scope, id, vp); + case ACCESS_LOST: + vp.setMagic(JS_OPTIMIZED_OUT); + return true; + default: + MOZ_ASSUME_UNREACHABLE("bad AccessResult"); + } } bool set(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id, bool strict, @@ -1388,9 +1474,19 @@ class DebugScopeProxy : public BaseProxyHandler { Rooted debugScope(cx, &proxy->as()); Rooted scope(cx, &proxy->as().scope()); - if (handleUnaliasedAccess(cx, debugScope, scope, id, SET, vp)) + + AccessResult access; + if (!handleUnaliasedAccess(cx, debugScope, scope, id, SET, vp, &access)) + return false; + + switch (access) { + case ACCESS_UNALIASED: return true; - return JSObject::setGeneric(cx, scope, scope, id, vp, strict); + case ACCESS_GENERIC: + return JSObject::setGeneric(cx, scope, scope, id, vp, strict); + default: + MOZ_ASSUME_UNREACHABLE("bad AccessResult"); + } } bool defineProperty(JSContext *cx, HandleObject proxy, HandleId id, @@ -1551,6 +1647,13 @@ DebugScopeObject::isForDeclarative() const return s.is() || s.is() || s.is(); } +bool +DebugScopeObject::getMaybeSentinelValue(JSContext *cx, HandleId id, MutableHandleValue vp) +{ + Rooted self(cx, this); + return DebugScopeProxy::singleton.getMaybeSentinelValue(cx, self, id, vp); +} + bool js_IsDebugScopeSlow(ProxyObject *proxy) { diff --git a/js/src/vm/ScopeObject.h b/js/src/vm/ScopeObject.h index 217b676eb395..f55df6faf991 100644 --- a/js/src/vm/ScopeObject.h +++ b/js/src/vm/ScopeObject.h @@ -801,6 +801,10 @@ class DebugScopeObject : public ProxyObject /* Currently, the 'declarative' scopes are Call and Block. */ bool isForDeclarative() const; + + // Get a property by 'id', but returns sentinel values instead of throwing + // on exceptional cases. + bool getMaybeSentinelValue(JSContext *cx, HandleId id, MutableHandleValue vp); }; /* Maintains per-compartment debug scope bookkeeping information. */ diff --git a/toolkit/devtools/server/actors/script.js b/toolkit/devtools/server/actors/script.js index 3ddcd62d8645..632355d2efa6 100644 --- a/toolkit/devtools/server/actors/script.js +++ b/toolkit/devtools/server/actors/script.js @@ -4547,6 +4547,8 @@ EnvironmentActor.prototype = { let arg = {}; let value = this.obj.getVariable(name); + // The slot is optimized out. + // FIXME: Need actual UI, bug 941287. if (value && value.optimizedOut) { continue; } @@ -4583,18 +4585,11 @@ EnvironmentActor.prototype = { continue; } - let value; - try { - value = this.obj.getVariable(name); - if (value && value.optimizedOut) { - continue; - } - } catch (e) { - // Avoid "Debugger scope is not live" errors for |arguments|, introduced - // in bug 746601. - if (name != "arguments") { - throw e; - } + let value = this.obj.getVariable(name); + // The slot is optimized out or arguments on a dead scope. + // FIXME: Need actual UI, bug 941287. + if (value && (value.optimizedOut || value.missingArguments)) { + continue; } // TODO: this part should be removed in favor of the commented-out part diff --git a/toolkit/devtools/webconsole/utils.js b/toolkit/devtools/webconsole/utils.js index 9760d0c98aa7..db5e75b3775b 100644 --- a/toolkit/devtools/webconsole/utils.js +++ b/toolkit/devtools/webconsole/utils.js @@ -1087,11 +1087,10 @@ let DebuggerEnvironmentSupport = { getProperty: function(aObj, aName) { // TODO: we should use getVariableDescriptor() here - bug 725815. - let result = undefined; - try { - result = aObj.getVariable(aName); - } catch (ex) { - // getVariable() throws for invalid identifiers. + let result = aObj.getVariable(aName); + // FIXME: Need actual UI, bug 941287. + if (result.optimizedOut || result.missingArguments) { + return null; } return result === undefined ? null : { value: result }; },