Fix Dartium by supporting external strings as well in Symbols::FromConcatAll

runtime/vm/symbols.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/symbols.h"
 
 #include "vm/handles.h"
 #include "vm/handles_impl.h"
 #include "vm/hash_table.h"
 #include "vm/isolate.h"
@@ skipping 376 matching lines @@
     return New(str2);
   } else if (str2.Length() == 0) {
     return New(str1);
   } else {
     return NewSymbol(ConcatString(str1, str2));
   }
 }
 
 
 // TODO(srdjan): If this becomes performance critical code, consider looking
-// up symbol from pieces instead of concatenating them first into a big string.
+// up symbol from hash of pieces instead of concatenating them first into
+// a string.
 RawString* Symbols::FromConcatAll(
     const GrowableHandlePtrArray<const String>& strs) {
   const intptr_t strs_length = strs.length();
   GrowableArray<intptr_t> lengths(strs_length);
 
   intptr_t len_sum = 0;
-  bool is_one_byte_string = true;
+  const intptr_t kOneByteChar = 1;
+  intptr_t char_size = kOneByteChar;
+
   for (intptr_t i = 0; i < strs_length; i++) {
     const String& str = strs[i];
     const intptr_t str_len = str.Length();
     if ((String::kMaxElements - len_sum) < str_len) {
       Exceptions::ThrowOOM();
       UNREACHABLE();
     }
     len_sum += str_len;
     lengths.Add(str_len);
-    if (!str.IsOneByteString()) {
-      is_one_byte_string = false;
-    }
+    char_size = Utils::Maximum(char_size, str.CharSize());
   }
+  const bool is_one_byte_string = char_size == kOneByteChar;
 
   Zone* zone = Thread::Current()->zone();
-
   if (is_one_byte_string) {
     uint8_t* buffer = zone->Alloc<uint8_t>(len_sum);
     const uint8_t* const orig_buffer = buffer;
     for (intptr_t i = 0; i < strs_length; i++) {
       NoSafepointScope no_safepoint;
       intptr_t str_len = lengths[i];
       const String& str = strs[i];

[siva, 2015/09/02 18:06:44]

ASSERT(str.IsOneByteString() || str.IsExternalOneByteString());

[srdjan, 2015/09/02 18:24:37]

Done.

-      memmove(buffer, OneByteString::CharAddr(str, 0), str_len);
+      const uint8_t* src_p = str.IsOneByteString() ?
+          OneByteString::CharAddr(str, 0) :
+          ExternalOneByteString::CharAddr(str, 0);
+      memmove(buffer, src_p, str_len);
       buffer += str_len;
     }
     ASSERT(len_sum == buffer - orig_buffer);
     return Symbols::FromLatin1(orig_buffer, len_sum);
   } else {
     uint16_t* buffer = zone->Alloc<uint16_t>(len_sum);
     const uint16_t* const orig_buffer = buffer;
     for (intptr_t i = 0; i < strs_length; i++) {
       NoSafepointScope no_safepoint;
       intptr_t str_len = lengths[i];
       const String& str = strs[i];
       if (str.IsTwoByteString()) {
         memmove(buffer, TwoByteString::CharAddr(str, 0), str_len * 2);
+      } else if (str.IsExternalTwoByteString()) {
+        memmove(buffer, ExternalTwoByteString::CharAddr(str, 0), str_len * 2);
       } else {
-        uint8_t* src_p = OneByteString::CharAddr(str, 0);
+        // One-byte to two-byte string copy.
+        ASSERT(str.IsOneByteString() || str.IsExternalOneByteString());
+        const uint8_t* src_p = str.IsOneByteString() ?
+            OneByteString::CharAddr(str, 0) :
+            ExternalOneByteString::CharAddr(str, 0);

[siva, 2015/09/02 18:06:44]

It might make sense to add a String::CharAddr(...) function as you use this
pattern in more than one place.

We already have a String::CharAt(...)

[srdjan, 2015/09/02 18:24:37]

String::CharAddr was removed because it caused confusion.

         for (int n = 0; n < str_len; n++) {
           buffer[n] = src_p[n];
         }
       }
       buffer += str_len;
     }
     ASSERT(len_sum == buffer - orig_buffer);
     return Symbols::FromUTF16(orig_buffer, len_sum);
   }
 }
@@ skipping 75 matching lines @@
 RawObject* Symbols::GetVMSymbol(intptr_t object_id) {
   ASSERT(IsVMSymbolId(object_id));
   intptr_t i = (object_id - kMaxPredefinedObjectIds);
   if ((i > kIllegal) && (i < Symbols::kMaxPredefinedId)) {
     return symbol_handles_[i]->raw();
   }
   return Object::null();
 }
 
 }  // namespace dart