bug 599481 - static string tables should be constant. r=alangpierce
authorIgor Bukanov <igor@mir2.org>
Sat, 25 Sep 2010 00:36:58 +0200
changeset 54716 71fa2c4f4cf820a31f47c32d258130b8e927d07a
parent 54715 9a25714382f44f4b33e9a69a25c5ec876767eb03
child 54717 d0a2aec8dcb8798cdd50d09a0456c8fb77c41f09
push id16011
push userrsayre@mozilla.com
push dateWed, 29 Sep 2010 06:01:57 +0000
treeherdermozilla-central@d7e659b4f80c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersalangpierce
bugs599481
milestone2.0b7pre
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 599481 - static string tables should be constant. r=alangpierce
js/src/jsapi-tests/testIntString.cpp
js/src/jsstr.cpp
js/src/jsstr.h
js/src/jsstrinlines.h
--- a/js/src/jsapi-tests/testIntString.cpp
+++ b/js/src/jsapi-tests/testIntString.cpp
@@ -1,17 +1,40 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sw=4 et tw=99:
  */
 
 #include "tests.h"
+#include "jsstr.h"
 
 BEGIN_TEST(testIntString_bug515273)
 {
     jsvalRoot v(cx);
-    EVAL("'42';", v.addr());
 
+    EVAL("'1';", v.addr());
     JSString *str = JSVAL_TO_STRING(v.value());
-    const char *bytes = JS_GetStringBytes(str);
-    CHECK(strcmp(bytes, "42") == 0);
+    CHECK(JSString::isStatic(str));
+    CHECK(strcmp(JS_GetStringBytes(str), "1") == 0);
+
+    EVAL("'42';", v.addr());
+    str = JSVAL_TO_STRING(v.value());
+    CHECK(JSString::isStatic(str));
+    CHECK(strcmp(JS_GetStringBytes(str), "42") == 0);
+
+    EVAL("'111';", v.addr());
+    str = JSVAL_TO_STRING(v.value());
+    CHECK(JSString::isStatic(str));
+    CHECK(strcmp(JS_GetStringBytes(str), "111") == 0);
+
+    /* Test other types of static strings. */
+    EVAL("'a';", v.addr());
+    str = JSVAL_TO_STRING(v.value());
+    CHECK(JSString::isStatic(str));
+    CHECK(strcmp(JS_GetStringBytes(str), "a") == 0);
+
+    EVAL("'bc';", v.addr());
+    str = JSVAL_TO_STRING(v.value());
+    CHECK(JSString::isStatic(str));
+    CHECK(strcmp(JS_GetStringBytes(str), "bc") == 0);
+
     return true;
 }
 END_TEST(testIntString_bug515273)
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3121,17 +3121,17 @@ static JSFunctionSpec string_methods[] =
     { {(c), 0x00} } }
 
 #ifdef __SUNPRO_CC
 #pragma pack(8)
 #else
 #pragma pack(push, 8)
 #endif
 
-JSString JSString::unitStringTable[]
+const JSString JSString::unitStringTable[]
 #ifdef __GNUC__
 __attribute__ ((aligned (8)))
 #endif
 = { R8(0) };
 
 #ifdef __SUNPRO_CC
 #pragma pack(0)
 #else
@@ -3147,30 +3147,30 @@ JSString JSString::unitStringTable[]
  */
 #define TO_SMALL_CHAR(c) ((c) >= '0' && (c) <= '9' ? (c) - '0' :              \
                           (c) >= 'a' && (c) <= 'z' ? (c) - 'a' + 10 :         \
                           (c) >= 'A' && (c) <= 'Z' ? (c) - 'A' + 36 :         \
                           JSString::INVALID_SMALL_CHAR)
 
 #define R TO_SMALL_CHAR
 
-JSString::SmallChar JSString::toSmallChar[] = { R7(0) };
+const JSString::SmallChar JSString::toSmallChar[] = { R7(0) };
 
 #undef R
 
 /*
  * This is used when we generate our table of short strings, so the compiler is
  * happier if we use |c| as few times as possible.
  */
 #define FROM_SMALL_CHAR(c) ((c) + ((c) < 10 ? '0' :      \
                                    (c) < 36 ? 'a' - 10 : \
                                    'A' - 36))
 #define R FROM_SMALL_CHAR
 
-jschar JSString::fromSmallChar[] = { R6(0) };
+const jschar JSString::fromSmallChar[] = { R6(0) };
 
 #undef R
 
 /*
  * For code-generation ease, length-2 strings are encoded as 12-bit int values,
  * where the upper 6 bits is the first character and the lower 6 bits is the
  * second character.
  */
