[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 <alexmarkov@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index 05d3029..635c4be 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -4260,7 +4260,7 @@
}
bool InstanceCallInstr::MatchesCoreName(const String& name) {
- return function_name().raw() == Library::PrivateCoreLibName(name).raw();
+ return Library::IsPrivateCoreLibName(function_name(), name);
}
RawFunction* InstanceCallInstr::ResolveForReceiverClass(
diff --git a/runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc b/runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc
index 0a6df8f..61f37dfe 100644
--- a/runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc
+++ b/runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc
@@ -820,13 +820,12 @@
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;
}
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 7fc9316..c427228 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -11377,6 +11377,15 @@
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();
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index ab8e168..fbf47ae 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -4031,8 +4031,13 @@
#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);
