Fix range checks for typed data in native code and the optimizer.

runtime/lib/typed_data.cc

 // Copyright (c) 2013, 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/bigint_operations.h"
 #include "vm/exceptions.h"
 #include "vm/native_entry.h"
 #include "vm/object.h"
 
 namespace dart {
 
 // TypedData.
 
-// Checks to see if offset_in_bytes is in the range.
-static bool RangeCheck(intptr_t offset_in_bytes, intptr_t length_in_bytes) {
-  return ((offset_in_bytes >= 0) &&
-          (length_in_bytes > 0) &&
-          (offset_in_bytes < length_in_bytes));
-}
-
-
 // Checks to see if offsetInBytes + num_bytes is in the range.
-static void SetRangeCheck(intptr_t offset_in_bytes,
+static void RangeCheck(intptr_t offset_in_bytes,
                           intptr_t num_bytes,
                           intptr_t length_in_bytes,
                           intptr_t element_size_in_bytes) {

[siva, 2013/09/20 22:03:45]

indentation

For a little more readability I am wondering if we should rename
'num_bytes' to 'access_length_in_bytes'

[Florian Schneider, 2013/09/23 11:02:26]

Done.

   if (!Utils::RangeCheck(offset_in_bytes, num_bytes, length_in_bytes)) {
     const String& error = String::Handle(String::NewFormatted(
         "index (%" Pd ") must be in the range [0..%" Pd ")",
-        (offset_in_bytes / element_size_in_bytes),
+        (offset_in_bytes / element_size_in_bytes) + num_bytes,

[siva, 2013/09/20 22:03:45]

I think this should be
((offset_in_bytes + num_bytes) / element_size_in_bytes)

[Florian Schneider, 2013/09/23 11:02:26]

Done.

         (length_in_bytes / element_size_in_bytes)));
     const Array& args = Array::Handle(Array::New(1));
     args.SetAt(0, error);
     Exceptions::ThrowByType(Exceptions::kRange, args);
   }
 }
 
 
 // Checks to see if a length will not result in an OOM error.
 static void LengthCheck(intptr_t len, intptr_t max) {
@@ skipping 38 matching lines @@
                          const Smi& length) {
   const DstType& dst_array = DstType::Cast(dst);
   const SrcType& src_array = SrcType::Cast(src);
   intptr_t element_size_in_bytes = dst_array.ElementSizeInBytes();
   intptr_t dst_offset_in_bytes = dst_start.Value() * element_size_in_bytes;
   intptr_t src_offset_in_bytes = src_start.Value() * element_size_in_bytes;
   intptr_t length_in_bytes = length.Value() * element_size_in_bytes;
   if (dst_array.ElementType() != src_array.ElementType()) {
     return Bool::False().raw();
   }
-  SetRangeCheck(src_offset_in_bytes,
+  RangeCheck(src_offset_in_bytes,
                 length_in_bytes,
                 src_array.LengthInBytes(),
                 element_size_in_bytes);

[siva, 2013/09/20 22:03:45]

indentation

[Florian Schneider, 2013/09/23 11:02:26]

Done.

-  SetRangeCheck(dst_offset_in_bytes,
+  RangeCheck(dst_offset_in_bytes,
                 length_in_bytes,
                 dst_array.LengthInBytes(),
                 element_size_in_bytes);

[siva, 2013/09/20 22:03:45]

indentation

[Florian Schneider, 2013/09/23 11:02:26]

Done.

   TypedData::Copy<DstType, SrcType>(dst_array, dst_offset_in_bytes,
                                     src_array, src_offset_in_bytes,
                                     length_in_bytes);
   return Bool::True().raw();
 }
 
 
 DEFINE_NATIVE_ENTRY(TypedData_setRange, 5) {
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, dst, arguments->NativeArgAt(0));
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, dst_start, arguments->NativeArgAt(1));
@@ skipping 65 matching lines @@
 }                                                                              \
 
 
 #define TYPED_DATA_NEW_NATIVE(name)                                            \
   TYPED_DATA_NEW(name)                                                         \
   EXT_TYPED_DATA_NEW(name)                                                     \
 
 
 CLASS_LIST_TYPED_DATA(TYPED_DATA_NEW_NATIVE)
 
-#define TYPED_DATA_GETTER(getter, object)                                      \
+#define TYPED_DATA_GETTER(getter, object, num_bytes)                           \
 DEFINE_NATIVE_ENTRY(TypedData_##getter, 2) {                                   \
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, instance, arguments->NativeArgAt(0)); \
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, offsetInBytes, arguments->NativeArgAt(1)); \
   if (instance.IsTypedData()) {                                                \
     const TypedData& array = TypedData::Cast(instance);                        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), num_bytes, array.LengthInBytes(), 1);    \
     return object::New(array.getter(offsetInBytes.Value()));                   \
   }                                                                            \
   if (instance.IsExternalTypedData()) {                                        \
     const ExternalTypedData& array = ExternalTypedData::Cast(instance);        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), num_bytes, array.LengthInBytes(), 1);    \
     return object::New(array.getter(offsetInBytes.Value()));                   \
   }                                                                            \
   const String& error = String::Handle(String::NewFormatted(                   \
       "Expected a TypedData object but found %s", instance.ToCString()));      \
   const Array& args = Array::Handle(Array::New(1));                            \
   args.SetAt(0, error);                                                        \
   Exceptions::ThrowByType(Exceptions::kArgument, args);                        \
   return object::null();                                                       \
 }                                                                              \
 
 
