Bug 1298640 - Track top-level functions in eval/global bodies for all-or-nothing redeclaration checks. (r=Waldo)
authorShu-yu Guo <shu@rfrn.org>
Fri, 02 Sep 2016 15:30:48 -0700
changeset 312573 701075b5e63c16a45fa0b9bbac8001e88ff98454
parent 312572 88222d0a92668a99c47602aa2a0968df4ebe7e98
child 312574 6c65ad93a66da0d5d9ceb7cec97d9c1c161a5aba
push id30646
push userryanvm@gmail.com
push dateSat, 03 Sep 2016 15:33:40 +0000
treeherdermozilla-central@1789229965bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1298640
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1298640 - Track top-level functions in eval/global bodies for all-or-nothing redeclaration checks. (r=Waldo)
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
js/src/frontend/SharedContext.h
js/src/js.msg
js/src/tests/ecma_6/LexicalEnvironment/eval-nondefinable-function.js
js/src/vm/EnvironmentObject.cpp
js/src/vm/EnvironmentObject.h
js/src/vm/Interpreter.cpp
js/src/vm/Scope.cpp
js/src/vm/Scope.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -1165,50 +1165,39 @@ BytecodeEmitter::EmitterScope::enterFunc
     if (!appendScopeNote(bce))
         return false;
 
     return checkEnvironmentChainLength(bce);
 }
 
 class DynamicBindingIter : public BindingIter
 {
-    uint32_t functionEnd_;
-
   public:
     explicit DynamicBindingIter(GlobalSharedContext* sc)
-      : BindingIter(*sc->bindings),
-        functionEnd_(sc->functionBindingEnd)
-    {
-        MOZ_ASSERT(functionEnd_ >= varStart_ && functionEnd_ <= letStart_);
-    }
+      : BindingIter(*sc->bindings)
+    { }
 
     explicit DynamicBindingIter(EvalSharedContext* sc)
-      : BindingIter(*sc->bindings, /* strict = */ false),
-        functionEnd_(sc->functionBindingEnd)
+      : BindingIter(*sc->bindings, /* strict = */ false)
     {
         MOZ_ASSERT(!sc->strict());
-        MOZ_ASSERT(functionEnd_ >= varStart_ && functionEnd_ <= letStart_);
     }
 
     JSOp bindingOp() const {
         switch (kind()) {
           case BindingKind::Var:
             return JSOP_DEFVAR;
           case BindingKind::Let:
             return JSOP_DEFLET;
           case BindingKind::Const:
             return JSOP_DEFCONST;
           default:
             MOZ_CRASH("Bad BindingKind");
         }
     }
-
-    bool isBodyLevelFunction() const {
-        return index_ < functionEnd_;
-    }
 };
 
 bool
 BytecodeEmitter::EmitterScope::enterGlobal(BytecodeEmitter* bce, GlobalSharedContext* globalsc)
 {
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     bce->setVarEmitterScope(this);
@@ -1238,17 +1227,17 @@ BytecodeEmitter::EmitterScope::enterGlob
         for (DynamicBindingIter bi(globalsc); bi; bi++) {
             NameLocation loc = NameLocation::fromBinding(bi.kind(), bi.location());
             JSAtom* name = bi.name();
             if (!putNameInCache(bce, name, loc))
                 return false;
 
             // Define the name in the prologue. Do not emit DEFVAR for
             // functions that we'll emit DEFFUN for.
-            if (bi.isBodyLevelFunction())
+            if (bi.isTopLevelFunction())
                 continue;
 
             if (!bce->emitAtomOp(name, bi.bindingOp()))
                 return false;
         }
     }
 
     // Note that to save space, we don't add free names to the cache for
@@ -1298,17 +1287,17 @@ BytecodeEmitter::EmitterScope::enterEval
         // scopes may introduce 'var' bindings to the nearest var scope.
         //
         // TODO: We may optimize strict eval bindings in the future to be on
         // the frame. For now, handle everything dynamically.
         if (!hasEnvironment() && evalsc->bindings) {
             for (DynamicBindingIter bi(evalsc); bi; bi++) {
                 MOZ_ASSERT(bi.bindingOp() == JSOP_DEFVAR);
 
-                if (bi.isBodyLevelFunction())
+                if (bi.isTopLevelFunction())
                     continue;
 
                 if (!bce->emitAtomOp(bi.name(), JSOP_DEFVAR))
                     return false;
             }
         }
 
         // As an optimization, if the eval does not have its own var
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -270,17 +270,16 @@ SharedContext::computeInWith(Scope* scop
     }
 }
 
 EvalSharedContext::EvalSharedContext(ExclusiveContext* cx, JSObject* enclosingEnv,
                                      Scope* enclosingScope, Directives directives,
                                      bool extraWarnings)
   : SharedContext(cx, Kind::Eval, directives, extraWarnings),
     enclosingScope_(cx, enclosingScope),
