Undo dex2dex compilation before invoking LoadHook

We need to undo any dex_to_dex compilation that might have taken place
before passing a dex file to any registered ClassFileLoadHooks to
ensure that no internal opcodes are present in any methods.

Test: ./test.py --host -j40
Bug: 36653594

Change-Id: Ia42c77312e685d69f6b3ea764fad01710b10ab45

15 files changed, 358 insertions, 42 deletions

diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 35e9d5db29..85100ae49d 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -23,6 +23,7 @@
 #include <string.h>
 #include <sys/file.h>
 #include <sys/stat.h>
+#include <zlib.h>
 
 #include <memory>
 #include <sstream>
@@ -67,6 +68,12 @@ const uint8_t DexFile::kDexMagicVersions[DexFile::kNumDexVersions][DexFile::kDex
   {'0', '3', '8', '\0'}
 };
 
+uint32_t DexFile::CalculateChecksum() const {
+  const uint32_t non_sum = OFFSETOF_MEMBER(DexFile::Header, signature_);
+  const uint8_t* non_sum_ptr = Begin() + non_sum;
+  return adler32(adler32(0L, Z_NULL, 0), non_sum_ptr, Size() - non_sum);
+}
+
 struct DexFile::AnnotationValue {
   JValue value_;
   uint8_t type_;
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 58b8e792ee..1b18d21cb1 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -1088,6 +1088,9 @@ class DexFile {
   static int64_t ReadSignedLong(const uint8_t* ptr, int zwidth);
   static uint64_t ReadUnsignedLong(const uint8_t* ptr, int zwidth, bool fill_on_right);
 
+  // Recalculates the checksum of the dex file. Does not use the current value in the header.
+  uint32_t CalculateChecksum() const;
+
   // Returns a human-readable form of the method at an index.
   std::string PrettyMethod(uint32_t method_idx, bool with_signature = true) const;
   // Returns a human-readable form of the field at an index.
@@ -1320,6 +1323,9 @@ class ClassDataItemIterator {
   uint32_t NumVirtualMethods() const {
     return header_.virtual_methods_size_;
   }
+  bool IsAtMethod() const {
+    return pos_ >= EndOfInstanceFieldsPos();
+  }
   bool HasNextStaticField() const {
     return pos_ < EndOfStaticFieldsPos();
   }
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index 0b3f16a3cb..11b3cd025a 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -17,7 +17,6 @@
 #include "dex_file_verifier.h"
 
 #include <inttypes.h>
-#include <zlib.h>
 
 #include <limits>
 #include <memory>
@@ -368,11 +367,8 @@ bool DexFileVerifier::CheckHeader() {
     return false;
   }
 
+  uint32_t adler_checksum = dex_file_->CalculateChecksum();
   // Compute and verify the checksum in the header.
-  uint32_t adler_checksum = adler32(0L, Z_NULL, 0);
-  const uint32_t non_sum = sizeof(header_->magic_) + sizeof(header_->checksum_);
-  const uint8_t* non_sum_ptr = reinterpret_cast<const uint8_t*>(header_) + non_sum;
-  adler_checksum = adler32(adler_checksum, non_sum_ptr, expected_size - non_sum);
   if (adler_checksum != header_->checksum_) {
     if (verify_checksum_) {
       ErrorStringPrintf("Bad checksum (%08x, expected %08x)", adler_checksum, header_->checksum_);
diff --git a/runtime/openjdkjvmti/Android.bp b/runtime/openjdkjvmti/Android.bp
index dd49ad0cfb..e38f265c5a 100644
--- a/runtime/openjdkjvmti/Android.bp
+++ b/runtime/openjdkjvmti/Android.bp
@@ -24,6 +24,7 @@ cc_defaults {
     defaults: ["art_defaults"],
     host_supported: true,
     srcs: ["events.cc",
+           "fixed_up_dex_file.cc",
            "object_tagging.cc",
            "OpenjdkJvmTi.cc",
            "ti_class.cc",
@@ -56,7 +57,10 @@ cc_defaults {
 art_cc_library {
     name: "libopenjdkjvmti",
     defaults: ["libopenjdkjvmti_defaults"],
-    shared_libs: ["libart"],
+    shared_libs: [
+        "libart",
+        "libart-compiler",
+    ],
 }
 
 art_cc_library {
@@ -65,5 +69,8 @@ art_cc_library {
         "art_debug_defaults",
         "libopenjdkjvmti_defaults",
     ],
-    shared_libs: ["libartd"],
+    shared_libs: [
+        "libartd",
+        "libartd-compiler",
+    ],
 }
diff --git a/runtime/openjdkjvmti/fixed_up_dex_file.cc b/runtime/openjdkjvmti/fixed_up_dex_file.cc
new file mode 100644
index 0000000000..3338358796
--- /dev/null
+++ b/runtime/openjdkjvmti/fixed_up_dex_file.cc
@@ -0,0 +1,145 @@
+/* Copyright (C) 2017 The Android Open Source Project
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This file implements interfaces from the file jvmti.h. This implementation
+ * is licensed under the same terms as the file jvmti.h.  The
+ * copyright and license information for the file jvmti.h follows.
+ *
+ * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "fixed_up_dex_file.h"
+#include "dex_file-inl.h"
+
+// Compiler includes.
+#include "dex/dex_to_dex_decompiler.h"
+
+// Runtime includes.
+#include "oat_file.h"
+#include "vdex_file.h"
+
+namespace openjdkjvmti {
+
+static void RecomputeDexChecksum(art::DexFile* dex_file)
+    REQUIRES_SHARED(art::Locks::mutator_lock_) {
+  reinterpret_cast<art::DexFile::Header*>(const_cast<uint8_t*>(dex_file->Begin()))->checksum_ =
+      dex_file->CalculateChecksum();
+}
+
+// TODO This is more complicated then it seems like it should be.
+// The fact we don't keep around the data of where in the flat binary log of dex-quickening changes
+// each dex file starts means we need to search for it. Since JVMTI is the exception though we are
+// not going to put in the effort to optimize for it.
+static void DoDexUnquicken(const art::DexFile& new_dex_file,
+                           const art::DexFile& original_dex_file)
+    REQUIRES_SHARED(art::Locks::mutator_lock_) {
+  const art::OatDexFile* oat_dex = original_dex_file.GetOatDexFile();
+  if (oat_dex == nullptr) {
+    return;
+  }
+  const art::OatFile* oat_file = oat_dex->GetOatFile();
+  if (oat_file == nullptr) {
+    return;
+  }
+  const art::VdexFile* vdex = oat_file->GetVdexFile();
+  if (vdex == nullptr || vdex->GetQuickeningInfo().size() == 0) {
+    return;
+  }
+  const art::ArrayRef<const uint8_t> quickening_info(vdex->GetQuickeningInfo());
+  const uint8_t* quickening_info_ptr = quickening_info.data();
+  for (const art::OatDexFile* cur_oat_dex : oat_file->GetOatDexFiles()) {
+    std::string error;
+    std::unique_ptr<const art::DexFile> cur_dex_file(cur_oat_dex->OpenDexFile(&error));
+    DCHECK(cur_dex_file.get() != nullptr);
+    // Is this the dex file we are looking for?
+    if (UNLIKELY(cur_dex_file->Begin() == original_dex_file.Begin())) {
+      // Simple sanity check.
+      CHECK_EQ(new_dex_file.NumClassDefs(), original_dex_file.NumClassDefs());
+      for (uint32_t i = 0; i < new_dex_file.NumClassDefs(); ++i) {
+        const art::DexFile::ClassDef& class_def = new_dex_file.GetClassDef(i);
+        const uint8_t* class_data = new_dex_file.GetClassData(class_def);
+        if (class_data == nullptr) {
+          continue;
+        }
+        for (art::ClassDataItemIterator it(new_dex_file, class_data); it.HasNext(); it.Next()) {
+          if (it.IsAtMethod() && it.GetMethodCodeItem() != nullptr) {
+            uint32_t quickening_size = *reinterpret_cast<const uint32_t*>(quickening_info_ptr);
+            quickening_info_ptr += sizeof(uint32_t);
+            art::optimizer::ArtDecompileDEX(
+                *it.GetMethodCodeItem(),
+                art::ArrayRef<const uint8_t>(quickening_info_ptr, quickening_size),
+                /*decompile_return_instruction*/true);
+            quickening_info_ptr += quickening_size;
+          }
+        }
+      }
+      // We don't need to bother looking through the rest of the dex-files.
+      break;
+    } else {
+      // Not the dex file we want. Skip over all the quickening info for all its classes.
+      for (uint32_t i = 0; i < cur_dex_file->NumClassDefs(); ++i) {
+        const art::DexFile::ClassDef& class_def = cur_dex_file->GetClassDef(i);
+        const uint8_t* class_data = cur_dex_file->GetClassData(class_def);
+        if (class_data == nullptr) {
+          continue;
+        }
+        for (art::ClassDataItemIterator it(*cur_dex_file, class_data); it.HasNext(); it.Next()) {
+          if (it.IsAtMethod() && it.GetMethodCodeItem() != nullptr) {
+            uint32_t quickening_size = *reinterpret_cast<const uint32_t*>(quickening_info_ptr);
+            quickening_info_ptr += sizeof(uint32_t);
+            quickening_info_ptr += quickening_size;
+          }
+        }
+      }
+    }
+  }
+}
+
+std::unique_ptr<FixedUpDexFile> FixedUpDexFile::Create(const art::DexFile& original) {
+  // Copy the data into mutable memory.
+  std::vector<unsigned char> data;
+  data.resize(original.Size());
+  memcpy(data.data(), original.Begin(), original.Size());
+  std::string error;
+  std::unique_ptr<const art::DexFile> new_dex_file(art::DexFile::Open(
+      data.data(),
+      data.size(),
+      /*location*/"Unquickening_dexfile.dex",
+      /*location_checksum*/0,
+      /*oat_dex_file*/nullptr,
+      /*verify*/false,
+      /*verify_checksum*/false,
+      &error));
+  if (new_dex_file.get() == nullptr) {
+    LOG(ERROR) << "Unable to open dex file from memory for unquickening! error: " << error;
+    return nullptr;
+  }
+
+  DoDexUnquicken(*new_dex_file, original);
+  RecomputeDexChecksum(const_cast<art::DexFile*>(new_dex_file.get()));
+  std::unique_ptr<FixedUpDexFile> ret(new FixedUpDexFile(std::move(new_dex_file), std::move(data)));
+  return ret;
+}
+
+}  // namespace openjdkjvmti
diff --git a/runtime/openjdkjvmti/fixed_up_dex_file.h b/runtime/openjdkjvmti/fixed_up_dex_file.h
new file mode 100644
index 0000000000..db12f489e9
--- /dev/null
+++ b/runtime/openjdkjvmti/fixed_up_dex_file.h
@@ -0,0 +1,82 @@
+/* Copyright (C) 2017 The Android Open Source Project
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This file implements interfaces from the file jvmti.h. This implementation
+ * is licensed under the same terms as the file jvmti.h.  The
+ * copyright and license information for the file jvmti.h follows.
+ *
+ * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#ifndef ART_RUNTIME_OPENJDKJVMTI_FIXED_UP_DEX_FILE_H_
+#define ART_RUNTIME_OPENJDKJVMTI_FIXED_UP_DEX_FILE_H_
+
+#include <memory>
+#include <vector>
+
+#include "jni.h"
+#include "jvmti.h"
+#include "base/mutex.h"
+#include "dex_file.h"
+
+namespace openjdkjvmti {
+
+// A holder for a DexFile that has been 'fixed up' to ensure it is fully compliant with the
+// published standard (no internal/quick opcodes, all fields are the defined values, etc). This is
+// used to ensure that agents get a consistent dex file regardless of what version of android they
+// are running on.
+class FixedUpDexFile {
+ public:
+  static std::unique_ptr<FixedUpDexFile> Create(const art::DexFile& original)
+      REQUIRES_SHARED(art::Locks::mutator_lock_);
+
+  const art::DexFile& GetDexFile() {
+    return *dex_file_;
+  }
+
+  const unsigned char* Begin() {
+    return data_.data();
+  }
+
+  size_t Size() {
+    return data_.size();
+  }
+
+ private:
+  explicit FixedUpDexFile(std::unique_ptr<const art::DexFile> fixed_up_dex_file,
+                          std::vector<unsigned char> data)
+      : dex_file_(std::move(fixed_up_dex_file)),
+        data_(std::move(data)) {}
+
+  // the fixed up DexFile
+  std::unique_ptr<const art::DexFile> dex_file_;
+  // The backing data for dex_file_.
+  const std::vector<unsigned char> data_;
+
+  DISALLOW_COPY_AND_ASSIGN(FixedUpDexFile);
+};
+
+}  // namespace openjdkjvmti
+
+#endif  // ART_RUNTIME_OPENJDKJVMTI_FIXED_UP_DEX_FILE_H_
diff --git a/runtime/openjdkjvmti/ti_class.cc b/runtime/openjdkjvmti/ti_class.cc
index 38fd1d4af3..907ab0252d 100644
--- a/runtime/openjdkjvmti/ti_class.cc
+++ b/runtime/openjdkjvmti/ti_class.cc
@@ -43,6 +43,7 @@
 #include "common_throws.h"
 #include "dex_file_annotations.h"
 #include "events-inl.h"
+#include "fixed_up_dex_file.h"
 #include "gc/heap.h"
 #include "gc_root.h"
 #include "handle.h"
@@ -161,6 +162,8 @@ struct ClassCallback : public art::ClassLoadCallback {
     art::JNIEnvExt* env = self->GetJniEnv();
     ScopedLocalRef<jobject> loader(
         env, class_loader.IsNull() ? nullptr : env->AddLocalReference<jobject>(class_loader.Get()));
+    std::unique_ptr<FixedUpDexFile> dex_file_copy(FixedUpDexFile::Create(initial_dex_file));
+
     // Go back to native.
     art::ScopedThreadSuspension sts(self, art::ThreadState::kNative);
     // Call all Non-retransformable agents.
@@ -174,14 +177,14 @@ struct ClassCallback : public art::ClassLoadCallback {
         loader.get(),
         name.c_str(),
         static_cast<jobject>(nullptr),  // Android doesn't seem to have protection domains
-        static_cast<jint>(initial_dex_file.Size()),
-        static_cast<const unsigned char*>(initial_dex_file.Begin()),
+        static_cast<jint>(dex_file_copy->Size()),
+        static_cast<const unsigned char*>(dex_file_copy->Begin()),
         static_cast<jint*>(&post_no_redefine_len),
         static_cast<unsigned char**>(&post_no_redefine_dex_data));
     if (post_no_redefine_dex_data == nullptr) {
       DCHECK_EQ(post_no_redefine_len, 0);
-      post_no_redefine_dex_data = const_cast<unsigned char*>(initial_dex_file.Begin());
-      post_no_redefine_len = initial_dex_file.Size();
+      post_no_redefine_dex_data = const_cast<unsigned char*>(dex_file_copy->Begin());
+      post_no_redefine_len = dex_file_copy->Size();
     } else {
       post_no_redefine_unique_ptr = std::unique_ptr<const unsigned char>(post_no_redefine_dex_data);
       DCHECK_GT(post_no_redefine_len, 0);
@@ -210,7 +213,7 @@ struct ClassCallback : public art::ClassLoadCallback {
       DCHECK_GT(final_len, 0);
     }
 
-    if (final_dex_data != initial_dex_file.Begin()) {
+    if (final_dex_data != dex_file_copy->Begin()) {
       LOG(WARNING) << "Changing class " << descriptor;
       art::ScopedObjectAccess soa(self);
       art::StackHandleScope<2> hs(self);
diff --git a/runtime/openjdkjvmti/ti_class_definition.cc b/runtime/openjdkjvmti/ti_class_definition.cc
index 2c2a79bc58..de8d8fe576 100644
--- a/runtime/openjdkjvmti/ti_class_definition.cc
+++ b/runtime/openjdkjvmti/ti_class_definition.cc
@@ -40,16 +40,18 @@
 
 namespace openjdkjvmti {
 
-bool ArtClassDefinition::IsModified(art::Thread* self) const {
-  if (modified) {
+bool ArtClassDefinition::IsModified() const {
+  // RedefineClasses calls always are 'modified' since they need to change the original_dex_file of
+  // the class.
+  if (redefined) {
     return true;
   }
   // Check if the dex file we want to set is the same as the current one.
-  art::StackHandleScope<1> hs(self);
-  art::Handle<art::mirror::Class> h_klass(hs.NewHandle(self->DecodeJObject(klass)->AsClass()));
-  const art::DexFile& cur_dex_file = h_klass->GetDexFile();
-  return static_cast<jint>(cur_dex_file.Size()) != dex_len ||
-      memcmp(cur_dex_file.Begin(), dex_data.get(), dex_len) != 0;
+  // Unfortunately we need to do this check even if no modifications have been done since it could
+  // be that agents were removed in the mean-time so we still have a different dex file. The dex
+  // checksum means this is likely to be fairly fast.
+  return static_cast<jint>(original_dex_file.size()) != dex_len ||
+      memcmp(&original_dex_file.At(0), dex_data.get(), dex_len) != 0;
 }
 
 }  // namespace openjdkjvmti
diff --git a/runtime/openjdkjvmti/ti_class_definition.h b/runtime/openjdkjvmti/ti_class_definition.h
index 9ca1c07cfd..7a2e922c4c 100644
--- a/runtime/openjdkjvmti/ti_class_definition.h
+++ b/runtime/openjdkjvmti/ti_class_definition.h
@@ -47,6 +47,7 @@ struct ArtClassDefinition {
   jobject protection_domain;
   jint dex_len;
   JvmtiUniquePtr<unsigned char> dex_data;
+  JvmtiUniquePtr<unsigned char> original_dex_file_memory;
   art::ArraySlice<const unsigned char> original_dex_file;
 
   ArtClassDefinition()
@@ -56,8 +57,9 @@ struct ArtClassDefinition {
         protection_domain(nullptr),
         dex_len(0),
         dex_data(nullptr),
+        original_dex_file_memory(nullptr),
         original_dex_file(),
-        modified(false) {}
+        redefined(false) {}
 
   ArtClassDefinition(ArtClassDefinition&& o) = default;
 
@@ -65,20 +67,27 @@ struct ArtClassDefinition {
     if (new_dex_data == nullptr) {
       return;
     } else if (new_dex_data != dex_data.get() || new_dex_len != dex_len) {
-      SetModified();
       dex_len = new_dex_len;
       dex_data = MakeJvmtiUniquePtr(env, new_dex_data);
     }
   }
 
-  void SetModified() {
-    modified = true;
+  void SetRedefined() {
+    redefined = true;
   }
 
-  bool IsModified(art::Thread* self) const REQUIRES_SHARED(art::Locks::mutator_lock_);
+  art::ArraySlice<const unsigned char> GetNewOriginalDexFile() const {
+    if (redefined) {
+      return original_dex_file;
+    } else {
+      return art::ArraySlice<const unsigned char>();
+    }
+  }
+
+  bool IsModified() const;
 
  private:
-  bool modified;
+  bool redefined;
 };
 
 }  // namespace openjdkjvmti
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index 7faddfb0f9..95a1b007ca 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -347,7 +347,7 @@ jvmtiError Redefiner::RedefineClasses(ArtJvmTiEnv* env,
     def.dex_len = definitions[i].class_byte_count;
     def.dex_data = MakeJvmtiUniquePtr(env, class_bytes_copy);
     // We are definitely modified.
-    def.SetModified();
+    def.SetRedefined();
     def.original_dex_file = art::ArraySlice<const unsigned char>(definitions[i].class_bytes,
                                                                  definitions[i].class_byte_count);
     res = Transformer::FillInTransformationData(env, definitions[i].klass, &def);
@@ -386,7 +386,7 @@ jvmtiError Redefiner::RedefineClassesDirect(ArtJvmTiEnv* env,
   Redefiner r(runtime, self, error_msg);
   for (const ArtClassDefinition& def : definitions) {
     // Only try to transform classes that have been modified.
-    if (def.IsModified(self)) {
+    if (def.IsModified()) {
       jvmtiError res = r.AddRedefinition(env, def);
       if (res != OK) {
         return res;
@@ -443,7 +443,7 @@ jvmtiError Redefiner::AddRedefinition(ArtJvmTiEnv* env, const ArtClassDefinition
                                    def.klass,
                                    dex_file.release(),
                                    signature_ptr,
-                                   def.original_dex_file));
+                                   def.GetNewOriginalDexFile()));
   return OK;
 }
 
diff --git a/runtime/openjdkjvmti/transform.cc b/runtime/openjdkjvmti/transform.cc
index 06aecbaee3..a1883b414c 100644
--- a/runtime/openjdkjvmti/transform.cc
+++ b/runtime/openjdkjvmti/transform.cc
@@ -39,6 +39,7 @@
 #include "dex_file.h"
 #include "dex_file_types.h"
 #include "events-inl.h"
+#include "fixed_up_dex_file.h"
 #include "gc_root-inl.h"
 #include "globals.h"
 #include "jni_env_ext-inl.h"
@@ -152,6 +153,7 @@ jvmtiError Transformer::GetDexDataForRetransformation(ArtJvmTiEnv* env,
                                                       /*out*/unsigned char** dex_data) {
   art::StackHandleScope<3> hs(art::Thread::Current());
   art::Handle<art::mirror::ClassExt> ext(hs.NewHandle(klass->GetExtData()));
+  const art::DexFile* dex_file = nullptr;
   if (!ext.IsNull()) {
     art::Handle<art::mirror::Object> orig_dex(hs.NewHandle(ext->GetOriginalDexFile()));
     if (!orig_dex.IsNull()) {
@@ -167,17 +169,20 @@ jvmtiError Transformer::GetDexDataForRetransformation(ArtJvmTiEnv* env,
             /*out*/dex_data);
       } else {
         DCHECK(orig_dex->IsDexCache());
-        const art::DexFile* dex_file = orig_dex->AsDexCache()->GetDexFile();
+        dex_file = orig_dex->AsDexCache()->GetDexFile();
         *dex_data_len = static_cast<jint>(dex_file->Size());
-        return CopyDataIntoJvmtiBuffer(env, dex_file->Begin(), dex_file->Size(), /*out*/dex_data);
       }
     }
   }
