Bug 1397385 - Fixup js::ComputeImplicitThis to not leak environments
It was possible to leak environments to script and debugger. This patch
simplifies the logic and removes confusing helper functions.
MozReview-Commit-ID: 4jEuYE4Q7bi
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/bug1397385.js
@@ -0,0 +1,15 @@
+var g = newGlobal();
+
+g.evaluate(`
+ function testInnerFun(defaultArg = 1) {
+ function innerFun(expectedThis) { return this; }
+ h();
+ }
+`);
+
+g.h = function () {
+ var res = (new Debugger(g)).getNewestFrame().eval('assertEq(innerFun(), this)');
+ assertEq("return" in res, true);
+}
+
+g.testInnerFun();
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Function/implicit-this-in-parameter-expression.js
@@ -0,0 +1,18 @@
+
+function f(a = eval(`
+ function g() {
+ 'use strict';
+ return this;
+ }
+
+ with ({}) {
+ g() /* implicit return value */
+ }
+ `)) {
+ return a
+};
+
+assertEq(f(), undefined);
+
+if (typeof reportCompare === "function")
+ reportCompare(true, true);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1460,60 +1460,47 @@ HandleError(JSContext* cx, InterpreterRe
*/
JS_STATIC_ASSERT(JSOP_SETNAME_LENGTH == JSOP_SETPROP_LENGTH);
/* See TRY_BRANCH_AFTER_COND. */
JS_STATIC_ASSERT(JSOP_IFNE_LENGTH == JSOP_IFEQ_LENGTH);
JS_STATIC_ASSERT(JSOP_IFNE == JSOP_IFEQ + 1);
/*
- * Compute the implicit |this| parameter for a call expression where the callee
- * funval was resolved from an unqualified name reference to a property on obj
- * (an object on the env chain).
- *
- * We can avoid computing |this| eagerly and push the implicit callee-coerced
- * |this| value, undefined, if either of these conditions hold:
- *
- * 1. The nominal |this|, obj, is a global object.
+ * Compute the implicit |this| value used by a call expression with an
+ * unqualified name reference. The environment the binding was found on is
+ * passed as argument, env.
*
- * 2. The nominal |this|, obj, has one of LexicalEnvironment or Call class (this
- * is what IsCacheableEnvironment tests). Such objects-as-envs must be
- * censored with undefined.
+ * The implicit |this| is |undefined| for all environment types except
+ * WithEnvironmentObject. This is the case for |with(...) {...}| expressions or
+ * if the embedding uses a non-syntactic WithEnvironmentObject.
*
- * Otherwise, we bind |this| to the result of GetThisValue(). Only names inside
- * |with| statements and embedding-specific environment objects fall into this
- * category.
- *
- * If the callee is a strict mode function, then code implementing JSOP_THIS
- * in the interpreter and JITs will leave undefined as |this|. If funval is a
- * function not in strict mode, JSOP_THIS code replaces undefined with funval's
- * global.
+ * NOTE: A non-syntactic WithEnvironmentObject may have a corresponding
+ * extensible LexicalEnviornmentObject, but it will not be considered as an
+ * implicit |this|. This is for compatibility with the Gecko subscript loader.
*/
static inline Value
-ComputeImplicitThis(JSObject* obj)
-{
- if (obj->is<GlobalObject>())
- return UndefinedValue();
-
- if (obj->is<NonSyntacticVariablesObject>())
+ComputeImplicitThis(JSObject* env)
+{
+ // Fast-path for GlobalObject
+ if (env->is<GlobalObject>())
return UndefinedValue();
- if (obj->is<WithEnvironmentObject>())
- return GetThisValueOfWith(obj);
-
- if (IsCacheableEnvironment(obj))
- return UndefinedValue();
+ // WithEnvironmentObjects have an actual implicit |this|
+ if (env->is<WithEnvironmentObject>())
+ return GetThisValueOfWith(env);
// Debugger environments need special casing, as despite being
// non-syntactic, they wrap syntactic environments and should not be
// treated like other embedding-specific non-syntactic environments.
- if (obj->is<DebugEnvironmentProxy>())
- return ComputeImplicitThis(&obj->as<DebugEnvironmentProxy>().environment());
-
- return GetThisValue(obj);
+ if (env->is<DebugEnvironmentProxy>())
+ return ComputeImplicitThis(&env->as<DebugEnvironmentProxy>().environment());
+
+ MOZ_ASSERT(env->is<EnvironmentObject>());
+ return UndefinedValue();
}
static MOZ_ALWAYS_INLINE bool
AddOperation(JSContext* cx, MutableHandleValue lhs, MutableHandleValue rhs, MutableHandleValue res)
{
if (lhs.isInt32() && rhs.isInt32()) {
int32_t l = lhs.toInt32(), r = rhs.toInt32();
int32_t t;
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -23,30 +23,16 @@
#include "jsobjinlines.h"
#include "jsscriptinlines.h"
#include "jit/BaselineFrame-inl.h"
namespace js {
-/*
- * We cache name lookup results only for the global object or for native
- * non-global objects without prototype or with prototype that never mutates,
- * see bug 462734 and bug 487039.
- */
-static inline bool
-IsCacheableEnvironment(JSObject* obj)
-{
- bool cacheable = obj->is<CallObject>() || obj->is<LexicalEnvironmentObject>();
-
- MOZ_ASSERT_IF(cacheable, !obj->getOpsLookupProperty());
- return cacheable;
-}
-
inline HandleObject
InterpreterFrame::environmentChain() const
{
return HandleObject::fromMarkedLocation(&envChain_);
}
inline GlobalObject&
InterpreterFrame::global() const