Ensure that hash for a symbol is not set to 0 when it is made external.

runtime/vm/object.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/object.h"
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "vm/assembler.h"
 #include "vm/cpu.h"
@@ skipping 17024 matching lines @@
                                 intptr_t length,
                                 void* peer,
                                 Dart_PeerFinalizer cback) const {
   String& result = String::Handle();
   void* external_data;
   Dart_WeakPersistentHandleFinalizer finalizer;
   {
     NoGCScope no_gc;
     ASSERT(array != NULL);
     intptr_t str_length = this->Length();
-    ASSERT(length >= (str_length * this->CharSize()));
+    RELEASE_ASSERT(length >= (str_length * this->CharSize()));
     intptr_t class_id = raw()->GetClassId();
     intptr_t used_size = 0;
     intptr_t original_size = 0;
     uword tags = raw_ptr()->tags_;
 
     ASSERT(!InVMHeap());
     if (class_id == kOneByteStringCid) {
       used_size = ExternalOneByteString::InstanceSize();
       original_size = OneByteString::InstanceSize(str_length);
       ASSERT(original_size >= used_size);
 
       // Copy the data into the external array.
       if (str_length > 0) {
         memmove(array, OneByteString::CharAddr(*this, 0), str_length);
       }
 
       // Update the class information of the object.
       const intptr_t class_id = kExternalOneByteStringCid;
       tags = RawObject::SizeTag::update(used_size, tags);
       tags = RawObject::ClassIdTag::update(class_id, tags);
       raw_ptr()->tags_ = tags;
       result = this->raw();
+      const uint8_t* ext_array = reinterpret_cast<const uint8_t*>(array);
       ExternalStringData<uint8_t>* ext_data = new ExternalStringData<uint8_t>(
-          reinterpret_cast<const uint8_t*>(array), peer, cback);
-      result.SetLength(str_length);
-      result.SetHash(0);
+          ext_array, peer, cback);
+      RELEASE_ASSERT(result.Length() == str_length);
+      RELEASE_ASSERT(!result.HasHash() ||

[Ivan Posva, 2014/08/14 20:42:12]

We are aware that this makes the externalization of strings more expensive,
correct?

[siva, 2014/08/14 21:04:57]

Will change it to a plain ASSERT.

+                     (result.Hash() == String::Hash(ext_array, str_length)));

[koda, 2014/08/14 20:43:30]

This is costly to always run in release mode, and we still have no guarantee it
will continue to be true (underlying bytes can change at any time).

[siva, 2014/08/14 21:04:57]

See above changing to a plain ASSERT

       ExternalOneByteString::SetExternalData(result, ext_data);
       external_data = ext_data;
       finalizer = ExternalOneByteString::Finalize;
     } else {
       ASSERT(class_id == kTwoByteStringCid);
       used_size = ExternalTwoByteString::InstanceSize();
       original_size = TwoByteString::InstanceSize(str_length);
       ASSERT(original_size >= used_size);
 
       // Copy the data into the external array.
       if (str_length > 0) {
         memmove(array,
                 TwoByteString::CharAddr(*this, 0),
                 (str_length * kTwoByteChar));
       }
 
       // Update the class information of the object.
       const intptr_t class_id = kExternalTwoByteStringCid;
       tags = RawObject::SizeTag::update(used_size, tags);
       tags = RawObject::ClassIdTag::update(class_id, tags);
       raw_ptr()->tags_ = tags;
       result = this->raw();
+      const uint16_t* ext_array = reinterpret_cast<const uint16_t*>(array);
       ExternalStringData<uint16_t>* ext_data = new ExternalStringData<uint16_t>(
-          reinterpret_cast<const uint16_t*>(array), peer, cback);
-      result.SetLength(str_length);
-      result.SetHash(0);
+          ext_array, peer, cback);
+      RELEASE_ASSERT(result.Length() == str_length);
+      RELEASE_ASSERT(!result.HasHash() ||
+                     (result.Hash() == String::Hash(ext_array, str_length)));
       ExternalTwoByteString::SetExternalData(result, ext_data);
       external_data = ext_data;
       finalizer = ExternalTwoByteString::Finalize;
     }
 
     // If there is any left over space fill it with either an Array object or
     // just a plain object (depending on the amount of left over space) so
     // that it can be traversed over successfully during garbage collection.
     Object::MakeUnusedSpaceTraversable(*this, original_size, used_size);
   }  // NoGCScope
@@ skipping 2308 matching lines @@
   return tag_label.ToCString();
 }
 
 
 void UserTag::PrintJSONImpl(JSONStream* stream, bool ref) const {
   Instance::PrintJSONImpl(stream, ref);
 }
 
 
 }  // namespace dart