-    functionBindingEnd(0),
     bindings(cx)
 {
     computeAllowSyntax(enclosingScope);
     computeInWith(enclosingScope);
     computeThisBinding(enclosingScope);
 
     // Like all things Debugger, Debugger.Frame.eval needs special
     // handling. Since the environment chain of such evals are non-syntactic
@@ -1369,18 +1368,17 @@ NewEmptyBindingData(ExclusiveContext* cx
         return nullptr;
     }
     PodZero(bindings);
     return bindings;
 }
 
 template <>
 Maybe<GlobalScope::Data*>
-Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope,
-                                             uint32_t* functionBindingEnd)
+Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope)
 {
     Vector<BindingName> funs(context);
     Vector<BindingName> vars(context);
     Vector<BindingName> lets(context);
     Vector<BindingName> consts(context);
 
     bool allBindingsClosedOver = pc->sc()->allBindingsClosedOver();
     for (BindingIter bi = scope.bindings(pc); bi; bi++) {
@@ -1415,22 +1413,20 @@ Parser<FullParseHandler>::newGlobalScope
         bindings = NewEmptyBindingData<GlobalScope>(context, alloc, numBindings);
         if (!bindings)
             return Nothing();
 
         // The ordering here is important. See comments in GlobalScope.
         BindingName* start = bindings->names;
         BindingName* cursor = start;
 
-        // Keep track of what vars are functions. This is only used in BCE to omit
-        // superfluous DEFVARs.
         PodCopy(cursor, funs.begin(), funs.length());
         cursor += funs.length();
-        *functionBindingEnd = cursor - start;
-
+
+        bindings->varStart = cursor - start;
         PodCopy(cursor, vars.begin(), vars.length());
         cursor += vars.length();
 
         bindings->letStart = cursor - start;
         PodCopy(cursor, lets.begin(), lets.length());
         cursor += lets.length();
 
         bindings->constStart = cursor - start;
@@ -1505,18 +1501,17 @@ Parser<FullParseHandler>::newModuleScope
         bindings->length = numBindings;
     }
 
     return Some(bindings);
 }
 
 template <>
 Maybe<EvalScope::Data*>
-Parser<FullParseHandler>::newEvalScopeData(ParseContext::Scope& scope,
-                                           uint32_t* functionBindingEnd)
+Parser<FullParseHandler>::newEvalScopeData(ParseContext::Scope& scope)
 {
     Vector<BindingName> funs(context);
     Vector<BindingName> vars(context);
 
     for (BindingIter bi = scope.bindings(pc); bi; bi++) {
         // Eval scopes only contain 'var' bindings. Make all bindings aliased
         // for now.
         MOZ_ASSERT(bi.kind() == BindingKind::Var);
@@ -1540,18 +1535,18 @@ Parser<FullParseHandler>::newEvalScopeDa
 
         BindingName* start = bindings->names;
         BindingName* cursor = start;
 
         // Keep track of what vars are functions. This is only used in BCE to omit
         // superfluous DEFVARs.
         PodCopy(cursor, funs.begin(), funs.length());
         cursor += funs.length();
-        *functionBindingEnd = cursor - start;
-
+
+        bindings->varStart = cursor - start;
         PodCopy(cursor, vars.begin(), vars.length());
         bindings->length = numBindings;
     }
 
     return Some(bindings);
 }
 
 template <>
@@ -1825,24 +1820,20 @@ Parser<FullParseHandler>::evalBody(EvalS
         MOZ_ASSERT(!si.done(),
                    "Eval must have found an enclosing function box scope that allows super.property");
     }
 #endif
 
     if (!FoldConstants(context, &body, this))
         return nullptr;
 
-    uint32_t functionBindingEnd = 0;
-    Maybe<EvalScope::Data*> bindings =
-        newEvalScopeData(pc->varScope(), &functionBindingEnd);
+    Maybe<EvalScope::Data*> bindings = newEvalScopeData(pc->varScope());
     if (!bindings)
         return nullptr;
-
     evalsc->bindings = *bindings;