-#define TYPED_DATA_SETTER(setter, object, get_object_value)                    \
+#define TYPED_DATA_SETTER(setter, object, get_object_value, num_bytes)         \
 DEFINE_NATIVE_ENTRY(TypedData_##setter, 3) {                                   \
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, instance, arguments->NativeArgAt(0)); \
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, offsetInBytes, arguments->NativeArgAt(1)); \
   GET_NON_NULL_NATIVE_ARGUMENT(object, value, arguments->NativeArgAt(2));      \

[siva, 2013/09/20 22:03:45]

maybe more readable to use elem_size_in_bytes instead of num_bytes

[Florian Schneider, 2013/09/23 11:02:26]

Changed to access_size.

   if (instance.IsTypedData()) {                                                \
     const TypedData& array = TypedData::Cast(instance);                        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), num_bytes, array.LengthInBytes(), 1);    \

[siva, 2013/09/20 22:03:45]

shouldn't this be num_bytes instead of '1' here?

[Florian Schneider, 2013/09/23 11:02:26]

Done.

     array.setter(offsetInBytes.Value(), value.get_object_value());             \
   } else if (instance.IsExternalTypedData()) {                                 \
     const ExternalTypedData& array = ExternalTypedData::Cast(instance);        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), num_bytes, array.LengthInBytes(), 1);    \
     array.setter(offsetInBytes.Value(), value.get_object_value());             \
   } else {                                                                     \
     const String& error = String::Handle(String::NewFormatted(                 \
         "Expected a TypedData object but found %s", instance.ToCString()));    \
     const Array& args = Array::Handle(Array::New(1));                          \
     args.SetAt(0, error);                                                      \
     Exceptions::ThrowByType(Exceptions::kArgument, args);                      \
   }                                                                            \
   return Object::null();                                                       \
 }
 
 
 #define TYPED_DATA_UINT64_GETTER(getter, object)                               \
 DEFINE_NATIVE_ENTRY(TypedData_##getter, 2) {                                   \
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, instance, arguments->NativeArgAt(0)); \
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, offsetInBytes, arguments->NativeArgAt(1)); \
   uint64_t value = 0;                                                          \
   if (instance.IsTypedData()) {                                                \
     const TypedData& array = TypedData::Cast(instance);                        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), 8, array.LengthInBytes(), 1);            \
     value = array.getter(offsetInBytes.Value());                               \
   } else if (instance.IsExternalTypedData()) {                                 \
     const ExternalTypedData& array = ExternalTypedData::Cast(instance);        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), 8, array.LengthInBytes(), 1);            \
     value = array.getter(offsetInBytes.Value());                               \
   } else {                                                                     \
     const String& error = String::Handle(String::NewFormatted(                 \
         "Expected a TypedData object but found %s", instance.ToCString()));    \
     const Array& args = Array::Handle(Array::New(1));                          \
     args.SetAt(0, error);                                                      \
     Exceptions::ThrowByType(Exceptions::kArgument, args);                      \
   }                                                                            \
   return Integer::NewFromUint64(value);                                        \
 }                                                                              \
