Bug 657260 - Move CheckStringLength to JSString. r=Waldo
authorBobby Holley <bobbyholley@gmail.com>
Fri, 07 Oct 2011 19:34:28 -0400
changeset 79027 702c4c6fc60d29e00dbde070369de276e72a320c
parent 79026 e39362de752fd4ef5ff89ff2edfbd78d1d634ba6
child 79028 8f10a82cf2de0a3302f5ea232d5dd09c0c9db836
push id506
push userclegnitto@mozilla.com
push dateWed, 09 Nov 2011 02:03:18 +0000
treeherdermozilla-aurora@63587fc7bb93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs657260
milestone10.0a1
Bug 657260 - Move CheckStringLength to JSString. r=Waldo
js/src/jsatom.cpp
js/src/jsstr.cpp
js/src/jsstrinlines.h
js/src/vm/String-inl.h
js/src/vm/String.cpp
js/src/vm/String.h
--- a/js/src/jsatom.cpp
+++ b/js/src/jsatom.cpp
@@ -550,17 +550,17 @@ js_AtomizeString(JSContext *cx, JSString
     return Atomize(cx, &chars, length, ib);
 }
 
 JSAtom *
 js_Atomize(JSContext *cx, const char *bytes, size_t length, InternBehavior ib, FlationCoding fc)
 {
     CHECK_REQUEST(cx);
 
-    if (!CheckStringLength(cx, length))
+    if (!JSString::validateLength(cx, length))
         return NULL;
 
     /*
      * Avoiding the malloc in InflateString on shorter strings saves us
      * over 20,000 malloc calls on mozilla browser startup. This compares to
      * only 131 calls where the string is longer than a 31 char (net) buffer.
      * The vast majority of atomized strings are already in the hashtable. So
      * js_AtomizeString rarely has to copy the temp string we make.
@@ -592,17 +592,17 @@ js_Atomize(JSContext *cx, const char *by
     return atom;
 }
 
 JSAtom *
 js_AtomizeChars(JSContext *cx, const jschar *chars, size_t length, InternBehavior ib)
 {
     CHECK_REQUEST(cx);
 
-    if (!CheckStringLength(cx, length))
+    if (!JSString::validateLength(cx, length))
         return NULL;
 
     return AtomizeInline(cx, &chars, length, ib);
 }
 
 JSAtom *
 js_GetExistingStringAtom(JSContext *cx, const jschar *chars, size_t length)
 {
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3028,17 +3028,17 @@ js_InitStringClass(JSContext *cx, JSObje
         return NULL;
 
     return proto;
 }
 
 JSFixedString *
 js_NewString(JSContext *cx, jschar *chars, size_t length)
 {
-    if (!CheckStringLength(cx, length))
+    if (!JSString::validateLength(cx, length))
         return NULL;
 
     JSFixedString *s = JSFixedString::new_(cx, chars, length);
     Probes::createString(cx, s, length);
     return s;
 }
 
 static JS_ALWAYS_INLINE JSFixedString *
--- a/js/src/jsstrinlines.h
+++ b/js/src/jsstrinlines.h
@@ -40,40 +40,20 @@
 #ifndef jsstrinlines_h___
 #define jsstrinlines_h___
 
 #include "jsatom.h"
 #include "jsstr.h"
 
 #include "jscntxtinlines.h"
 #include "jsgcinlines.h"
+#include "vm/String-inl.h"
 
 namespace js {
 
-static inline bool
-CheckStringLength(JSContext *cx, size_t length)
-{
-    if (JS_UNLIKELY(length > JSString::MAX_LENGTH)) {
-        if (JS_ON_TRACE(cx)) {
-            /*
-             * If we can't leave the trace, signal OOM condition, otherwise
-             * exit from trace before throwing.
-             */
-            if (!CanLeaveTrace(cx))
-                return NULL;
-
-            LeaveTrace(cx);
-        }
-        js_ReportAllocationOverflow(cx);
-        return false;
-    }
-
-    return true;
-}
-
 /*
  * String builder that eagerly checks for over-allocation past the maximum
  * string length.
  *
  * Note: over-allocation is not checked for when using the infallible
  * |replaceRawBuffer|, so the implementation of |finishString| also must check
  * for over-allocation.
  */
