commit 722b8391118c701e1df4327b90ed37c1724f855e
author Clement Skau <cskau@google.com> Wed Jun 26 14:06:34 2019 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Wed Jun 26 14:06:34 2019 +0000
tree 75b07ce26d2b9358ec95c56eb0f084e2b4dced86
parent b17cc1f9437bf26b7813e3febe1c3fd9ca49e88f

[VM] Fixes bug in CodeDeserial..::PostLoad due to Code::function.

This was triggered by 85360 which introduced external code observers
called during dart init.
The bug is however susceptible to racing as tests often shutdown the VM
before the code is fully deserialised.

Code::function() is deceptive as it does in fact not guarantee that its
return value is a Function. This in turn caused observer code to try access
non-exists members on not-actually-Functions.

Tested:
  tools/test.py -n dartk-asan-linux-debug-x64 vm/cc/DartAPI_DartInitializeCallsCodeObserver

Change-Id: I12e367be06a3630d2d401a37b9097273c13f31e1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107289
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/vm/clustered_snapshot.cc

diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc
index 37dce77..aef1813 100644
--- a/runtime/vm/clustered_snapshot.cc
+++ b/runtime/vm/clustered_snapshot.cc
@@ -1581,11 +1581,9 @@
   void PostLoad(const Array& refs, Snapshot::Kind kind, Zone* zone) {
     if (!CodeObservers::AreActive()) return;
     Code& code = Code::Handle(zone);
-    Function& function = Function::Handle(zone);
     for (intptr_t id = start_index_; id < stop_index_; id++) {
       code ^= refs.At(id);
-      function = code.function();
-      Code::NotifyCodeObservers(function, code, code.is_optimized());
+      Code::NotifyCodeObservers(code, code.is_optimized());
     }
   }
 #endif  // !DART_PRECOMPILED_RUNTIME

runtime/vm/dart_api_impl_test.cc

diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc
index 7c3c611..09e49c0 100644
--- a/runtime/vm/dart_api_impl_test.cc
+++ b/runtime/vm/dart_api_impl_test.cc
@@ -79,6 +79,11 @@
 
   // Reinitialize and ensure we can execute Dart code.
   EXPECT(Dart_Initialize(&params) == NULL);
+
+  // Wait for 5 seconds to let the kernel service load the snapshot,
+  // which should trigger calls to the code observer.
+  sleep(5);
+
   EXPECT(was_called);
   EXPECT(Dart_Cleanup() == NULL);
 }

runtime/vm/object.cc

diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index c817150..f3d56c8 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -14905,6 +14905,20 @@
   return code.raw();
 }
 
+void Code::NotifyCodeObservers(const Code& code, bool optimized) {
+#if !defined(PRODUCT)
+  ASSERT(!Thread::Current()->IsAtSafepoint());
+  if (CodeObservers::AreActive()) {
+    const Object& owner = Object::Handle(code.owner());
+    if (owner.IsFunction()) {
+      NotifyCodeObservers(Function::Cast(owner), code, optimized);
+    } else {
+      NotifyCodeObservers(code.Name(), code, optimized);
+    }
+  }
+#endif
+}
+
 void Code::NotifyCodeObservers(const Function& function,
                                const Code& code,
                                bool optimized) {

runtime/vm/object.h

diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index 5091f1a..11a7c9b 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -5442,6 +5442,9 @@
     StorePointer(&raw_ptr()->exception_handlers_, handlers.raw());
   }
 
+  // WARNING: function() returns the owner which is not guaranteed to be
+  // a Function. It is up to the caller to guarantee it isn't a stub, class,
+  // or something else.
   // TODO(turnidge): Consider dropping this function and making
   // everybody use owner().  Currently this function is misused - even
   // while generating the snapshot.
@@ -5488,6 +5491,7 @@
                                CodeStatistics* stats);
 
   // Notifies all active [CodeObserver]s.
+  static void NotifyCodeObservers(const Code& code, bool optimized);
   static void NotifyCodeObservers(const Function& function,
                                   const Code& code,
                                   bool optimized);