-  // TODO De-quicken the dex file before passing it to the agents.
-  LOG(WARNING) << "Dex file is not de-quickened yet! Quickened dex instructions might be present";
-  const art::DexFile& dex = klass->GetDexFile();
-  *dex_data_len = static_cast<jint>(dex.Size());
-  return CopyDataIntoJvmtiBuffer(env, dex.Begin(), *dex_data_len, /*out*/dex_data);
+  if (dex_file == nullptr) {
+    dex_file = &klass->GetDexFile();
+    *dex_data_len = static_cast<jint>(dex_file->Size());
+  }
+  std::unique_ptr<FixedUpDexFile> fixed_dex_file(FixedUpDexFile::Create(*dex_file));
+  return CopyDataIntoJvmtiBuffer(env,
+                                 fixed_dex_file->Begin(),
+                                 fixed_dex_file->Size(),
+                                 /*out*/dex_data);
 }
 
 // TODO Move this function somewhere more appropriate.
@@ -209,6 +214,31 @@ jvmtiError Transformer::FillInTransformationData(ArtJvmTiEnv* env,
     jvmtiError res = GetDexDataForRetransformation(env, hs_klass, &def->dex_len, &new_data);
     if (res == OK) {
       def->dex_data = MakeJvmtiUniquePtr(env, new_data);
+      // TODO This whole thing is a bit of a mess.
+      // We need to keep track of what the runtime should think an unmodified dex file is since
+      // we need to be able to tell if anything changes. This might be different then the currently
+      // loaded dex file since we need to un-quicken stuff.
+      if (hs_klass->GetExtData() == nullptr ||
+          hs_klass->GetExtData()->GetOriginalDexFile() == nullptr) {
+        // We have never redefined this yet. Keep track of what the (de-quickened) dex file looks
+        // like so we can tell if anything has changed.
+        // Really we would like to just always do the 'else' block but the fact that we de-quickened
+        // stuff screws us over.
+        unsigned char* original_data_memory = nullptr;
+        res = env->Allocate(def->dex_len, &original_data_memory);
+        if (res != OK) {
+          return res;
+        }
+        memcpy(original_data_memory, new_data, def->dex_len);
+        def->original_dex_file_memory = MakeJvmtiUniquePtr(env, original_data_memory);
+        def->original_dex_file = art::ArraySlice<const unsigned char>(original_data_memory,
+                                                                      def->dex_len);
+      } else {
+        // We know that we have been redefined at least once (there is an original_dex_file set in
+        // the class) so we can just use the current dex file directly.
+        def->original_dex_file = art::ArraySlice<const unsigned char>(
+            hs_klass->GetDexFile().Begin(), hs_klass->GetDexFile().Size());
+      }
     } else {
       return res;
     }
diff --git a/test/932-transform-saves/src/Transform.java b/test/932-transform-saves/src/Transform.java
index 8e8af355da..83f7aa4b4d 100644
--- a/test/932-transform-saves/src/Transform.java
+++ b/test/932-transform-saves/src/Transform.java
@@ -23,6 +23,6 @@ class Transform {
     // of the two different strings were the same).
     // We know the string ids will be different because lexicographically:
     // "Goodbye" < "LTransform;" < "hello".
-    System.out.println("hello");
+    System.out.println("foobar");
   }
 }
