Fix bug in optimizing string .length loads.

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/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
@@ skipping 1157 matching lines @@
       Type::ZoneHandle(Type::SmiType()));
   length_load->set_result_cid(kSmiCid);
   length_load->set_recognized_kind(MethodRecognizer::kObjectArrayLength);
 
   call->ReplaceWith(length_load, current_iterator());
   RemovePushArguments(call);
 }
 
 
 static LoadFieldInstr* BuildLoadStringLength(Value* str) {
-  const bool is_immutable = true;  // String length is immutable.
+  // Treat length loads as mutable (i.e. affected by side effects) to avoid
+  // hoisting them since we can't hoist the preceding class-check. This
+  // is because of externalization of strings that affects their class-id.
+  const bool is_immutable = false;
   LoadFieldInstr* load = new LoadFieldInstr(
       str,
       String::length_offset(),
       Type::ZoneHandle(Type::SmiType()),
       is_immutable);
   load->set_result_cid(kSmiCid);
+  load->set_recognized_kind(MethodRecognizer::kStringBaseLength);
   return load;
 }
 
 
 void FlowGraphOptimizer::InlineStringLengthGetter(InstanceCallInstr* call) {
   // Check receiver class.
   AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
 
   LoadFieldInstr* load = BuildLoadStringLength(call->ArgumentAt(0)->value());
-  load->set_recognized_kind(MethodRecognizer::kStringBaseLength);
   call->ReplaceWith(load, current_iterator());
   RemovePushArguments(call);
 }
 
 
 void FlowGraphOptimizer::InlineStringIsEmptyGetter(InstanceCallInstr* call) {
   // Check receiver class.
   AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
 
   LoadFieldInstr* load = BuildLoadStringLength(call->ArgumentAt(0)->value());
@@ skipping 113 matching lines @@
     const String& constant_string =
         String::Cast(string_def->value());
     ConstantInstr* index_def = index->definition()->AsConstant();
     if (index_def->value().IsSmi()) {
       intptr_t constant_index = Smi::Cast(index_def->value()).Value();
       skip_check = (constant_index < constant_string.Length());
     }
   }
   if (!skip_check) {
     // Insert bounds check.
-    const bool is_immutable = true;
-    LoadFieldInstr* length = new LoadFieldInstr(
-        str->Copy(),
-        CheckArrayBoundInstr::LengthOffsetFor(cid),
-        Type::ZoneHandle(Type::SmiType()),
-        is_immutable);
-    length->set_result_cid(kSmiCid);
-    length->set_recognized_kind(MethodRecognizer::kStringBaseLength);
+    LoadFieldInstr* length = BuildLoadStringLength(str->Copy());
     InsertBefore(call, length, NULL, Definition::kValue);
     InsertBefore(call,
                  new CheckArrayBoundInstr(new Value(length),
                                           index->Copy(),
                                           cid,
                                           call),
                  call->env(),
                  Definition::kEffect);
   }
   return new LoadIndexedInstr(str,
@@ skipping 2913 matching lines @@
 
   if (FLAG_trace_constant_propagation) {
     OS::Print("\n==== After constant propagation ====\n");
     FlowGraphPrinter printer(*graph_);
     printer.PrintBlocks();
   }
 }
 
 
 }  // namespace dart