@@ skipping 10 matching lines @@
   if (value.IsBigint()) {                                                      \
     const Bigint& bigint = Bigint::Cast(value);                                \
     ASSERT(BigintOperations::FitsIntoUint64(bigint));                          \
     object_value = BigintOperations::AbsToUint64(bigint);                      \
   } else {                                                                     \
     ASSERT(value.IsMint() || value.IsSmi());                                   \
     object_value = value.AsInt64Value();                                       \
   }                                                                            \
   if (instance.IsTypedData()) {                                                \
     const TypedData& array = TypedData::Cast(instance);                        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), 8, array.LengthInBytes(), 1);            \
     array.setter(offsetInBytes.Value(), object_value);                         \
   } else if (instance.IsExternalTypedData()) {                                 \
     const ExternalTypedData& array = ExternalTypedData::Cast(instance);        \
-    ASSERT(RangeCheck(offsetInBytes.Value(), array.LengthInBytes()));          \
+    RangeCheck(offsetInBytes.Value(), 8, array.LengthInBytes(), 1);            \
     array.setter(offsetInBytes.Value(), object_value);                         \
   } else {                                                                     \
     const String& error = String::Handle(String::NewFormatted(                 \
         "Expected a TypedData object but found %s", instance.ToCString()));    \
     const Array& args = Array::Handle(Array::New(1));                          \
     args.SetAt(0, error);                                                      \
     Exceptions::ThrowByType(Exceptions::kArgument, args);                      \
   }                                                                            \
   return Object::null();                                                       \
 }
 
 
-#define TYPED_DATA_NATIVES(getter, setter, object, get_object_value)           \
-  TYPED_DATA_GETTER(getter, object)                                            \
-  TYPED_DATA_SETTER(setter, object, get_object_value)                          \
+#define TYPED_DATA_NATIVES(getter, setter, object, get_object_value, num_bytes)\
+  TYPED_DATA_GETTER(getter, object, num_bytes)                                 \
+  TYPED_DATA_SETTER(setter, object, get_object_value, num_bytes)               \
 
 
 #define TYPED_DATA_UINT64_NATIVES(getter, setter, object)                      \
   TYPED_DATA_UINT64_GETTER(getter, object)                                     \
   TYPED_DATA_UINT64_SETTER(setter, object)                                     \
 
 
-TYPED_DATA_NATIVES(GetInt8, SetInt8, Smi, Value)
-TYPED_DATA_NATIVES(GetUint8, SetUint8, Smi, Value)
-TYPED_DATA_NATIVES(GetInt16, SetInt16, Smi, Value)
-TYPED_DATA_NATIVES(GetUint16, SetUint16, Smi, Value)
-TYPED_DATA_NATIVES(GetInt32, SetInt32, Integer, AsInt64Value)
-TYPED_DATA_NATIVES(GetUint32, SetUint32, Integer, AsInt64Value)
-TYPED_DATA_NATIVES(GetInt64, SetInt64, Integer, AsInt64Value)
+TYPED_DATA_NATIVES(GetInt8, SetInt8, Smi, Value, 1)
+TYPED_DATA_NATIVES(GetUint8, SetUint8, Smi, Value, 1)
+TYPED_DATA_NATIVES(GetInt16, SetInt16, Smi, Value, 2)
+TYPED_DATA_NATIVES(GetUint16, SetUint16, Smi, Value, 2)
+TYPED_DATA_NATIVES(GetInt32, SetInt32, Integer, AsInt64Value, 4)
+TYPED_DATA_NATIVES(GetUint32, SetUint32, Integer, AsInt64Value, 4)
+TYPED_DATA_NATIVES(GetInt64, SetInt64, Integer, AsInt64Value, 8)
 TYPED_DATA_UINT64_NATIVES(GetUint64, SetUint64, Integer)
-TYPED_DATA_NATIVES(GetFloat32, SetFloat32, Double, value)
-TYPED_DATA_NATIVES(GetFloat64, SetFloat64, Double, value)
-TYPED_DATA_NATIVES(GetFloat32x4, SetFloat32x4, Float32x4, value)
+TYPED_DATA_NATIVES(GetFloat32, SetFloat32, Double, value, 4)
+TYPED_DATA_NATIVES(GetFloat64, SetFloat64, Double, value, 8)
+TYPED_DATA_NATIVES(GetFloat32x4, SetFloat32x4, Float32x4, value, 16)
 
 
 DEFINE_NATIVE_ENTRY(ByteData_ToEndianInt16, 2) {
   GET_NON_NULL_NATIVE_ARGUMENT(Smi, host_value, arguments->NativeArgAt(0));
   GET_NON_NULL_NATIVE_ARGUMENT(Bool, little_endian, arguments->NativeArgAt(1));
   int16_t value = host_value.Value();
   if (little_endian.value()) {
     value = Utils::HostToLittleEndian16(value);
   } else {
     value = Utils::HostToBigEndian16(value);
@@ skipping 98 matching lines @@
     value = bit_cast<double>(
         Utils::HostToLittleEndian64(bit_cast<uint64_t>(value)));
   } else {
     value = bit_cast<double>(
         Utils::HostToBigEndian64(bit_cast<uint64_t>(value)));
   }
   return Double::New(value);
 }
 
 }  // namespace dart