Fix memory-issue String::fromCharCodes, where a GC could invalidate an address.

runtime/lib/string.cc

 // Copyright (c) 2011, 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/bootstrap_natives.h"
 
 #include "include/dart_api.h"
 #include "vm/exceptions.h"
 #include "vm/dart_api_impl.h"
 #include "vm/isolate.h"
@@ skipping 139 matching lines @@
 
 
 DEFINE_NATIVE_ENTRY(OneByteString_allocate, 1) {
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, length_obj, arguments->NativeArgAt(0));
   return OneByteString::New(length_obj.Value(), Heap::kNew);
 }
 
 
 DEFINE_NATIVE_ENTRY(OneByteString_allocateFromOneByteList, 1) {
   Instance& list = Instance::CheckedHandle(arguments->NativeArgAt(0));
-  uint8_t* data = NULL;
-  intptr_t length = 0;
   if (list.IsTypedData()) {
     const TypedData& array = TypedData::Cast(list);
-    length = array.LengthInBytes();
-    data = reinterpret_cast<uint8_t*>(array.DataAddr(0));
+    intptr_t length = array.LengthInBytes();
+    return OneByteString::New(array, 0, length, Heap::kNew);
   } else if (list.IsExternalTypedData()) {
     const ExternalTypedData& array = ExternalTypedData::Cast(list);
-    length = array.LengthInBytes();
-    data = reinterpret_cast<uint8_t*>(array.DataAddr(0));
+    intptr_t length = array.LengthInBytes();
+    return OneByteString::New(array, 0, length, Heap::kNew);
   } else if (RawObject::IsTypedDataViewClassId(list.GetClassId())) {
     const Instance& view = Instance::Cast(list);
-    length = Smi::Value(TypedDataView::Length(view));
+    intptr_t length = Smi::Value(TypedDataView::Length(view));
     const Instance& data_obj = Instance::Handle(TypedDataView::Data(view));
     intptr_t data_offset = Smi::Value(TypedDataView::OffsetInBytes(view));
     if (data_obj.IsTypedData()) {
       const TypedData& array = TypedData::Cast(data_obj);
-      data = reinterpret_cast<uint8_t*>(array.DataAddr(data_offset));
+      return OneByteString::New(array, data_offset, length, Heap::kNew);
     } else if (data_obj.IsExternalTypedData()) {
       const ExternalTypedData& array = ExternalTypedData::Cast(data_obj);
-      data = reinterpret_cast<uint8_t*>(array.DataAddr(data_offset));
+      return OneByteString::New(array, data_offset, length, Heap::kNew);
     } else {
       UNREACHABLE();
     }
   } else if (list.IsArray()) {
     const Array& array = Array::Cast(list);
     intptr_t length = array.Length();
     String& string = String::Handle(OneByteString::New(length, Heap::kNew));
     for (int i = 0; i < length; i++) {
       intptr_t value = Smi::Value(reinterpret_cast<RawSmi*>(array.At(i)));
       OneByteString::SetCharAt(string, i, value);
     }
     return string.raw();
   } else if (list.IsGrowableObjectArray()) {
     const GrowableObjectArray& array = GrowableObjectArray::Cast(list);
     intptr_t length = array.Length();
     String& string = String::Handle(OneByteString::New(length, Heap::kNew));
     for (int i = 0; i < length; i++) {
       intptr_t value = Smi::Value(reinterpret_cast<RawSmi*>(array.At(i)));
       OneByteString::SetCharAt(string, i, value);
     }
     return string.raw();
   } else {
     UNREACHABLE();
   }
-  return OneByteString::New(data, length, Heap::kNew);
+  return Object::null();

[Ivan Posva, 2014/03/24 18:30:43]

Please put an UNREACHABLE here and drop the others in this function. Returning a
null object from this runtime call is not a valid option.

[Anders Johnsen, 2014/03/24 18:48:46]

Done, partially. I need to return something after UNREACHABLE, to satisfy the
compiler. 'Object::null()' seams like the obvious choice.

 }
 
 
 DEFINE_NATIVE_ENTRY(OneByteString_setAt, 3) {
   GET_NON_NULL_NATIVE_ARGUMENT(String, receiver, arguments->NativeArgAt(0));
   ASSERT(receiver.IsOneByteString());
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, index_obj, arguments->NativeArgAt(1));
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, code_point_obj, arguments->NativeArgAt(2));
   ASSERT((0 <= code_point_obj.Value()) && (code_point_obj.Value() <= 0xFF));
   OneByteString::SetCharAt(receiver, index_obj.Value(), code_point_obj.Value());
@@ skipping 133 matching lines @@
       ? String::Handle(OneByteString::New(length_value, Heap::kNew))
       : String::Handle(TwoByteString::New(length_value, Heap::kNew));
   NoGCScope no_gc;
 
   uint16_t* data_position = reinterpret_cast<uint16_t*>(codeUnits.DataAddr(0));
   String::Copy(result, 0, data_position, length_value);
   return result.raw();
 }
 
 }  // namespace dart