@@ -3181,17 +3181,17 @@ jschar JSString::fromSmallChar[] = { R6(
     { {FROM_SMALL_CHAR((c) >> 6), FROM_SMALL_CHAR((c) & 0x3F), 0x00} } }
 
 #ifdef __SUNPRO_CC
 #pragma pack(8)
 #else
 #pragma pack(push, 8)
 #endif
 
-JSString JSString::length2StringTable[]
+const JSString JSString::length2StringTable[]
 #ifdef __GNUC__
 __attribute__ ((aligned (8)))
 #endif
 = { R12(0) };
 
 #ifdef __SUNPRO_CC
 #pragma pack(0)
 #else
@@ -3223,17 +3223,17 @@ const char JSString::deflatedLength2Stri
 JS_STATIC_ASSERT(100 + (1 << 7) + (1 << 4) + (1 << 3) + (1 << 2) == 256);
 
 #ifdef __SUNPRO_CC
 #pragma pack(8)
 #else
 #pragma pack(push, 8)
 #endif
 
-JSString JSString::hundredStringTable[]
+const JSString JSString::hundredStringTable[]
 #ifdef __GNUC__
 __attribute__ ((aligned (8)))
 #endif
 = { R7(100), /* 100 through 227 */
     R4(100 + (1 << 7)), /* 228 through 243 */
     R3(100 + (1 << 7) + (1 << 4)), /* 244 through 251 */
     R2(100 + (1 << 7) + (1 << 4) + (1 << 3)) /* 252 through 255 */
 };
@@ -3241,17 +3241,17 @@ JSString JSString::hundredStringTable[]
 #undef R
 
 #define R(c) ((c) < 10 ? JSString::unitStringTable + ((c) + '0') :            \
               (c) < 100 ? JSString::length2StringTable +                      \
               ((size_t)TO_SMALL_CHAR(((c) / 10) + '0') << 6) +                \
               TO_SMALL_CHAR(((c) % 10) + '0') :                               \
               JSString::hundredStringTable + ((c) - 100))
 
-JSString *JSString::intStringTable[] = { R8(0) };
+const JSString *const JSString::intStringTable[] = { R8(0) };
 
 #undef R
 
 #ifdef __SUNPRO_CC
 #pragma pack(0)
 #else
 #pragma pack(pop)
 #endif
