Special case object lookup lambda in String.replace, bug 605317. r=jorendorff

js/src/jit-test/tests/basic/testReplaceMap.js

@@ -0,0 +1,27 @@
+
+// String.replace on functions returning hashmap elements.
+
+function first() {
+  var arr = {a: "hello", b: "there"};
+  var s = 'a|b';
+  return s.replace(/[a-z]/g, function(a) { return arr[a]; }, 'g');
+}
+assertEq(first(), "hello|there");
+
+function second() {
+  var arr = {a: "hello", c: "there"};
+  var s = 'a|b|c';
+  return s.replace(/[a-z]/g, function(a) { return arr[a]; }, 'g');
+}
+assertEq(second(), "hello|undefined|there");
+
+Object.defineProperty(Object.prototype, "b", {get: function() { return "what"; }});
+
+assertEq(second(), "hello|what|there");
+
+function third() {
+  var arr = {a: "hello", b: {toString: function() { arr = {}; return "three"; }}, c: "there"};
+  var s = 'a|b|c';
+  return s.replace(/[a-z]/g, function(a) { return arr[a]; }, 'g');
+}
+assertEq(third(), "hello|three|undefined");

js/src/jsatom.cpp

@@ -463,42 +463,13 @@ js_AtomizeString(JSContext *cx, JSString *str, uintN flags)
     if (str->isAtomized())
         return STRING_TO_ATOM(str);
 
-    size_t length = str->length();
+    const jschar *chars;
-    if (length == 1) {
+    size_t length;
-        jschar c = str->chars()[0];
+    str->getCharsAndLength(chars, length);
-        if (c < UNIT_STRING_LIMIT)
-            return STRING_TO_ATOM(JSString::unitString(c));
-    }
 
-    if (length == 2) {
+    JSString *staticStr = JSString::lookupStaticString(chars, length);
-        jschar *chars = str->chars();
+    if (staticStr)
-        if (JSString::fitsInSmallChar(chars[0]) &&
+        return STRING_TO_ATOM(staticStr);
-            JSString::fitsInSmallChar(chars[1])) {
-            return STRING_TO_ATOM(JSString::length2String(chars[0], chars[1]));
-        }
-    }
-
-    /*
-     * Here we know that JSString::intStringTable covers only 256 (or at least
-     * not 1000 or more) chars. We rely on order here to resolve the unit vs.
-     * int string/length-2 string atom identity issue by giving priority to unit
-     * strings for "0" through "9" and length-2 strings for "10" through "99".
-     */
-    JS_STATIC_ASSERT(INT_STRING_LIMIT <= 999);
-    if (length == 3) {
-        const jschar *chars = str->chars();
-
-        if ('1' <= chars[0] && chars[0] <= '9' &&
-            '0' <= chars[1] && chars[1] <= '9' &&
-            '0' <= chars[2] && chars[2] <= '9') {
-            jsint i = (chars[0] - '0') * 100 +
-                      (chars[1] - '0') * 10 +
-                      (chars[2] - '0');
-
-            if (jsuint(i) < INT_STRING_LIMIT)
-                return STRING_TO_ATOM(JSString::intString(i));
-        }
-    }
 
     JSAtomState *state = &cx->runtime->atomState;
     AtomSet &atoms = state->atoms;

js/src/jsobj.cpp

@@ -4459,14 +4459,6 @@ error: // TRACE_2 jumps here on error.
     return false;
 }
 
-JS_FRIEND_API(JSBool)
-js_LookupProperty(JSContext *cx, JSObject *obj, jsid id, JSObject **objp,
-                  JSProperty **propp)
-{
-    return js_LookupPropertyWithFlags(cx, obj, id, cx->resolveFlags,
-                                      objp, propp) >= 0;
-}
-
 #define SCOPE_DEPTH_ACCUM(bs,val)                                             \
     JS_SCOPE_DEPTH_METERING(JS_BASIC_STATS_ACCUM(bs, val))
 
