Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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

… r=bhackett, a=akeybl
  • Loading branch information...
commit 83b79beccda21df15a3f65a0c4d142f4738424d1 1 parent 9c44977
authored January 27, 2012
1  js/src/frontend/Parser.cpp
@@ -1332,6 +1332,7 @@ Parser::functionArguments(TreeContext &funtc, FunctionBox *funbox, ParseNode **l
1332 1332
                  *     until after all arguments have been parsed.
1333 1333
                  */
1334 1334
                 if (funtc.decls.lookupFirst(name)) {
  1335
+                    funtc.bindings.noteDup();
1335 1336
                     duplicatedArg = name;
1336 1337
                     if (destructuringArg)
1337 1338
                         goto report_dup_and_destructuring;
5  js/src/jit-test/tests/basic/bug714614.js
... ...
@@ -0,0 +1,5 @@
  1
+function testForVarInWith(foo, foo) {
  2
+  return eval("with ({}) { for (var x = 0; x < 5; x++); } (function() { return delete x; })");
  3
+}
  4
+f = testForVarInWith()();
  5
+
1  js/src/jsscope.h
@@ -466,7 +466,6 @@ struct Shape : public js::gc::Cell
466 466
     friend class js::Bindings;
467 467
     friend struct js::StackShape;
468 468
     friend struct js::StackBaseShape;
469  
-    friend bool IsShapeAboutToBeFinalized(JSContext *cx, const js::Shape *shape);
470 469
 
471 470
   protected:
472 471
     HeapPtrBaseShape    base_;
41  js/src/jsscript.cpp
@@ -184,6 +184,47 @@ Bindings::add(JSContext *cx, JSAtom *name, BindingKind kind)
184 184
     return true;
185 185
 }
186 186
 
  187
+Shape *
  188
+Bindings::callObjectShape(JSContext *cx) const
  189
+{
  190
+    if (!hasDup())
  191
+        return lastShape();
  192
+
  193
+    /*
  194
+     * Build a vector of non-duplicate properties in order from last added
  195
+     * to first (i.e., the order we normally have iterate over Shapes). Choose
  196
+     * the last added property in each set of dups.
  197
+     */
  198
+    Vector<const Shape *> shapes(cx);
  199
+    HashSet<jsid> seen(cx);
  200
+    if (!seen.init())
  201
+        return false;
  202
+
  203
+    for (Shape::Range r = lastShape()->all(); !r.empty(); r.popFront()) {
  204
+        const Shape &s = r.front();
  205
+        HashSet<jsid>::AddPtr p = seen.lookupForAdd(s.propid());
  206
+        if (!p) {
  207
+            if (!seen.add(p, s.propid()))
  208
+                return NULL;
  209
+            if (!shapes.append(&s))
  210
+                return NULL;
  211
+        }
  212
+    }
  213
+
  214
+    /*
  215
+     * Now build the Shape without duplicate properties.
  216
+     */
  217
+    RootedVarShape shape(cx);
  218
+    shape = initialShape(cx);
  219
+    for (int i = shapes.length() - 1; i >= 0; --i) {
  220
+        shape = shape->getChildBinding(cx, shapes[i]);
  221
+        if (!shape)
  222
+            return NULL;
  223
+    }
  224
+
  225
+    return shape;
  226
+}
  227
+
187 228
 bool
188 229
 Bindings::getLocalNameArray(JSContext *cx, Vector<JSAtom *> *namesp)
189 230
 {
13  js/src/jsscript.h
@@ -175,7 +175,9 @@ class Bindings {
175 175
     uint16_t nargs;
176 176
     uint16_t nvars;
177 177
     uint16_t nupvars;
  178
+    bool     hasDup_:1;     // true if there are duplicate argument names
178 179
 
  180
+    inline Shape *initialShape(JSContext *cx) const;
179 181
   public:
180 182
     inline Bindings(JSContext *cx);
181 183
 
@@ -207,9 +209,15 @@ class Bindings {
207 209
     /* Ensure these bindings have a shape lineage. */
208 210
     inline bool ensureShape(JSContext *cx);
209 211
 
210  
-    /* Returns the shape lineage generated for these bindings. */
  212
+    /* Return the shape lineage generated for these bindings. */
211 213
     inline Shape *lastShape() const;
212 214
 
  215
+    /*
  216
+     * Return the shape to use to create a call object for these bindings.
  217
+     * The result is guaranteed not to have duplicate property names.
  218
+     */
  219
+    Shape *callObjectShape(JSContext *cx) const;
  220
+
213 221
     /* See Scope::extensibleParents */
214 222
     inline bool extensibleParents();
215 223
     bool setExtensibleParents(JSContext *cx);
@@ -262,6 +270,9 @@ class Bindings {
262 270
         return add(cx, NULL, ARGUMENT);
263 271
     }
264 272
 
  273
+    void noteDup() { hasDup_ = true; }
  274
+    bool hasDup() const { return hasDup_; }
  275
+
265 276
     /*
266 277
      * Look up an argument or variable name, returning its kind when found or
267 278
      * NONE when no such name exists. When indexp is not null and the name
20  js/src/jsscriptinlines.h
@@ -58,7 +58,7 @@ namespace js {
58 58
 
59 59
 inline
60 60
 Bindings::Bindings(JSContext *cx)
61  
-    : lastBinding(NULL), nargs(0), nvars(0), nupvars(0)
  61
+    : lastBinding(NULL), nargs(0), nvars(0), nupvars(0), hasDup_(false)
62 62
 {}
63 63
 
64 64
 inline void
@@ -90,16 +90,22 @@ Bindings::lastShape() const
90 90
     return lastBinding;
91 91
 }
92 92
 
  93
+Shape *
  94
+Bindings::initialShape(JSContext *cx) const
  95
+{
  96
+    /* Get an allocation kind to match an empty call object. */
  97
+    gc::AllocKind kind = gc::FINALIZE_OBJECT4;
  98
+    JS_ASSERT(gc::GetGCKindSlots(kind) == CallObject::RESERVED_SLOTS + 1);
  99
+
  100
+    return EmptyShape::getInitialShape(cx, &CallClass, NULL, NULL, kind,
  101
+                                       BaseShape::VAROBJ);
  102
+}
  103
+
93 104
 bool
94 105
 Bindings::ensureShape(JSContext *cx)
95 106
 {
96 107
     if (!lastBinding) {
97  
-        /* Get an allocation kind to match an empty call object. */
98  
-        gc::AllocKind kind = gc::FINALIZE_OBJECT4;
99  
-        JS_ASSERT(gc::GetGCKindSlots(kind) == CallObject::RESERVED_SLOTS + 1);
100  
-
101  
-        lastBinding = EmptyShape::getInitialShape(cx, &CallClass, NULL, NULL, kind,
102  
-                                                  BaseShape::VAROBJ);
  108
+        lastBinding = initialShape(cx);
103 109
         if (!lastBinding)
104 110
             return false;
105 111
     }
15  js/src/vm/ScopeObject.cpp
... ...
@@ -1,4 +1,4 @@
1  
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  1
+/* -*- Mode: C++; tab-width: 6; indent-tabs-mode: nil; c-basic-offset: 4 -*-
2 2
  * vim: set ts=8 sw=4 et tw=78:
3 3
  *
4 4
  * ***** BEGIN LICENSE BLOCK *****
@@ -65,8 +65,12 @@ using namespace js::types;
65 65
 CallObject *
66 66
 CallObject::create(JSContext *cx, JSScript *script, JSObject &enclosing, JSObject *callee)
67 67
 {
68  
-    Bindings &bindings = script->bindings;
69  
-    gc::AllocKind kind = gc::GetGCObjectKind(bindings.lastShape()->numFixedSlots() + 1);
  68
+    RootedVarShape shape(cx);
  69
+    shape = script->bindings.callObjectShape(cx);
  70
+    if (shape == NULL)
  71
+        return NULL;
  72
+
  73
+    gc::AllocKind kind = gc::GetGCObjectKind(shape->numFixedSlots() + 1);
70 74
 
71 75
     RootedVarTypeObject type(cx);
72 76
 
@@ -75,12 +79,9 @@ CallObject::create(JSContext *cx, JSScript *script, JSObject &enclosing, JSObjec
75 79
         return NULL;
76 80
 
77 81
     HeapValue *slots;
78  
-    if (!PreallocateObjectDynamicSlots(cx, bindings.lastShape(), &slots))
  82
+    if (!PreallocateObjectDynamicSlots(cx, shape, &slots))
79 83
         return NULL;
80 84
 
81  
-    RootedVarShape shape(cx);
82  
-    shape = bindings.lastShape();
83  
-
84 85
     JSObject *obj = JSObject::create(cx, kind, shape, type, slots);
85 86
     if (!obj)
86 87
         return NULL;

0 notes on commit 83b79be

Please sign in to comment.
Something went wrong with that request. Please try again.