@@ -3486,17 +3486,17 @@ js_NewDependentString(JSContext *cx, JSS
         return cx->runtime->emptyString;
 
     if (start == 0 && length == base->length())
         return base;
 
     jschar *chars = base->chars() + start;
 
     if (length == 1 && *chars < UNIT_STRING_LIMIT)
-        return &JSString::unitStringTable[*chars];
+        return const_cast<JSString *>(&JSString::unitStringTable[*chars]);
 
     /* Try to avoid long chains of dependent strings. */
     while (base->isDependent())
         base = base->dependentBase();
 
     JS_ASSERT(base->isFlat());
 
     ds = js_NewGCString(cx);
--- a/js/src/jsstr.h
+++ b/js/src/jsstr.h
@@ -271,24 +271,26 @@ struct JSString {
     JS_ALWAYS_INLINE jschar *inlineStorage() {
         JS_ASSERT(isFlat());
         return mInlineStorage;
     }
 
     /* Specific flat string initializer and accessor methods. */
     JS_ALWAYS_INLINE void initFlat(jschar *chars, size_t length) {
         JS_ASSERT(length <= MAX_LENGTH);
+        JS_ASSERT(!isStatic(this));
         e.mBase = NULL;
         e.mCapacity = 0;
         mLengthAndFlags = (length << FLAGS_LENGTH_SHIFT) | FLAT;
         mChars = chars;
     }
 
     JS_ALWAYS_INLINE void initFlatMutable(jschar *chars, size_t length, size_t cap) {
         JS_ASSERT(length <= MAX_LENGTH);
+        JS_ASSERT(!isStatic(this));
         e.mBase = NULL;
         e.mCapacity = cap;
         mLengthAndFlags = (length << FLAGS_LENGTH_SHIFT) | FLAT | MUTABLE;
         mChars = chars;
     }
 
     JS_ALWAYS_INLINE jschar *flatChars() const {
         JS_ASSERT(isFlat());
@@ -329,36 +331,44 @@ struct JSString {
      * On the other hand, if the thread sees that the flag is unset, it could
      * be seeing a stale value when another thread has just atomized the string
      * and set the flag. But this can lead only to an extra call to
      * js_AtomizeString. This function would find that the string was already
      * hashed and return it with the atomized bit set.
      */
     inline void flatSetAtomized() {
         JS_ASSERT(isFlat());
+        JS_ASSERT(!isStatic(this));
         JS_ATOMIC_SET_MASK((jsword *)&mLengthAndFlags, ATOMIZED);
     }
 
     inline void flatSetMutable() {
         JS_ASSERT(isFlat());
         JS_ASSERT(!isAtomized());
         mLengthAndFlags |= MUTABLE;
     }
 
     inline void flatClearMutable() {
         JS_ASSERT(isFlat());
-        mLengthAndFlags &= ~MUTABLE;
+
+        /*
+         * We cannot eliminate the flag check before writing to mLengthAndFlags as
+         * static strings may reside in write-protected memory. See bug 599481.
+         */
+        if (mLengthAndFlags & MUTABLE)
+            mLengthAndFlags &= ~MUTABLE;
     }
 
     /*
      * The chars pointer should point somewhere inside the buffer owned by bstr.
      * The caller still needs to pass bstr for GC purposes.
      */
     inline void initDependent(JSString *bstr, jschar *chars, size_t len) {
         JS_ASSERT(len <= MAX_LENGTH);
+        JS_ASSERT(!isStatic(this));
         e.mParent = NULL;
         mChars = chars;
         mLengthAndFlags = DEPENDENT | (len << FLAGS_LENGTH_SHIFT);
         e.mBase = bstr;
     }
 
     inline JSString *dependentBase() const {
         JS_ASSERT(isDependent());
@@ -373,16 +383,17 @@ struct JSString {
         JS_ASSERT(isDependent());
         return length();
     }
 
     /* Rope-related initializers and accessors. */
     inline void initTopNode(JSString *left, JSString *right, size_t len,
                             JSRopeBufferInfo *buf) {
         JS_ASSERT(left->length() + right->length() <= MAX_LENGTH);
+        JS_ASSERT(!isStatic(this));
         mLengthAndFlags = TOP_NODE | (len << FLAGS_LENGTH_SHIFT);
         mLeft = left;
         e.mRight = right;
         e.mBufferWithInfo = buf;
     }
 
     inline void convertToInteriorNode(JSString *parent) {
         JS_ASSERT(isTopNode());
@@ -512,26 +523,26 @@ struct JSString {
     }
 
 #ifdef __SUNPRO_CC
 #pragma align 8 (__1cIJSStringPunitStringTable_, __1cIJSStringSlength2StringTable_, __1cIJSStringShundredStringTable_)
 #endif
 
     static const SmallChar INVALID_SMALL_CHAR = -1;
 
-    static jschar fromSmallChar[];
-    static SmallChar toSmallChar[];
-    static JSString unitStringTable[];
-    static JSString length2StringTable[];
-    static JSString hundredStringTable[];
+    static const jschar fromSmallChar[];
+    static const SmallChar toSmallChar[];
+    static const JSString unitStringTable[];
+    static const JSString length2StringTable[];
+    static const JSString hundredStringTable[];
     /*
      * Since int strings can be unit strings, length-2 strings, or hundred
      * strings, we keep a table to map from integer to the correct string.
      */
-    static JSString *intStringTable[];
+    static const JSString *const intStringTable[];
     static const char deflatedIntStringTable[];
     static const char deflatedUnitStringTable[];
     static const char deflatedLength2StringTable[];
 
     static JSString *unitString(jschar c);
     static JSString *getUnitString(JSContext *cx, JSString *str, size_t index);
     static JSString *length2String(jschar c1, jschar c2);
     static JSString *intString(jsint i);
--- a/js/src/jsstrinlines.h
+++ b/js/src/jsstrinlines.h
@@ -41,17 +41,17 @@
 #define jsstrinlines_h___
 
 #include "jsstr.h"
 
 inline JSString *
 JSString::unitString(jschar c)
 {
     JS_ASSERT(c < UNIT_STRING_LIMIT);
-    return &unitStringTable[c];
+    return const_cast<JSString *>(&unitStringTable[c]);
 }
 
 inline JSString *
 JSString::getUnitString(JSContext *cx, JSString *str, size_t index)
 {
     JS_ASSERT(index < str->length());
     jschar c = str->chars()[index];
     if (c < UNIT_STRING_LIMIT)
@@ -59,25 +59,26 @@ JSString::getUnitString(JSContext *cx, J
     return js_NewDependentString(cx, str, index, 1);
 }
 
 inline JSString *
 JSString::length2String(jschar c1, jschar c2)
 {
     JS_ASSERT(fitsInSmallChar(c1));
     JS_ASSERT(fitsInSmallChar(c2));
-    return &length2StringTable[(((size_t)toSmallChar[c1]) << 6) + toSmallChar[c2]];
+    return const_cast<JSString *>
+           (&length2StringTable[(((size_t)toSmallChar[c1]) << 6) + toSmallChar[c2]]);
 }
 
 inline JSString *
 JSString::intString(jsint i)
 {
     jsuint u = jsuint(i);
     JS_ASSERT(u < INT_STRING_LIMIT);
-    return JSString::intStringTable[u];
+    return const_cast<JSString *>(JSString::intStringTable[u]);
 }
 
 inline void
 JSString::finalize(JSContext *cx, unsigned thingKind) {
     if (JS_LIKELY(thingKind == js::gc::FINALIZE_STRING)) {
         JS_ASSERT(!JSString::isStatic(this));
         JS_RUNTIME_UNMETER(cx->runtime, liveStrings);
         if (isDependent()) {