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

runtime/vm/flow_graph_optimizer.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/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/dart_entry.h"
 #include "vm/flow_graph_builder.h"
@@ skipping 2631 matching lines @@
   intptr_t element_size = FlowGraphCompiler::ElementSizeFor(receiver_cid);
   ConstantInstr* bytes_per_element =
       flow_graph()->GetConstant(Smi::Handle(Smi::New(element_size)));
   BinarySmiOpInstr* len_in_bytes =
       new BinarySmiOpInstr(Token::kMUL,
                            new Value(length),
                            new Value(bytes_per_element),
                            call->deopt_id());
   InsertBefore(call, len_in_bytes, call->env(), Definition::kValue);
 
-    // Check byte_index < len_in_bytes.
+  // Check byte_index + num_bytes < len_in_bytes.

[Ivan Posva, 2013/09/20 17:20:30]

Where is the check that byte_index is positive?

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

We have been using the name 'element_size_in_bytes' for this consistently, could
we call this 'element_size_in_bytes' instead of 'num_bytes'

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

Changed to access_size.

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

Good catch. Changed into two checks using CheckArrayBoundInstr and added a test:

0 <= element_size - 1 < length

and

0 <= byte_index < length - (element_size - 1)

+  ConstantInstr* num_bytes =
+      flow_graph()->GetConstant(Smi::Handle(Smi::New(
+          FlowGraphCompiler::ElementSizeFor(view_cid) - 1)));
+  BinarySmiOpInstr* adjusted_index =
+      new BinarySmiOpInstr(Token::kADD,
+                           new Value(byte_index),
+                           new Value(num_bytes),
+                           call->deopt_id());
+  InsertBefore(call, adjusted_index, call->env(), Definition::kValue);
   InsertBefore(call,
                new CheckArrayBoundInstr(new Value(len_in_bytes),
-                                        new Value(byte_index),
+                                        new Value(adjusted_index),
                                         call->deopt_id()),
                call->env(),
                Definition::kEffect);
 
   // Insert load of elements for external typed arrays.
   if (RawObject::IsExternalTypedDataClassId(receiver_cid)) {
     LoadUntaggedInstr* elements =
         new LoadUntaggedInstr(new Value(*array),
                               ExternalTypedData::data_offset());
     InsertBefore(call, elements, NULL, Definition::kValue);
@@ skipping 5005 matching lines @@
   }
 
   // Insert materializations at environment uses.
   for (intptr_t i = 0; i < exits.length(); i++) {
     CreateMaterializationAt(exits[i], alloc, alloc->cls(), *fields);
   }
 }
 
 
 }  // namespace dart