@@ -4587,8 +4579,8 @@ static JS_ALWAYS_INLINE int
 js_LookupPropertyWithFlagsInline(JSContext *cx, JSObject *obj, jsid id, uintN flags,
                                  JSObject **objp, JSProperty **propp)
 {
-    /* Convert string indices to integers if appropriate. */
+    /* We should not get string indices which aren't already integers here. */
-    id = js_CheckForStringIndex(id);
+    JS_ASSERT(id == js_CheckForStringIndex(id));
 
     /* Search scopes starting with obj and following the prototype link. */
     JSObject *start = obj;
@@ -4636,10 +4628,23 @@ js_LookupPropertyWithFlagsInline(JSContext *cx, JSObject *obj, jsid id, uintN fl
     return protoIndex;
 }
 
+JS_FRIEND_API(JSBool)
+js_LookupProperty(JSContext *cx, JSObject *obj, jsid id, JSObject **objp,
+                  JSProperty **propp)
+{
+    /* Convert string indices to integers if appropriate. */
+    id = js_CheckForStringIndex(id);
+
+    return js_LookupPropertyWithFlagsInline(cx, obj, id, cx->resolveFlags, objp, propp) >= 0;
+}
+
 int
 js_LookupPropertyWithFlags(JSContext *cx, JSObject *obj, jsid id, uintN flags,
                            JSObject **objp, JSProperty **propp)
 {
+    /* Convert string indices to integers if appropriate. */
+    id = js_CheckForStringIndex(id);
+
     return js_LookupPropertyWithFlagsInline(cx, obj, id, flags, objp, propp);
 }
 

js/src/jsstr.cpp

@@ -81,6 +81,7 @@
 #include "jsobjinlines.h"
 #include "jsregexpinlines.h"
 #include "jsstrinlines.h"
+#include "jsautooplen.h"        // generated headers last
 
 using namespace js;
 using namespace js::gc;
@@ -1971,6 +1972,7 @@ struct ReplaceData
     JSString           *str;           /* 'this' parameter object as a string */
     RegExpGuard        g;              /* regexp parameter object and private data */
     JSObject           *lambda;        /* replacement function object or null */
+    JSObject           *elembase;      /* object for function(a){return b[a]} replace */
     JSString           *repstr;        /* replacement string */
     jschar             *dollar;        /* null or pointer to first $ in repstr */
     jschar             *dollarEnd;     /* limit pointer for js_strchr_limit */
@@ -2068,6 +2070,58 @@ class PreserveRegExpStatics
 static bool
 FindReplaceLength(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, size_t *sizep)
 {
+    JSObject *base = rdata.elembase;
+    if (base) {
+        /*
+         * The base object is used when replace was passed a lambda which looks like
+         * 'function(a) { return b[a]; }' for the base object b.  b will not change
+         * in the course of the replace unless we end up making a scripted call due
+         * to accessing a scripted getter or a value with a scripted toString.
+         */
+        JS_ASSERT(rdata.lambda);
+        JS_ASSERT(!base->getOps()->lookupProperty);
+        JS_ASSERT(!base->getOps()->getProperty);
+
+        Value match;
+        if (!res->createLastMatch(cx, &match))
+            return false;
+        JSString *str = match.toString();
+
+        JSAtom *atom;
+        if (str->isAtomized()) {
+            atom = STRING_TO_ATOM(str);
+        } else {
+            atom = js_AtomizeString(cx, str, 0);
+            if (!atom)
+                return false;
+        }
+        jsid id = ATOM_TO_JSID(atom);
+
+        JSObject *holder;
+        JSProperty *prop = NULL;
+        if (js_LookupPropertyWithFlags(cx, base, id, JSRESOLVE_QUALIFIED, &holder, &prop) < 0)
+            return false;
+
+        /* Only handle the case where the property exists and is on this object. */
+        if (prop && holder == base) {
+            Shape *shape = (Shape *) prop;
+            if (shape->slot != SHAPE_INVALID_SLOT && shape->hasDefaultGetter()) {
+                Value value = base->getSlot(shape->slot);
+                if (value.isString()) {
+                    rdata.repstr = value.toString();
+                    *sizep = rdata.repstr->length();
+                    return true;
+                }
+            }
+        }
+
+        /*
+         * Couldn't handle this property, fall through and despecialize to the
+         * general lambda case.
+         */
+        rdata.elembase = NULL;
+    }
+
     JSObject *lambda = rdata.lambda;
     if (lambda) {
         /*
@@ -2438,10 +2492,46 @@ js::str_replace(JSContext *cx, uintN argc, Value *vp)
     /* Extract replacement string/function. */
     if (argc >= optarg && js_IsCallable(vp[3])) {
         rdata.lambda = &vp[3].toObject();
+        rdata.elembase = NULL;
         rdata.repstr = NULL;
         rdata.dollar = rdata.dollarEnd = NULL;
+
+        if (rdata.lambda->isFunction()) {
+            JSFunction *fun = rdata.lambda->getFunctionPrivate();
+            if (fun->isInterpreted()) {
+                /*
+                 * Pattern match the script to check if it is is indexing into a
+                 * particular object, e.g. 'function(a) { return b[a]; }'.  Avoid
+                 * calling the script in such cases, which are used by javascript
+                 * packers (particularly the popular Dean Edwards packer) to efficiently
+                 * encode large scripts.  We only handle the code patterns generated
+                 * by such packers here.
+                 */
+                JSScript *script = fun->u.i.script;
+                jsbytecode *pc = script->code;
+
+                Value table = UndefinedValue();
+                if (JSOp(*pc) == JSOP_GETFCSLOT) {
+                    table = rdata.lambda->getFlatClosureUpvar(GET_UINT16(pc));
+                    pc += JSOP_GETFCSLOT_LENGTH;
+                }
+
+                if (table.isObject() &&
+                    JSOp(*pc) == JSOP_GETARG && GET_SLOTNO(pc) == 0 &&
+                    JSOp(*(pc + JSOP_GETARG_LENGTH)) == JSOP_GETELEM &&
+                    JSOp(*(pc + JSOP_GETARG_LENGTH + JSOP_GETELEM_LENGTH)) == JSOP_RETURN) {
+                    Class *clasp = table.toObject().getClass();
+                    if (clasp->isNative() &&
+                        !clasp->ops.lookupProperty &&
+                        !clasp->ops.getProperty) {
+                        rdata.elembase = &table.toObject();
+                    }
+                }
+            }
+        }
     } else {
         rdata.lambda = NULL;
+        rdata.elembase = NULL;
         rdata.repstr = ArgToRootedString(cx, argc, vp, 1);
         if (!rdata.repstr)
             return false;
@@ -3514,8 +3604,9 @@ js_NewDependentString(JSContext *cx, JSString *base, size_t start,
 
     jschar *chars = base->chars() + start;
 
-    if (length == 1 && *chars < UNIT_STRING_LIMIT)
+    JSString *staticStr = JSString::lookupStaticString(chars, length);
-        return const_cast<JSString *>(&JSString::unitStringTable[*chars]);
+    if (staticStr)
+        return staticStr;
 
     /* Try to avoid long chains of dependent strings. */
     while (base->isDependent())

js/src/jsstr.h

@@ -543,6 +543,8 @@ struct JSString {
     static JSString *getUnitString(JSContext *cx, JSString *str, size_t index);
     static JSString *length2String(jschar c1, jschar c2);
     static JSString *intString(jsint i);
+
+    static JSString *lookupStaticString(const jschar *chars, size_t length);
     
     JS_ALWAYS_INLINE void finalize(JSContext *cx, unsigned thingKind);
 };

js/src/jsstrinlines.h

@@ -76,6 +76,43 @@ JSString::intString(jsint i)
     return const_cast<JSString *>(JSString::intStringTable[u]);
 }
 
+/* Get a static atomized string for chars if possible. */
+inline JSString *
+JSString::lookupStaticString(const jschar *chars, size_t length)
+{
+    if (length == 1) {
+        if (chars[0] < UNIT_STRING_LIMIT)
+            return unitString(chars[0]);
+    }
+
+    if (length == 2) {
+        if (fitsInSmallChar(chars[0]) && fitsInSmallChar(chars[1]))
+            return length2String(chars[0], chars[1]);
+    }
+
+    /*
+     * Here we know that JSString::intStringTable covers only 256 (or at least
+     * not 1000 or more) chars. We rely on order here to resolve the unit vs.
+     * int string/length-2 string atom identity issue by giving priority to unit
+     * strings for "0" through "9" and length-2 strings for "10" through "99".
+     */
+    JS_STATIC_ASSERT(INT_STRING_LIMIT <= 999);
+    if (length == 3) {
+        if ('1' <= chars[0] && chars[0] <= '9' &&
+            '0' <= chars[1] && chars[1] <= '9' &&
+            '0' <= chars[2] && chars[2] <= '9') {
+            jsint i = (chars[0] - '0') * 100 +
+                      (chars[1] - '0') * 10 +
+                      (chars[2] - '0');
+
+            if (jsuint(i) < INT_STRING_LIMIT)
+                return intString(i);
+        }
+    }
+
+    return NULL;
+}
+
 inline void
 JSString::finalize(JSContext *cx, unsigned thingKind) {
     if (JS_LIKELY(thingKind == js::gc::FINALIZE_STRING)) {