diff --git a/test/983-source-transform-verify/expected.txt b/test/983-source-transform-verify/expected.txt
index 85746f39f5..0a94212ad2 100644
--- a/test/983-source-transform-verify/expected.txt
+++ b/test/983-source-transform-verify/expected.txt
@@ -1 +1,2 @@
 Dex file hook for Transform
+Dex file hook for java/lang/Object
diff --git a/test/983-source-transform-verify/source_transform.cc b/test/983-source-transform-verify/source_transform.cc
index 21ba7a307a..e8b7e13dcf 100644
--- a/test/983-source-transform-verify/source_transform.cc
+++ b/test/983-source-transform-verify/source_transform.cc
@@ -25,7 +25,9 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "bytecode_utils.h"
 #include "dex_file.h"
+#include "dex_instruction.h"
 #include "jit/jit.h"
 #include "jni.h"
 #include "native_stack_dump.h"
@@ -41,6 +43,8 @@
 namespace art {
 namespace Test983SourceTransformVerify {
 
+constexpr bool kSkipInitialLoad = true;
+
 // The hook we are using.
 void JNICALL CheckDexFileHook(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED,
                               JNIEnv* jni_env ATTRIBUTE_UNUSED,
@@ -52,7 +56,7 @@ void JNICALL CheckDexFileHook(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED,
                               const unsigned char* class_data,
                               jint* new_class_data_len ATTRIBUTE_UNUSED,
                               unsigned char** new_class_data ATTRIBUTE_UNUSED) {
-  if (class_being_redefined == nullptr) {
+  if (kSkipInitialLoad && class_being_redefined == nullptr) {
     // Something got loaded concurrently. Just ignore it for now.
     return;
   }
@@ -64,13 +68,37 @@ void JNICALL CheckDexFileHook(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED,
   std::unique_ptr<const DexFile> dex(DexFile::Open(class_data,
                                                    class_data_len,
                                                    "fake_location.dex",
-                                                   0,
-                                                   nullptr,
-                                                   true,
-                                                   true,
+                                                   /*location_checksum*/ 0,
+                                                   /*oat_dex_file*/ nullptr,
+                                                   /*verify*/ true,
+                                                   /*verify_checksum*/ true,
                                                    &error));
   if (dex.get() == nullptr) {
     std::cout << "Failed to verify dex file for " << name << " because " << error << std::endl;
+    return;
+  }
+  for (uint32_t i = 0; i < dex->NumClassDefs(); i++) {
+    const DexFile::ClassDef& def = dex->GetClassDef(i);
+    const uint8_t* data_item = dex->GetClassData(def);
+    if (data_item == nullptr) {
+      continue;
+    }
+    for (ClassDataItemIterator it(*dex, data_item); it.HasNext(); it.Next()) {
+      if (!it.IsAtMethod() || it.GetMethodCodeItem() == nullptr) {
+        continue;
+      }
+      for (CodeItemIterator code_it(*it.GetMethodCodeItem()); !code_it.Done(); code_it.Advance()) {
+        const Instruction& inst = code_it.CurrentInstruction();
+        int forbiden_flags = (Instruction::kVerifyError | Instruction::kVerifyRuntimeOnly);
+        if (inst.Opcode() == Instruction::RETURN_VOID_NO_BARRIER ||
+            (inst.GetVerifyExtraFlags() & forbiden_flags) != 0) {
+          std::cout << "Unexpected instruction found in " << dex->PrettyMethod(it.GetMemberIndex())
+                    << " [Dex PC: 0x" << std::hex << code_it.CurrentDexPc() << std::dec << "] : "
+                    << inst.DumpString(dex.get()) << std::endl;
+          continue;
+        }
+      }
+    }
   }
 }
 
@@ -93,6 +121,5 @@ jint OnLoad(JavaVM* vm,
   return 0;
 }
 
-
 }  // namespace Test983SourceTransformVerify
 }  // namespace art
diff --git a/test/983-source-transform-verify/src/Main.java b/test/983-source-transform-verify/src/Main.java
index 35fb945209..5f42d29abe 100644
--- a/test/983-source-transform-verify/src/Main.java
+++ b/test/983-source-transform-verify/src/Main.java
@@ -25,6 +25,7 @@ public class Main {
     Transform abc = new Transform();
     enableCommonRetransformation(true);
     doCommonClassRetransformation(Transform.class);
+    doCommonClassRetransformation(Object.class);
     enableCommonRetransformation(false);
   }