[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

@@ -1581,11 +1581,9 @@ class CodeDeserializationCluster : public DeserializationCluster {
   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

@@ -79,6 +79,11 @@ UNIT_TEST_CASE(DartAPI_DartInitializeCallsCodeObserver) {
 
   // 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

@@ -14905,6 +14905,20 @@ RawCode* Code::FinalizeCode(FlowGraphCompiler* compiler,
   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

@@ -5442,6 +5442,9 @@ class Code : public Object {
     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 @@ class Code : public Object {
                                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);