[vm/compiler] Fix for flaky crash in CallSpecializer::ReplaceWithInstanceOf

There were rare crashes

../../runtime/vm/compiler/call_specializer.cc: 1393: error: expected: call->MatchesCoreName(Symbols::_simpleInstanceOf())
version=2.3.0-edge.796ebc6069bde3a59475a9b45075f49e50b0cc34 (Thu May 9 09:38:28 2019 -0700) on "linux_x64"
thread=82540, isolate=vm-service(0x558748c5d200)
  pc 0x000055874635dd6c fp 0x00007fa94dd3bf30 dart::Profiler::DumpStackTrace(void*)
  pc 0x0000558745f90332 fp 0x00007fa94dd3c010 dart::Assert::Fail(char const*, ...)
  pc 0x000055874653909b fp 0x00007fa94dd3c0a0 dart::CallSpecializer::ReplaceWithInstanceOf(dart::InstanceCallInstr*)
  pc 0x0000558746496ed6 fp 0x00007fa94dd3c0f0 dart::FlowGraphVisitor::VisitBlocks()

when running

tools/test.py --repeat 5000 -n dartkb-mixed-linux-debug-x64 language_2/null_test

at the rate ~1-3 crashes per 30,000 test cases.

The problem is that in function

bool InstanceCallInstr::MatchesCoreName(const String& name) {
  return function_name().raw() == Library::PrivateCoreLibName(name).raw();
}

'function_name().raw()' is evaluated before PrivateCoreLibName(name) is called
and saved in a temporary (register). PrivateCoreLibName may trigger GC and
relocate objects. In such case, 'PrivateCoreLibName(name).raw()' results in a
moved object, which is compared to a stale object address.

This CL fixes InstanceCallInstr::MatchesCoreName and other similar places
by introducing Library::IsPrivateCoreLibName, which is also a little bit more
efficient as it avoids extra symbol table lookup.

Change-Id: I4dc91c586b0c595a3e85d6da13b98fc2248fb8fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102120
Commit-Queue: Alexander Markov <[email protected]>
Reviewed-by: Aart Bik <[email protected]>

Author: alexmarkov

runtime/vm/compiler/backend/il.cc

@@ -4260,7 +4260,7 @@ void InstanceCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 }
 
 bool InstanceCallInstr::MatchesCoreName(const String& name) {
-  return function_name().raw() == Library::PrivateCoreLibName(name).raw();
+  return Library::IsPrivateCoreLibName(function_name(), name);
 }
 
 RawFunction* InstanceCallInstr::ResolveForReceiverClass(

runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc

@@ -820,13 +820,12 @@ void BytecodeFlowGraphBuilder::BuildInterfaceCallCommon(
     intptr_t argument_count = arg_desc.Count();
     ASSERT(argument_count <= 2);
     checked_argument_count = argument_count;
-  } else if (name.raw() ==
-             Library::PrivateCoreLibName(Symbols::_simpleInstanceOf()).raw()) {
+  } else if (Library::IsPrivateCoreLibName(name,
+                                           Symbols::_simpleInstanceOf())) {
     ASSERT(arg_desc.Count() == 2);
     checked_argument_count = 2;
     token_kind = Token::kIS;
-  } else if (name.raw() ==
-             Library::PrivateCoreLibName(Symbols::_instanceOf()).raw()) {
+  } else if (Library::IsPrivateCoreLibName(name, Symbols::_instanceOf())) {
     token_kind = Token::kIS;
   }
 

runtime/vm/object.cc

@@ -11377,6 +11377,15 @@ const String& Library::PrivateCoreLibName(const String& member) {
   return private_name;
 }
 
+bool Library::IsPrivateCoreLibName(const String& name, const String& member) {
+  Zone* zone = Thread::Current()->zone();
+  const auto& core_lib = Library::Handle(zone, Library::CoreLibrary());
+  const auto& private_key = String::Handle(zone, core_lib.private_key());
+
+  ASSERT(core_lib.IsPrivate(member));
+  return name.EqualsConcat(member, private_key);
+}
+
 RawClass* Library::LookupCoreClass(const String& class_name) {
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();

runtime/vm/object.h

@@ -4031,8 +4031,13 @@ class Library : public Object {
 #endif  // defined(DART_NO_SNAPSHOT).
 
   static bool IsPrivate(const String& name);
+
   // Construct the full name of a corelib member.
   static const String& PrivateCoreLibName(const String& member);
+
+  // Returns true if [name] matches full name of corelib [member].
+  static bool IsPrivateCoreLibName(const String& name, const String& member);
+
   // Lookup class in the core lib which also contains various VM
   // helper methods and classes. Allow look up of private classes.
   static RawClass* LookupCoreClass(const String& class_name);