Fix identifier handling in strings

Give identifiers a full 32bit range, so we don't accidentally
get into overflows. Make sure UINT_MAX can be used as an identifier.

Change-Id: I7d031f9eff0ea2edd7d3e182670fbd37aaf04040
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

src/v4/qv4identifier.h

@@ -70,9 +70,8 @@ public:
         if (str->subtype == String::StringType_ArrayIndex)
             return str;
 
-        str->stringIdentifier = currentIndex;
+        str->identifier = currentIndex;
         if (currentIndex <= USHRT_MAX) {
-            str->subtype = String::StringType_Identifier;
             identifiers.insert(s, str);
             ++currentIndex;
         }
@@ -80,21 +79,19 @@ public:
     }
 
     void toIdentifier(String *s) {
-        if (s->subtype >= String::StringType_Identifier)
+        if (s->identifier != UINT_MAX)
             return;
         if (s->subtype == String::StringType_Unknown)
             s->createHashValue();
-        if (s->subtype >= String::StringType_Identifier)
+        if (s->subtype == String::StringType_ArrayIndex)
             return;
         QHash<QString, String*>::const_iterator it = identifiers.find(s->toQString());
         if (it != identifiers.constEnd()) {
-            s->subtype = String::StringType_Identifier;
+            s->identifier = (*it)->identifier;
-            s->stringIdentifier = (*it)->stringIdentifier;
             return;
         }
-        s->stringIdentifier = currentIndex;
+        s->identifier = currentIndex;
         if (currentIndex <= USHRT_MAX) {
-            s->subtype = String::StringType_Identifier;
             identifiers.insert(s->toQString(), s);
             ++currentIndex;
         }

src/v4/qv4internalclass.cpp

@@ -60,7 +60,7 @@ InternalClass::InternalClass(const QQmlJS::VM::InternalClass &other)
 uint InternalClass::getOrAddMember(Object *object, String *string)
 {
     engine->identifierCache->toIdentifier(string);
-    uint id = string->stringIdentifier;
+    uint id = string->identifier;
 
     QHash<uint, uint>::const_iterator it = propertyTable.constFind(id);
     if (it != propertyTable.constEnd())
@@ -115,7 +115,7 @@ void InternalClass::removeMember(Object *object, uint id)
 uint InternalClass::find(String *string)
 {
     engine->identifierCache->toIdentifier(string);
-    uint id = string->stringIdentifier;
+    uint id = string->identifier;
 
     QHash<uint, uint>::const_iterator it = propertyTable.constFind(id);
     if (it != propertyTable.constEnd())

src/v4/qv4object.cpp

@@ -69,7 +69,7 @@ Object::Object(ExecutionEngine *engine)
     : prototype(0)
     , internalClass(engine->emptyClass)
     , memberDataAlloc(0), memberData(0)
-    , arrayOffset(0), arrayDataLen(0), arrayAlloc(0), sparseArray(0)
+    , arrayOffset(0), arrayDataLen(0), arrayAlloc(0), arrayData(0), sparseArray(0)
 {
     type = Type_Object;
 }
@@ -221,7 +221,7 @@ PropertyDescriptor *Object::insertMember(String *s)
 PropertyDescriptor *Object::__getOwnProperty__(ExecutionContext *ctx, String *name)
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __getOwnProperty__(ctx, idx);
 
     uint member = internalClass->find(name);
@@ -246,7 +246,7 @@ PropertyDescriptor *Object::__getOwnProperty__(ExecutionContext *ctx, uint index
 PropertyDescriptor *Object::__getPropertyDescriptor__(ExecutionContext *ctx, String *name)
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __getPropertyDescriptor__(ctx, idx);
 
 
@@ -282,7 +282,7 @@ PropertyDescriptor *Object::__getPropertyDescriptor__(ExecutionContext *ctx, uin
 Value Object::__get__(ExecutionContext *ctx, String *name, bool *hasProperty)
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __get__(ctx, idx, hasProperty);
 
     name->makeIdentifier(ctx);
@@ -346,7 +346,7 @@ Value Object::__get__(ExecutionContext *ctx, uint index, bool *hasProperty)
 void Object::__put__(ExecutionContext *ctx, String *name, Value value)
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __put__(ctx, idx, value);
 
     name->makeIdentifier(ctx);
@@ -486,7 +486,7 @@ void Object::__put__(ExecutionContext *ctx, uint index, Value value)
 bool Object::__hasProperty__(const ExecutionContext *ctx, String *name) const
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __hasProperty__(ctx, idx);
 
     name->makeIdentifier(ctx);
@@ -510,7 +510,7 @@ bool Object::__hasProperty__(const ExecutionContext *ctx, uint index) const
 bool Object::__delete__(ExecutionContext *ctx, String *name)
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __delete__(ctx, idx);
 
     name->makeIdentifier(ctx);
@@ -519,7 +519,7 @@ bool Object::__delete__(ExecutionContext *ctx, String *name)
     if (memberIdx != UINT_MAX) {
         PropertyDescriptor &pd = memberData[memberIdx];
         if (pd.isConfigurable()) {
-            internalClass->removeMember(this, name->stringIdentifier);
+            internalClass->removeMember(this, name->identifier);
             memmove(memberData + memberIdx, memberData + memberIdx + 1, (internalClass->size - memberIdx)*sizeof(PropertyDescriptor));
             return true;
         }
@@ -565,7 +565,7 @@ bool Object::__delete__(ExecutionContext *ctx, uint index)
 bool Object::__defineOwnProperty__(ExecutionContext *ctx, String *name, const PropertyDescriptor *desc)
 {
     uint idx = name->asArrayIndex();
-    if (idx != String::InvalidArrayIndex)
+    if (idx != UINT_MAX)
         return __defineOwnProperty__(ctx, idx, desc);
 
     name->makeIdentifier(ctx);

src/v4/qv4string.cpp

@@ -52,20 +52,20 @@ static uint toArrayIndex(const QChar *ch, const QChar *end, bool *ok)
     *ok = false;
     uint i = ch->unicode() - '0';
     if (i > 9)
-        return String::InvalidArrayIndex;
+        return UINT_MAX;
     ++ch;
     // reject "01", "001", ...
     if (i == 0 && ch != end)
-        return String::InvalidArrayIndex;
+        return UINT_MAX;
 
     while (ch < end) {
         uint x = ch->unicode() - '0';
         if (x > 9)
-            return String::InvalidArrayIndex;
+            return UINT_MAX;
         uint n = i*10 + x;
         if (n < i)
             // overflow
-            return String::InvalidArrayIndex;
+            return UINT_MAX;
         i = n;
         ++ch;
     }
@@ -79,9 +79,10 @@ uint String::toUInt(bool *ok) const
 
     if (subtype == StringType_Unknown)
         createHashValue();
-    if (subtype == StringType_ArrayIndex)
+    if (subtype >= StringType_UInt)
         return stringHash;
 
+    // ### this conversion shouldn't be required
     double d = __qmljs_string_to_number(this);
     uint l = (uint)d;
     if (d == l)
@@ -104,7 +105,7 @@ void String::createHashValue() const
     bool ok;
     stringHash = toArrayIndex(ch, end, &ok);
     if (ok) {
-        subtype = StringType_ArrayIndex;
+        subtype = (stringHash == UINT_MAX) ? StringType_UInt : StringType_ArrayIndex;
         return;
     }
 

src/v4/qv4string.h

@@ -55,12 +55,12 @@ struct String : public Managed {
     enum StringType {
         StringType_Unknown,
         StringType_Regular,
-        StringType_Identifier,
+        StringType_UInt,
         StringType_ArrayIndex
     };
 
     String(const QString &text)
-        : _text(text), stringHash(InvalidHashValue)
+        : _text(text), stringHash(UINT_MAX), identifier(UINT_MAX)
     { type = Type_String; subtype = StringType_Unknown; }
 
     inline bool isEqualTo(const String *other) const {
@@ -68,12 +68,11 @@ struct String : public Managed {
             return true;
         if (hashValue() != other->hashValue())
             return false;
-        if (subtype == other->subtype) {
+        if (identifier != UINT_MAX && identifier == other->identifier)
-            if (subtype == StringType_ArrayIndex)
+            return true;
-                return true;
+        if (subtype >= StringType_UInt && subtype == other->subtype)
-            if (subtype == StringType_Identifier)
+            return true;
-                return stringIdentifier == other->stringIdentifier;
+
-        }
         return toQString() == other->toQString();
     }
 
@@ -87,10 +86,6 @@ struct String : public Managed {
 
         return stringHash;
     }
-    enum {
-        InvalidArrayIndex = 0xffffffff,
-        InvalidHashValue  = 0xffffffff
-    };
     uint asArrayIndex() const {
         if (subtype == StringType_Unknown)
             createHashValue();
@@ -101,19 +96,18 @@ struct String : public Managed {
     uint toUInt(bool *ok) const;
 
     void makeIdentifier(const ExecutionContext *ctx) {
-        if (subtype == StringType_Identifier)
+        if (identifier != UINT_MAX)
             return;
         makeIdentifierImpl(ctx);
     }
 
     void makeIdentifierImpl(const ExecutionContext *ctx);
 
-private:
-    friend struct Identifiers;
     void createHashValue() const;
 
     QString _text;
     mutable uint stringHash;
+    mutable uint identifier;
 };
 
 } // namespace VM