Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Bug 714614: don't create call objects with duplicated property names,…

… r=bhackett, a=akeybl
  • Loading branch information...
commit 83b79beccda21df15a3f65a0c4d142f4738424d1 1 parent 9c44977
David Mandelin authored
View
1  js/src/frontend/Parser.cpp
@@ -1332,6 +1332,7 @@ Parser::functionArguments(TreeContext &funtc, FunctionBox *funbox, ParseNode **l
* until after all arguments have been parsed.
*/
if (funtc.decls.lookupFirst(name)) {
+ funtc.bindings.noteDup();
duplicatedArg = name;
if (destructuringArg)
goto report_dup_and_destructuring;
View
5 js/src/jit-test/tests/basic/bug714614.js
@@ -0,0 +1,5 @@
+function testForVarInWith(foo, foo) {
+ return eval("with ({}) { for (var x = 0; x < 5; x++); } (function() { return delete x; })");
+}
+f = testForVarInWith()();
+
View
1  js/src/jsscope.h
@@ -466,7 +466,6 @@ struct Shape : public js::gc::Cell
friend class js::Bindings;
friend struct js::StackShape;
friend struct js::StackBaseShape;
- friend bool IsShapeAboutToBeFinalized(JSContext *cx, const js::Shape *shape);
protected:
HeapPtrBaseShape base_;
View
41 js/src/jsscript.cpp
@@ -184,6 +184,47 @@ Bindings::add(JSContext *cx, JSAtom *name, BindingKind kind)
return true;
}
+Shape *
+Bindings::callObjectShape(JSContext *cx) const
+{
+ if (!hasDup())
+ return lastShape();
+
+ /*
+ * Build a vector of non-duplicate properties in order from last added
+ * to first (i.e., the order we normally have iterate over Shapes). Choose
+ * the last added property in each set of dups.
+ */
+ Vector<const Shape *> shapes(cx);
+ HashSet<jsid> seen(cx);
+ if (!seen.init())
+ return false;
+
+ for (Shape::Range r = lastShape()->all(); !r.empty(); r.popFront()) {
+ const Shape &s = r.front();
+ HashSet<jsid>::AddPtr p = seen.lookupForAdd(s.propid());
+ if (!p) {
+ if (!seen.add(p, s.propid()))
+ return NULL;
+ if (!shapes.append(&s))
+ return NULL;
+ }
+ }
+
+ /*
+ * Now build the Shape without duplicate properties.
+ */
+ RootedVarShape shape(cx);
+ shape = initialShape(cx);
+ for (int i = shapes.length() - 1; i >= 0; --i) {
+ shape = shape->getChildBinding(cx, shapes[i]);
+ if (!shape)
+ return NULL;
+ }
+
+ return shape;
+}
+
bool
Bindings::getLocalNameArray(JSContext *cx, Vector<JSAtom *> *namesp)
{
View
13 js/src/jsscript.h
@@ -175,7 +175,9 @@ class Bindings {
uint16_t nargs;
uint16_t nvars;
uint16_t nupvars;
+ bool hasDup_:1; // true if there are duplicate argument names
+ inline Shape *initialShape(JSContext *cx) const;
public:
inline Bindings(JSContext *cx);
@@ -207,9 +209,15 @@ class Bindings {
/* Ensure these bindings have a shape lineage. */
inline bool ensureShape(JSContext *cx);
- /* Returns the shape lineage generated for these bindings. */
+ /* Return the shape lineage generated for these bindings. */
inline Shape *lastShape() const;
+ /*
+ * Return the shape to use to create a call object for these bindings.
+ * The result is guaranteed not to have duplicate property names.
+ */
+ Shape *callObjectShape(JSContext *cx) const;
+
/* See Scope::extensibleParents */
inline bool extensibleParents();
bool setExtensibleParents(JSContext *cx);
@@ -262,6 +270,9 @@ class Bindings {
return add(cx, NULL, ARGUMENT);
}
+ void noteDup() { hasDup_ = true; }
+ bool hasDup() const { return hasDup_; }
+
/*
* Look up an argument or variable name, returning its kind when found or
* NONE when no such name exists. When indexp is not null and the name
View
20 js/src/jsscriptinlines.h
@@ -58,7 +58,7 @@ namespace js {
inline
Bindings::Bindings(JSContext *cx)
- : lastBinding(NULL), nargs(0), nvars(0), nupvars(0)
+ : lastBinding(NULL), nargs(0), nvars(0), nupvars(0), hasDup_(false)
{}
inline void
@@ -90,16 +90,22 @@ Bindings::lastShape() const
return lastBinding;
}
+Shape *
+Bindings::initialShape(JSContext *cx) const
+{
+ /* Get an allocation kind to match an empty call object. */
+ gc::AllocKind kind = gc::FINALIZE_OBJECT4;
+ JS_ASSERT(gc::GetGCKindSlots(kind) == CallObject::RESERVED_SLOTS + 1);
+
+ return EmptyShape::getInitialShape(cx, &CallClass, NULL, NULL, kind,
+ BaseShape::VAROBJ);
+}
+
bool
Bindings::ensureShape(JSContext *cx)
{
if (!lastBinding) {
- /* Get an allocation kind to match an empty call object. */
- gc::AllocKind kind = gc::FINALIZE_OBJECT4;
- JS_ASSERT(gc::GetGCKindSlots(kind) == CallObject::RESERVED_SLOTS + 1);
-
- lastBinding = EmptyShape::getInitialShape(cx, &CallClass, NULL, NULL, kind,
- BaseShape::VAROBJ);
+ lastBinding = initialShape(cx);
if (!lastBinding)
return false;
}
View
15 js/src/vm/ScopeObject.cpp
@@ -1,4 +1,4 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+/* -*- Mode: C++; tab-width: 6; indent-tabs-mode: nil; c-basic-offset: 4 -*-
* vim: set ts=8 sw=4 et tw=78:
*
* ***** BEGIN LICENSE BLOCK *****
@@ -65,8 +65,12 @@ using namespace js::types;
CallObject *
CallObject::create(JSContext *cx, JSScript *script, JSObject &enclosing, JSObject *callee)
{
- Bindings &bindings = script->bindings;
- gc::AllocKind kind = gc::GetGCObjectKind(bindings.lastShape()->numFixedSlots() + 1);
+ RootedVarShape shape(cx);
+ shape = script->bindings.callObjectShape(cx);
+ if (shape == NULL)
+ return NULL;
+
+ gc::AllocKind kind = gc::GetGCObjectKind(shape->numFixedSlots() + 1);
RootedVarTypeObject type(cx);
@@ -75,12 +79,9 @@ CallObject::create(JSContext *cx, JSScript *script, JSObject &enclosing, JSObjec
return NULL;
HeapValue *slots;
- if (!PreallocateObjectDynamicSlots(cx, bindings.lastShape(), &slots))
+ if (!PreallocateObjectDynamicSlots(cx, shape, &slots))
return NULL;
- RootedVarShape shape(cx);
- shape = bindings.lastShape();
-
JSObject *obj = JSObject::create(cx, kind, shape, type, slots);
if (!obj)
return NULL;
Please sign in to comment.
Something went wrong with that request. Please try again.