[vm] Verify canonical objects before taking a snapshot or shutting down.

Don't verify recursive types.

Bug: https://github.com/dart-lang/sdk/issues/31376
Bug: https://github.com/dart-lang/sdk/issues/27003
Change-Id: I783bcc45d55c78141dccb1b479c1d15234a9b363
Reviewed-on: https://dart-review.googlesource.com/21221
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Régis Crelier <regis@google.com>

runtime/vm/clustered_snapshot.cc

@@ -5633,9 +5633,9 @@ FullSnapshotWriter::FullSnapshotWriter(Snapshot::Kind kind,
   ASSERT(object_store != NULL);
 
 #if defined(DEBUG)
-  // Ensure the class table is valid.
   isolate()->ValidateClassTable();
-#endif
+  isolate()->ValidateConstants();
+#endif  // DEBUG
   // Can't have any mutation happening while we're serializing.
   ASSERT(isolate()->background_compiler() == NULL);
 

runtime/vm/heap.h

@@ -12,7 +12,6 @@
 #include "vm/pages.h"
 #include "vm/scavenger.h"
 #include "vm/spaces.h"
-#include "vm/verifier.h"
 #include "vm/weak_table.h"
 
 namespace dart {

runtime/vm/isolate.cc

@@ -176,9 +176,11 @@ void Isolate::RegisterClassAt(intptr_t index, const Class& cls) {
   class_table()->RegisterAt(index, cls);
 }
 
+#if defined(DEBUG)
 void Isolate::ValidateClassTable() {
   class_table()->Validate();
 }
+#endif  // DEBUG
 
 void Isolate::RehashConstants() {
   StackZone stack_zone(Thread::Current());
@@ -201,6 +203,26 @@ void Isolate::RehashConstants() {
   }
 }
 
+#if defined(DEBUG)
+void Isolate::ValidateConstants() {
+  if (FLAG_precompiled_mode) {
+    // TODO(27003)
+    return;
+  }
+  if (HasAttemptedReload()) {
+    return;
+  }
+  // Verify that all canonical instances are correctly setup in the
+  // corresponding canonical tables.
+  StopBackgroundCompiler();
+  heap()->CollectAllGarbage();
+  Thread* thread = Thread::Current();
+  HeapIterationScope iteration(thread);
+  VerifyCanonicalVisitor check_canonical(thread);
+  iteration.IterateObjects(&check_canonical);
+}
+#endif  // DEBUG
+
 void Isolate::SendInternalLibMessage(LibMsgId msg_id, uint64_t capability) {
   const Array& msg = Array::Handle(Array::New(3));
   Object& element = Object::Handle();
@@ -1560,18 +1582,9 @@ static void ShutdownIsolate(uword parameter) {
     ASSERT(thread->isolate() == isolate);
     StackZone zone(thread);
     HandleScope handle_scope(thread);
-// TODO(27003): Enable for precompiled.
-#if defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME)
-    if (!isolate->HasAttemptedReload()) {
-      // For this verification we need to stop the background compiler earlier.
-      // This would otherwise happen in Dart::ShowdownIsolate.
-      isolate->StopBackgroundCompiler();
-      isolate->heap()->CollectAllGarbage();
-      HeapIterationScope iteration(thread);
-      VerifyCanonicalVisitor check_canonical(thread);
-      iteration.IterateObjects(&check_canonical);
-    }
-#endif  // defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME)
+#if defined(DEBUG)
+    isolate->ValidateConstants();
+#endif  // defined(DEBUG)
     const Error& error = Error::Handle(thread->sticky_error());
     if (!error.IsNull() && !error.IsUnwindError()) {
       OS::PrintErr("in ShutdownIsolate: %s\n", error.ToErrorCString());

runtime/vm/isolate.h

@@ -22,6 +22,7 @@
 #include "vm/thread.h"
 #include "vm/timer.h"
 #include "vm/token_position.h"
+#include "vm/verifier.h"
 
 namespace dart {
 
@@ -183,9 +184,14 @@ class Isolate : public BaseIsolate {
   // Register a newly introduced class.
   void RegisterClass(const Class& cls);
   void RegisterClassAt(intptr_t index, const Class& cls);
+#if defined(DEBUG)
   void ValidateClassTable();
+#endif
 
   void RehashConstants();
+#if defined(DEBUG)
+  void ValidateConstants();
+#endif
 
   // Visits weak object pointers.
   void VisitWeakPersistentHandles(HandleVisitor* visitor);

runtime/vm/isolate_reload.cc

@@ -1395,16 +1395,8 @@ void IsolateReloadContext::Commit() {
   }
 
 #ifdef DEBUG
-  {
-    // Verify that all canonical instances are correctly setup in the
-    // corresponding canonical tables.
-    Thread* thread = Thread::Current();
-    I->heap()->CollectAllGarbage();
-    HeapIterationScope iteration(thread);
-    VerifyCanonicalVisitor check_canonical(thread);
-    iteration.IterateObjects(&check_canonical);
-  }
-#endif  // DEBUG
+  I->ValidateConstants();
+#endif
 
   if (FLAG_identity_reload) {
     if (saved_num_cids_ != I->class_table()->NumCids()) {

runtime/vm/object.cc

@@ -17292,7 +17292,7 @@ RawAbstractType* Type::Canonicalize(TrailPtr trail) const {
 
 #if defined(DEBUG)
 bool Type::CheckIsCanonical(Thread* thread) const {
-  if (IsMalformed()) {
+  if (IsMalformed() || IsRecursive()) {
     return true;
   }
   if (type_class() == Object::dynamic_class()) {

runtime/vm/snapshot_test.cc

@@ -1313,6 +1313,7 @@ VM_UNIT_TEST_CASE(FullSnapshot) {
 
     // Write snapshot with object content.
     {
+      TransitionNativeToVM transition(thread);
       FullSnapshotWriter writer(
           Snapshot::kFull, NULL, &isolate_snapshot_data_buffer,
           &malloc_allocator, NULL, NULL /* image_writer */);
@@ -1369,6 +1370,7 @@ VM_UNIT_TEST_CASE(FullSnapshot1) {
 
     // Write snapshot with object content.
     {
+      TransitionNativeToVM transition(thread);
       FullSnapshotWriter writer(
           Snapshot::kFull, NULL, &isolate_snapshot_data_buffer,
           &malloc_allocator, NULL, NULL /* image_writer */);

tests/isolate/isolate.status

@@ -21,9 +21,6 @@ kill3_test: Pass, Fail    # Bad test: expects total message order
 
 message3_test/int32x4: Fail, Crash, Timeout # Issue 21818
 
-[ $runtime == vm && $compiler == app_jit && $mode == debug ]
-compile_time_error_test/none: Crash # Issue 31376
-
 [ $compiler == dart2js && $runtime == safarimobilesim ]
 compile_time_error_test/none: Pass, Slow
 

tests/language_2/language_2_vm.status

@@ -1065,9 +1065,6 @@ vm/causal_async_exception_stack_test: RuntimeError
 vm/causal_async_exception_stack2_test: RuntimeError
 vm/math_vm_test: Crash
 
-[ $runtime == vm && $compiler == app_jit && $mode == debug ]
-issue23244_test: Crash # Issue 31376
-
 [ $runtime == vm && $compiler == app_jit ]
 async_star_cancel_while_paused_test: RuntimeError
 async_star_pause_test: Fail, OK