@@ -237,17 +217,17 @@ StringBuffer::length() const
     JS_STATIC_ASSERT(jsint(JSString::MAX_LENGTH) == JSString::MAX_LENGTH);
     JS_ASSERT(cb.length() <= JSString::MAX_LENGTH);
     return jsint(cb.length());
 }
 
 inline bool
 StringBuffer::checkLength(size_t length)
 {
-    return CheckStringLength(context(), length);
+    return JSString::validateLength(context(), length);
 }
 
 extern bool
 ValueToStringBufferSlow(JSContext *cx, const Value &v, StringBuffer &sb);
 
 inline bool
 ValueToStringBuffer(JSContext *cx, const Value &v, StringBuffer &sb)
 {
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -38,18 +38,40 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef String_inl_h__
 #define String_inl_h__
 
 #include "String.h"
 
+#include "jscntxt.h"
 #include "jsgcinlines.h"
 
+JS_ALWAYS_INLINE bool
+JSString::validateLength(JSContext *cx, size_t length)
+{
+    if (JS_UNLIKELY(length > JSString::MAX_LENGTH)) {
+        if (JS_ON_TRACE(cx)) {
+            /*
+             * If we can't leave the trace, signal OOM condition, otherwise
+             * exit from trace before throwing.
+             */
+            if (!js::CanLeaveTrace(cx))
+                return NULL;
+
+            js::LeaveTrace(cx);
+        }
+        js_ReportAllocationOverflow(cx);
+        return false;
+    }
+
+    return true;
+}
+
 JS_ALWAYS_INLINE void
 JSRope::init(JSString *left, JSString *right, size_t length)
 {
     d.lengthAndFlags = buildLengthAndFlags(length, ROPE_BIT);
     d.u1.left = left;
     d.s.u2.right = right;
 }
 
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -263,16 +263,18 @@ js_ConcatStrings(JSContext *cx, JSString
     if (leftLen == 0)
         return right;
 
     size_t rightLen = right->length();
     if (rightLen == 0)
         return left;
 
     size_t wholeLength = leftLen + rightLen;
+    if (!JSString::validateLength(cx, wholeLength))
+        return NULL;
 
     if (JSShortString::lengthFits(wholeLength)) {
         JSShortString *str = js_NewGCShortString(cx);
         if (!str)
             return NULL;
         const jschar *leftChars = left->getChars(cx);
         if (!leftChars)
             return NULL;
@@ -282,26 +284,16 @@ js_ConcatStrings(JSContext *cx, JSString
 
         jschar *buf = str->init(wholeLength);
         PodCopy(buf, leftChars, leftLen);
         PodCopy(buf + leftLen, rightChars, rightLen);
         buf[wholeLength] = 0;
         return str;
     }
 
-    if (wholeLength > JSString::MAX_LENGTH) {
-        if (JS_ON_TRACE(cx)) {
-            if (!CanLeaveTrace(cx))
-                return NULL;
-            LeaveTrace(cx);
-        }
-        js_ReportAllocationOverflow(cx);
-        return NULL;
-    }
-
     return JSRope::new_(cx, left, right, wholeLength);
 }
 
 JSFixedString *
 JSDependentString::undepend(JSContext *cx)
 {
     JS_ASSERT(isDependent());
 
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -256,16 +256,23 @@ class JSString : public js::gc::Cell
 
     static const size_t EXTENSIBLE_FLAGS  = JS_BIT(2) | JS_BIT(3);
     static const size_t NON_STATIC_ATOM   = JS_BIT(3);
 
     size_t buildLengthAndFlags(size_t length, size_t flags) {
         return (length << LENGTH_SHIFT) | flags;
     }
 
+    /*
+     * Helper function to validate that a string of a given length is
+     * representable by a JSString. An allocation overflow is reported if false
+     * is returned.
+     */
+    static inline bool validateLength(JSContext *cx, size_t length);
+
     static void staticAsserts() {
         JS_STATIC_ASSERT(JS_BITS_PER_WORD >= 32);
         JS_STATIC_ASSERT(((JSString::MAX_LENGTH << JSString::LENGTH_SHIFT) >>
                            JSString::LENGTH_SHIFT) == JSString::MAX_LENGTH);
         JS_STATIC_ASSERT(sizeof(JSString) ==
                          offsetof(JSString, d.inlineStorage) +
                          NUM_INLINE_CHARS * sizeof(jschar));
     }