-    evalsc->functionBindingEnd = functionBindingEnd;
 
     return body;
 }
 
 template <>
 ParseNode*
 Parser<FullParseHandler>::globalBody(GlobalSharedContext* globalsc)
 {
@@ -1859,23 +1850,20 @@ Parser<FullParseHandler>::globalBody(Glo
         return nullptr;
 
     if (!checkStatementsEOF())
         return nullptr;
 
     if (!FoldConstants(context, &body, this))
         return nullptr;
 
-    uint32_t functionBindingEnd = 0;
-    Maybe<GlobalScope::Data*> bindings = newGlobalScopeData(pc->varScope(), &functionBindingEnd);
+    Maybe<GlobalScope::Data*> bindings = newGlobalScopeData(pc->varScope());
     if (!bindings)
         return nullptr;
-
     globalsc->bindings = *bindings;
-    globalsc->functionBindingEnd = functionBindingEnd;
 
     return body;
 }
 
 template <>
 ParseNode*
 Parser<FullParseHandler>::moduleBody(ModuleSharedContext* modulesc)
 {
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -1285,21 +1285,19 @@ class Parser final : private JS::AutoGCR
                                                     DeclarationKind kind, TokenPos pos);
     bool noteDeclaredName(HandlePropertyName name, DeclarationKind kind, TokenPos pos);
     bool noteUsedName(HandlePropertyName name);
     bool hasUsedName(HandlePropertyName name);
 
     // Required on Scope exit.
     bool propagateFreeNamesAndMarkClosedOverBindings(ParseContext::Scope& scope);
 
-    mozilla::Maybe<GlobalScope::Data*> newGlobalScopeData(ParseContext::Scope& scope,
-                                                          uint32_t* functionBindingEnd);
+    mozilla::Maybe<GlobalScope::Data*> newGlobalScopeData(ParseContext::Scope& scope);
     mozilla::Maybe<ModuleScope::Data*> newModuleScopeData(ParseContext::Scope& scope);
-    mozilla::Maybe<EvalScope::Data*> newEvalScopeData(ParseContext::Scope& scope,
-                                                      uint32_t* functionBindingEnd);
+    mozilla::Maybe<EvalScope::Data*> newEvalScopeData(ParseContext::Scope& scope);
     mozilla::Maybe<FunctionScope::Data*> newFunctionScopeData(ParseContext::Scope& scope,
                                                               bool hasParameterExprs);
     mozilla::Maybe<VarScope::Data*> newVarScopeData(ParseContext::Scope& scope);
     mozilla::Maybe<LexicalScope::Data*> newLexicalScopeData(ParseContext::Scope& scope);
     Node finishLexicalScope(ParseContext::Scope& scope, Node body);
 
     Node propertyName(YieldHandling yieldHandling, Node propList,
                       PropertyType* propType, MutableHandleAtom propAtom);
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -375,32 +375,22 @@ class SharedContext
     }
 };
 
 class MOZ_STACK_CLASS GlobalSharedContext : public SharedContext
 {
     ScopeKind scopeKind_;
 
   public:
-    // We omit DEFFVAR in the prologue for global functions since we emit
-    // DEFFUN for them. In order to distinguish function vars, functions are
-    // ordered before vars. See Parser::newGlobalScopeData and
-    // EmitterScope::enterGlobal.
-    //
-    // This is only used in BytecodeEmitter, and is thus not kept in
-    // GlobalScope::Data.
-    uint32_t functionBindingEnd;
-
     Rooted<GlobalScope::Data*> bindings;
 
     GlobalSharedContext(ExclusiveContext* cx, ScopeKind scopeKind, Directives directives,
                         bool extraWarnings)
       : SharedContext(cx, Kind::Global, directives, extraWarnings),
         scopeKind_(scopeKind),
-        functionBindingEnd(0),
         bindings(cx)
     {
         MOZ_ASSERT(scopeKind == ScopeKind::Global || scopeKind == ScopeKind::NonSyntactic);
         thisBinding_ = ThisBinding::Global;
     }
 
     Scope* compilationEnclosingScope() const override {
         return nullptr;
@@ -418,25 +408,16 @@ SharedContext::asGlobalContext()
     return static_cast<GlobalSharedContext*>(this);
 }
 
 class MOZ_STACK_CLASS EvalSharedContext : public SharedContext
 {
     RootedScope enclosingScope_;
 
   public:
-    // We omit DEFFVAR in the prologue for body-level functions since we emit
-    // DEFFUN for them. In order to distinguish function vars, functions are
-    // ordered before vars. See Parser::newEvalScopeData and
-    // EmitterScope::enterEval.
-    //
-    // This is only used in BytecodeEmitter, and is thus not kept in
-    // EvalScope::Data.
-    uint32_t functionBindingEnd;
-
     Rooted<EvalScope::Data*> bindings;
 
     EvalSharedContext(ExclusiveContext* cx, JSObject* enclosingEnv, Scope* enclosingScope,
                       Directives directives, bool extraWarnings);
 
     Scope* compilationEnclosingScope() const override {
         return enclosingScope_;
     }
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -113,16 +113,17 @@ MSG_DEF(JSMSG_JSON_CYCLIC_VALUE,       1
 MSG_DEF(JSMSG_BAD_INSTANCEOF_RHS,      1, JSEXN_TYPEERR, "invalid 'instanceof' operand {0}")
 MSG_DEF(JSMSG_BAD_LEFTSIDE_OF_ASS,     0, JSEXN_REFERENCEERR, "invalid assignment left-hand side")
 MSG_DEF(JSMSG_BAD_PROTOTYPE,           1, JSEXN_TYPEERR, "'prototype' property of {0} is not an object")
 MSG_DEF(JSMSG_IN_NOT_OBJECT,           1, JSEXN_TYPEERR, "invalid 'in' operand {0}")
 MSG_DEF(JSMSG_TOO_MANY_CON_SPREADARGS, 0, JSEXN_RANGEERR, "too many constructor arguments")
 MSG_DEF(JSMSG_TOO_MANY_FUN_SPREADARGS, 0, JSEXN_RANGEERR, "too many function arguments")
 MSG_DEF(JSMSG_UNINITIALIZED_LEXICAL,   1, JSEXN_REFERENCEERR, "can't access lexical declaration `{0}' before initialization")
 MSG_DEF(JSMSG_BAD_CONST_ASSIGN,        1, JSEXN_TYPEERR, "invalid assignment to const `{0}'")
+MSG_DEF(JSMSG_CANT_DECLARE_GLOBAL_BINDING, 2, JSEXN_TYPEERR, "cannot declare global binding `{0}': {1}")
 
 // Date
 MSG_DEF(JSMSG_INVALID_DATE,            0, JSEXN_RANGEERR, "invalid date")
 MSG_DEF(JSMSG_BAD_TOISOSTRING_PROP,    0, JSEXN_TYPEERR, "toISOString property is not callable")
 
 // String
 MSG_DEF(JSMSG_BAD_URI,                 0, JSEXN_URIERR, "malformed URI sequence")
 MSG_DEF(JSMSG_INVALID_NORMALIZE_FORM,  0, JSEXN_RANGEERR, "form must be one of 'NFC', 'NFD', 'NFKC', or 'NFKD'")
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/LexicalEnvironment/eval-nondefinable-function.js
@@ -0,0 +1,10 @@
+try {
+  eval("var shouldNotBeDefined1; function NaN(){}; var shouldNotBeDefined2;");
+} catch (e) {
+}
+
+assertEq(Object.getOwnPropertyDescriptor(this, 'shouldNotBeDefined2'), undefined);
+assertEq(Object.getOwnPropertyDescriptor(this, 'shouldNotBeDefined1'), undefined);
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -3177,38 +3177,109 @@ js::CheckVarNameConflict(JSContext* cx, 
 {
     if (Shape* shape = lexicalEnv->lookup(cx, name)) {
         ReportRuntimeRedeclaration(cx, name, shape->writable() ? "let" : "const");
         return false;
     }
     return true;
 }
 
+static void
+ReportCannotDeclareGlobalBinding(JSContext* cx, HandlePropertyName name, const char* reason)
+{
+    JSAutoByteString printable;
+    if (AtomToPrintableString(cx, name, &printable)) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                             JSMSG_CANT_DECLARE_GLOBAL_BINDING,
+                             printable.ptr(), reason);
+    }
+}
+
+bool
+js::CheckCanDeclareGlobalBinding(JSContext* cx, Handle<GlobalObject*> global,
+                                 HandlePropertyName name, bool isFunction)
+{
+    RootedId id(cx, NameToId(name));
+    Rooted<PropertyDescriptor> desc(cx);
+    if (!GetOwnPropertyDescriptor(cx, global, id, &desc))
+        return false;
+
+    // ES 8.1.14.15 CanDeclareGlobalVar
+    // ES 8.1.14.16 CanDeclareGlobalFunction
+
+    // Step 4.
+    if (!desc.object()) {
+        // 8.1.14.15 step 6.
+        // 8.1.14.16 step 5.
+        if (global->nonProxyIsExtensible())
+            return true;
+
+        ReportCannotDeclareGlobalBinding(cx, name, "global is non-extensible");
+        return false;
+    }
+
+    // Global functions have additional restrictions.
+    if (isFunction) {
+        // 8.1.14.16 step 6.
+        if (desc.configurable())
+            return true;
+
+        // 8.1.14.16 step 7.
+        if (desc.isDataDescriptor() && desc.writable() && desc.enumerable())
+            return true;
+
+        ReportCannotDeclareGlobalBinding(cx, name,
+                                         "property must be configurable or "
+                                         "both writable and enumerable");
+        return false;
+    }
+
+    return true;
+}
+
 bool
 js::CheckGlobalDeclarationConflicts(JSContext* cx, HandleScript script,
                                     Handle<LexicalEnvironmentObject*> lexicalEnv,
                                     HandleObject varObj)
 {
     // Due to the extensibility of the global lexical environment, we must
     // check for redeclaring a binding.
     //
     // In the case of non-syntactic environment chains, we are checking
     // redeclarations against the non-syntactic lexical environment and the
     // variables object that the lexical environment corresponds to.
     RootedPropertyName name(cx);
     Rooted<BindingIter> bi(cx, BindingIter(script));
 
+    // ES 15.1.11 GlobalDeclarationInstantiation
+
+    // Step 6.
+    //
+    // Check 'var' declarations do not conflict with existing bindings in the
+    // global lexical environment.
     for (; bi; bi++) {
         if (bi.kind() != BindingKind::Var)
             break;
         name = bi.name()->asPropertyName();
         if (!CheckVarNameConflict(cx, lexicalEnv, name))
             return false;
+
+        // Step 10 and 12.
+        //
+        // Check that global functions and vars may be declared.
+        if (varObj->is<GlobalObject>()) {
+            Handle<GlobalObject*> global = varObj.as<GlobalObject>();
+            if (!CheckCanDeclareGlobalBinding(cx, global, name, bi.isTopLevelFunction()))
+                return false;
+        }
     }
 
+    // Step 5.
+    //
+    // Check that lexical bindings do not conflict.
     for (; bi; bi++) {
         name = bi.name()->asPropertyName();
         if (!CheckLexicalNameConflict(cx, lexicalEnv, varObj, name))
             return false;
     }
 
     return true;
 }
@@ -3251,26 +3322,41 @@ bool
 js::CheckEvalDeclarationConflicts(JSContext* cx, HandleScript script,
                                   HandleObject scopeChain, HandleObject varObj)
 {
     if (!script->bodyScope()->as<EvalScope>().hasBindings())
         return true;
 
     RootedObject obj(cx, scopeChain);
 
-    // ES6 18.2.1.2 step d
+    // ES 18.2.1.3.
+
+    // Step 5.
     //
     // Check that a direct eval will not hoist 'var' bindings over lexical
     // bindings with the same name.
     while (obj != varObj) {
         if (!CheckVarNameConflictsInEnv(cx, script, obj))
             return false;
         obj = obj->enclosingEnvironment();
     }
 
+    // Step 8.
+    //
+    // Check that global functions may be declared.
+    if (varObj->is<GlobalObject>()) {
+        Handle<GlobalObject*> global = varObj.as<GlobalObject>();
+        RootedPropertyName name(cx);
+        for (Rooted<BindingIter> bi(cx, BindingIter(script)); bi; bi++) {
+            name = bi.name()->asPropertyName();
+            if (!CheckCanDeclareGlobalBinding(cx, global, name, bi.isTopLevelFunction()))
+                return false;
+        }
+    }
+
     return true;
 }
 
 bool
 js::InitFunctionEnvironmentObjects(JSContext* cx, AbstractFramePtr frame)
 {
     MOZ_ASSERT(frame.isFunctionFrame());
     MOZ_ASSERT(frame.callee()->needsSomeEnvironmentObject());
--- a/js/src/vm/EnvironmentObject.h
+++ b/js/src/vm/EnvironmentObject.h
@@ -1078,16 +1078,20 @@ MOZ_MUST_USE bool
 GetThisValueForDebuggerMaybeOptimizedOut(JSContext* cx, AbstractFramePtr frame,
                                          jsbytecode* pc, MutableHandleValue res);
 
 MOZ_MUST_USE bool
 CheckVarNameConflict(JSContext* cx, Handle<LexicalEnvironmentObject*> lexicalEnv,
                      HandlePropertyName name);
 
 MOZ_MUST_USE bool
+CheckCanDeclareGlobalBinding(JSContext* cx, Handle<GlobalObject*> global,
+                             HandlePropertyName name, bool isFunction);
+
+MOZ_MUST_USE bool
 CheckLexicalNameConflict(JSContext* cx, Handle<LexicalEnvironmentObject*> lexicalEnv,
                          HandleObject varObj, HandlePropertyName name);
 
 MOZ_MUST_USE bool
 CheckGlobalDeclarationConflicts(JSContext* cx, HandleScript script,
                                 Handle<LexicalEnvironmentObject*> lexicalEnv,
                                 HandleObject varObj);
 
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -4391,25 +4391,19 @@ js::DefFunOperation(JSContext* cx, Handl
      * scope object (for aliased).
      */
     MOZ_ASSERT(parent->isNative() || parent->is<DebugEnvironmentProxy>());
     if (parent->is<GlobalObject>()) {
         if (shape->configurable()) {
             if (!DefineProperty(cx, parent, name, rval, nullptr, nullptr, attrs))
                 return false;
         } else {
-            if (shape->isAccessorDescriptor() || !shape->writable() || !shape->enumerable()) {
-                JSAutoByteString bytes;
-                if (AtomToPrintableString(cx, name, &bytes)) {
-                    JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_REDEFINE_PROP,
-                                         bytes.ptr());
-                }
-
-                return false;
-            }
+            MOZ_ASSERT(shape->isDataDescriptor());
+            MOZ_ASSERT(shape->writable());
+            MOZ_ASSERT(shape->enumerable());
         }
 
         // Careful: the presence of a shape, even one appearing to derive from
         // a variable declaration, doesn't mean it's in [[VarNames]].
         if (!parent->compartment()->addToVarNames(cx, name))
             return false;
     }
 
--- a/js/src/vm/Scope.cpp
+++ b/js/src/vm/Scope.cpp
@@ -1200,78 +1200,82 @@ BindingIter::BindingIter(JSScript* scrip
 void
 BindingIter::init(LexicalScope::Data& data, uint32_t firstFrameSlot, uint8_t flags)
 {
     // Named lambda scopes can only have environment slots. If the callee
     // isn't closed over, it is accessed via JSOP_CALLEE.
     if (flags & IsNamedLambda) {
         // Named lambda binding is weird. Normal BindingKind ordering rules
         // don't apply.
-        init(0, 0, 0, 0, 0,
+        init(0, 0, 0, 0, 0, 0,
              CanHaveEnvironmentSlots | flags,
              firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_),
              data.names, data.length);
     } else {
         //            imports - [0, 0)
         // positional formals - [0, 0)
         //      other formals - [0, 0)
+        //    top-level funcs - [0, 0)
         //               vars - [0, 0)
         //               lets - [0, data.constStart)
         //             consts - [data.constStart, data.length)
-        init(0, 0, 0, 0, data.constStart,
+        init(0, 0, 0, 0, 0, data.constStart,
              CanHaveFrameSlots | CanHaveEnvironmentSlots | flags,
              firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_),
              data.names, data.length);
     }
 }
 
 void
 BindingIter::init(FunctionScope::Data& data, uint8_t flags)
 {
     flags = CanHaveFrameSlots | CanHaveEnvironmentSlots | flags;
     if (!(flags & HasFormalParameterExprs))
         flags |= CanHaveArgumentSlots;
 
     //            imports - [0, 0)
     // positional formals - [0, data.nonPositionalFormalStart)
     //      other formals - [data.nonPositionalParamStart, data.varStart)
+    //    top-level funcs - [data.varStart, data.varStart)
     //               vars - [data.varStart, data.length)
     //               lets - [data.length, data.length)
     //             consts - [data.length, data.length)
-    init(0, data.nonPositionalFormalStart, data.varStart, data.length, data.length,
+    init(0, data.nonPositionalFormalStart, data.varStart, data.varStart, data.length, data.length,
          flags,
          0, JSSLOT_FREE(&CallObject::class_),
          data.names, data.length);
 }
 
 void
 BindingIter::init(VarScope::Data& data, uint32_t firstFrameSlot)
 {
     //            imports - [0, 0)
     // positional formals - [0, 0)
     //      other formals - [0, 0)
+    //    top-level funcs - [0, 0)
     //               vars - [0, data.length)
     //               lets - [data.length, data.length)
     //             consts - [data.length, data.length)
-    init(0, 0, 0, data.length, data.length,
+    init(0, 0, 0, 0, data.length, data.length,
          CanHaveFrameSlots | CanHaveEnvironmentSlots,
          firstFrameSlot, JSSLOT_FREE(&VarEnvironmentObject::class_),
          data.names, data.length);
 }
 
 void
 BindingIter::init(GlobalScope::Data& data)
 {
     //            imports - [0, 0)
     // positional formals - [0, 0)
     //      other formals - [0, 0)
-    //               vars - [0, data.letStart)
+    //    top-level funcs - [0, data.varStart)
+    //               vars - [data.varStart, data.letStart)
     //               lets - [data.letStart, data.constStart)
     //             consts - [data.constStart, data.length)
-    init(0, 0, 0, data.letStart, data.constStart,
+    init(0, 0, 0, data.varStart, data.letStart, data.constStart,
          CannotHaveSlots,
          UINT32_MAX, UINT32_MAX,
          data.names, data.length);
 }
 
 void
 BindingIter::init(EvalScope::Data& data, bool strict)
 {
@@ -1286,34 +1290,36 @@ BindingIter::init(EvalScope::Data& data,
         flags = CannotHaveSlots;
         firstFrameSlot = UINT32_MAX;
         firstEnvironmentSlot = UINT32_MAX;
     }
 
     //            imports - [0, 0)
     // positional formals - [0, 0)
     //      other formals - [0, 0)
-    //               vars - [0, data.length)
+    //    top-level funcs - [0, data.varStart)
+    //               vars - [data.varStart, data.length)
     //               lets - [data.length, data.length)
     //             consts - [data.length, data.length)
-    init(0, 0, 0, data.length, data.length,
+    init(0, 0, 0, data.varStart, data.length, data.length,
          flags, firstFrameSlot, firstEnvironmentSlot,
          data.names, data.length);
 }
 
 void
 BindingIter::init(ModuleScope::Data& data)
 {
     //            imports - [0, data.varStart)
     // positional formals - [data.varStart, data.varStart)
     //      other formals - [data.varStart, data.varStart)
+    //    top-level funcs - [data.varStart, data.varStart)
     //               vars - [data.varStart, data.letStart)
     //               lets - [data.letStart, data.constStart)
     //             consts - [data.constStart, data.length)
-    init(data.varStart, data.varStart, data.varStart, data.letStart, data.constStart,
+    init(data.varStart, data.varStart, data.varStart, data.varStart, data.letStart, data.constStart,
          CanHaveFrameSlots | CanHaveEnvironmentSlots,
          0, JSSLOT_FREE(&ModuleEnvironmentObject::class_),
          data.names, data.length);
 }
 
 PositionalFormalParameterIter::PositionalFormalParameterIter(JSScript* script)
   : BindingIter(script)
 {
--- a/js/src/vm/Scope.h
+++ b/js/src/vm/Scope.h
@@ -623,19 +623,21 @@ class GlobalScope : public Scope
 
   public:
     // Data is public because it is created by the frontend. See
     // Parser<FullParseHandler>::newGlobalScopeData.
     struct Data
     {
         // Bindings are sorted by kind.
         //
-        //   vars - [0, letStart)
-        //   lets - [letStart, constStart)
-        // consts - [constStart, length)
+        // top-level funcs - [0, varStart)
+        //            vars - [varStart, letStart)
+        //            lets - [letStart, constStart)
+        //          consts - [constStart, length)
+        uint32_t varStart;
         uint32_t letStart;
         uint32_t constStart;
         uint32_t length;
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
         BindingName names[1];
 
@@ -717,17 +719,22 @@ class EvalScope : public Scope
 
   public:
     // Data is public because it is created by the frontend. See
     // Parser<FullParseHandler>::newEvalScopeData.
     struct Data
     {
         // All bindings in an eval script are 'var' bindings. The implicit
         // lexical scope around the eval is present regardless of strictness
-        // and is its own LexicalScope.
+        // and is its own LexicalScope. However, we need to track top-level
+        // functions specially for redeclaration checks.
+        //
+        // top-level funcs - [0, varStart)
+        //            vars - [varStart, length)
+        uint32_t varStart;
         uint32_t length;
 
         // Frame slots [0, nextFrameSlot) are live when this is the innermost
         // scope.
         uint32_t nextFrameSlot;
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
@@ -887,40 +894,44 @@ class BindingIter
   protected:
     // Bindings are sorted by kind. Because different Scopes have differently
     // laid out Data for packing, BindingIter must handle all binding kinds.
     //
     // Kind ranges:
     //
     //            imports - [0, positionalFormalStart)
     // positional formals - [positionalFormalStart, nonPositionalFormalStart)
-    //      other formals - [nonPositionalParamStart, varStart)
+    //      other formals - [nonPositionalParamStart, topLevelFunctionStart)
+    //    top-level funcs - [topLevelFunctionStart, varStart)
     //               vars - [varStart, letStart)
     //               lets - [letStart, constStart)
     //             consts - [constStart, length)
     //
     // Access method when not closed over:
     //
     //            imports - name
     // positional formals - argument slot
     //      other formals - frame slot
+    //    top-level funcs - frame slot
     //               vars - frame slot
     //               lets - frame slot
     //             consts - frame slot
     //
     // Access method when closed over:
     //
     //            imports - name
-    // positional formals - environment slot
-    //      other formals - environment slot
-    //               vars - environment slot
-    //               lets - environment slot
-    //             consts - environment slot
+    // positional formals - environment slot or name
+    //      other formals - environment slot or name
+    //    top-level funcs - environment slot or name
+    //               vars - environment slot or name
+    //               lets - environment slot or name
+    //             consts - environment slot or name
     uint32_t positionalFormalStart_;
     uint32_t nonPositionalFormalStart_;
+    uint32_t topLevelFunctionStart_;
     uint32_t varStart_;
     uint32_t letStart_;
     uint32_t constStart_;
     uint32_t length_;
 
     uint32_t index_;
 
     enum Flags : uint8_t {
@@ -942,22 +953,24 @@ class BindingIter
     uint8_t flags_;
     uint16_t argumentSlot_;
     uint32_t frameSlot_;
     uint32_t environmentSlot_;
 
     BindingName* names_;
 
     void init(uint32_t positionalFormalStart, uint32_t nonPositionalFormalStart,
-              uint32_t varStart, uint32_t letStart, uint32_t constStart,
+              uint32_t topLevelFunctionStart, uint32_t varStart,
+              uint32_t letStart, uint32_t constStart,
               uint8_t flags, uint32_t firstFrameSlot, uint32_t firstEnvironmentSlot,
               BindingName* names, uint32_t length)
     {
         positionalFormalStart_ = positionalFormalStart;
         nonPositionalFormalStart_ = nonPositionalFormalStart;
+        topLevelFunctionStart_ = topLevelFunctionStart;
         varStart_ = varStart;
         letStart_ = letStart;
         constStart_ = constStart;
         length_ = length;
         index_ = 0;
         flags_ = flags;
         argumentSlot_ = 0;
         frameSlot_ = firstFrameSlot;
@@ -1108,32 +1121,37 @@ class BindingIter
         MOZ_ASSERT(isNamedLambda());
         return BindingLocation::NamedLambdaCallee();
     }
 
     BindingKind kind() const {
         MOZ_ASSERT(!done());
         if (index_ < positionalFormalStart_)
             return BindingKind::Import;
-        if (index_ < varStart_) {
+        if (index_ < topLevelFunctionStart_) {
             // When the parameter list has expressions, the parameters act
             // like lexical bindings and have TDZ.
             if (hasFormalParameterExprs())
                 return BindingKind::Let;
             return BindingKind::FormalParameter;
         }
         if (index_ < letStart_)
             return BindingKind::Var;
         if (index_ < constStart_)
             return BindingKind::Let;
         if (isNamedLambda())
             return BindingKind::NamedLambdaCallee;
         return BindingKind::Const;
     }
 
+    bool isTopLevelFunction() const {
+        MOZ_ASSERT(!done());
+        return index_ >= topLevelFunctionStart_ && index_ < varStart_;
+    }
+
     bool hasArgumentSlot() const {
         MOZ_ASSERT(!done());
         if (hasFormalParameterExprs())
             return false;
         return index_ >= positionalFormalStart_ && index_ < nonPositionalFormalStart_;
     }
 
     uint16_t argumentSlot() const {
@@ -1262,16 +1280,17 @@ class BindingIterOperations
     bool isLast() const { return iter().isLast(); }
     bool canHaveArgumentSlots() const { return iter().canHaveArgumentSlots(); }
     bool canHaveFrameSlots() const { return iter().canHaveFrameSlots(); }
     bool canHaveEnvironmentSlots() const { return iter().canHaveEnvironmentSlots(); }
     JSAtom* name() const { return iter().name(); }
     bool closedOver() const { return iter().closedOver(); }
     BindingLocation location() const { return iter().location(); }
     BindingKind kind() const { return iter().kind(); }
+    bool isTopLevelFunction() const { return iter().isTopLevelFunction(); }
     bool hasArgumentSlot() const { return iter().hasArgumentSlot(); }
     uint16_t argumentSlot() const { return iter().argumentSlot(); }
     uint32_t nextFrameSlot() const { return iter().nextFrameSlot(); }
     uint32_t nextEnvironmentSlot() const { return iter().nextEnvironmentSlot(); }
 };
 
 template <typename Outer>
 class MutableBindingIterOperations : public BindingIterOperations<Outer>