Proactively dequicken on debuggable switch.

Previously we would generally not really consider dex2dex quickening
with debuggable processes. This could cause problems for structural
redefinition since the -quick opcodes are incompatible with the types
of changes structural redefinition allows. Furthermore this can cause
some unexpected behavior where (for example) check-casts might appear
to pass even if debugger activity should cause it to fail.

In order to fix these issues we make the runtime more proactively
dequicken dex-files when we start or switch to JAVA_DEBUGGABLE mode.

Test: ./test.py --target --host
Test: adb install -t ~/misc/Bandhook-Kotlin/app/build/outputs/apk/debug/app-debug.apk && adb shell monkey -p com.antonioleiva.bandhookkotlin -c android.intent.category.LAUNCHER 1
Bug: 134162467
Bug: 144168550
Change-Id: I2673c91b72ae7048d2ff71a1cf68cf552d4e8004

7 files changed, 121 insertions, 20 deletions

diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 978b1abbaf..2732de56f7 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -19,6 +19,7 @@
 
 #include <atomic>
 
+#include "android-base/thread_annotations.h"
 #include "art_field-inl.h"
 #include "art_method-inl.h"
 #include "base/mutex.h"
@@ -27,12 +28,14 @@
 #include "dex/dex_file_structs.h"
 #include "gc_root-inl.h"
 #include "handle_scope-inl.h"
+#include "jni/jni_internal.h"
 #include "mirror/class_loader.h"
 #include "mirror/dex_cache-inl.h"
 #include "mirror/iftable.h"
 #include "mirror/object_array-inl.h"
 #include "obj_ptr-inl.h"
 #include "scoped_thread_state_change-inl.h"
+#include "well_known_classes.h"
 
 namespace art {
 
@@ -449,6 +452,18 @@ inline ObjPtr<mirror::ObjectArray<mirror::Class>> ClassLinker::GetClassRoots() {
   return class_roots;
 }
 
+template <typename Visitor>
+void ClassLinker::VisitKnownDexFiles(Thread* self, Visitor visitor) {
+  ReaderMutexLock rmu(self, *Locks::dex_lock_);
+  std::for_each(dex_caches_.begin(),
+                dex_caches_.end(),
+                [&](DexCacheData& dcd) REQUIRES(Locks::mutator_lock_) {
+                  if (dcd.IsValid()) {
+                    visitor(dcd.dex_file);
+                  }
+                });
+}
+
 }  // namespace art
 
 #endif  // ART_RUNTIME_CLASS_LINKER_INL_H_
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 49e14300c7..4e38e6bce5 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -111,6 +111,18 @@ class ClassLoaderVisitor {
       REQUIRES_SHARED(Locks::classlinker_classes_lock_, Locks::mutator_lock_) = 0;
 };
 
+template <typename Func>
+class ClassLoaderFuncVisitor final : public ClassLoaderVisitor {
+ public:
+  explicit ClassLoaderFuncVisitor(Func func) : func_(func) {}
+  void Visit(ObjPtr<mirror::ClassLoader> cl) override REQUIRES_SHARED(Locks::mutator_lock_) {
+    func_(cl);
+  }
+
+ private:
+  Func func_;
+};
+
 class AllocatorVisitor {
  public:
   virtual ~AllocatorVisitor() {}
@@ -461,6 +473,9 @@ class ClassLinker {
   void VisitRoots(RootVisitor* visitor, VisitRootFlags flags)
       REQUIRES(!Locks::dex_lock_, !Locks::classlinker_classes_lock_, !Locks::trace_lock_)
       REQUIRES_SHARED(Locks::mutator_lock_);
+  // Visits all dex-files accessible by any class-loader or the BCP.
+  template<typename Visitor>
+  void VisitKnownDexFiles(Thread* self, Visitor visitor) REQUIRES(Locks::mutator_lock_);
 
   bool IsDexFileRegistered(Thread* self, const DexFile& dex_file)
       REQUIRES(!Locks::dex_lock_)
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 30ba6dd005..81356c2f2d 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -183,6 +183,10 @@ class OatFileBase : public OatFile {
   }
 
  private:
+  // Returns true if we want to remove quickened opcodes before loading the VDEX file, false
+  // otherwise.
+  bool ShouldUnquickenVDex() const;
+
   DISALLOW_COPY_AND_ASSIGN(OatFileBase);
 };
 
@@ -267,6 +271,13 @@ OatFileBase* OatFileBase::OpenOatFile(int zip_fd,
   return ret.release();
 }
 
+bool OatFileBase::ShouldUnquickenVDex() const {
+  // We sometimes load oat files without a runtime (eg oatdump) and don't want to do anything in
+  // that case. If we are debuggable there are no -quick opcodes to unquicken. If the runtime is not
+  // debuggable we don't care whether there are -quick opcodes or not so no need to do anything.
+  return Runtime::Current() != nullptr && !IsDebuggable() && Runtime::Current()->IsJavaDebuggable();
+}
+
 bool OatFileBase::LoadVdex(const std::string& vdex_filename,
                            bool writable,
                            bool low_4gb,
@@ -277,7 +288,7 @@ bool OatFileBase::LoadVdex(const std::string& vdex_filename,
                                   vdex_filename,
                                   writable,
                                   low_4gb,
-                                  /* unquicken=*/ false,
+                                  ShouldUnquickenVDex(),
                                   error_msg);
   if (vdex_.get() == nullptr) {
     *error_msg = StringPrintf("Failed to load vdex file '%s' %s",
@@ -299,16 +310,17 @@ bool OatFileBase::LoadVdex(int vdex_fd,
     if (rc == -1) {
       PLOG(WARNING) << "Failed getting length of vdex file";
     } else {
-      vdex_ = VdexFile::OpenAtAddress(vdex_begin_,
-                                      vdex_end_ - vdex_begin_,
-                                      /*mmap_reuse=*/ vdex_begin_ != nullptr,
-                                      vdex_fd,
-                                      s.st_size,
-                                      vdex_filename,
-                                      writable,
-                                      low_4gb,
-                                      /*unquicken=*/ false,
-                                      error_msg);
+      vdex_ = VdexFile::OpenAtAddress(
+          vdex_begin_,
+          vdex_end_ - vdex_begin_,
+          /*mmap_reuse=*/ vdex_begin_ != nullptr,
+          vdex_fd,
+          s.st_size,
+          vdex_filename,
+          writable,
+          low_4gb,
+          ShouldUnquickenVDex(),
+          error_msg);
       if (vdex_.get() == nullptr) {
         *error_msg = "Failed opening vdex file.";
         return false;
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 92222a2fd7..8861a095c7 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -35,6 +35,7 @@
 #include <cstdlib>
 #include <limits>
 #include <thread>
+#include <unordered_set>
 #include <vector>
 
 #include "android-base/strings.h"
@@ -104,7 +105,7 @@
 #include "mirror/class-alloc-inl.h"
 #include "mirror/class-inl.h"
 #include "mirror/class_ext.h"
-#include "mirror/class_loader.h"
+#include "mirror/class_loader-inl.h"
 #include "mirror/emulated_stack_frame.h"
 #include "mirror/field.h"
 #include "mirror/method.h"
@@ -2851,6 +2852,27 @@ void Runtime::DeoptimizeBootImage() {
       jit->GetCodeCache()->ClearEntryPointsInZygoteExecSpace();
     }
   }
+  // Also de-quicken all -quick opcodes. We do this for both BCP and non-bcp so if we are swapping
+  // debuggable during startup by a plugin (eg JVMTI) even non-BCP code has its vdex files deopted.
+  std::unordered_set<const VdexFile*> vdexs;
+  GetClassLinker()->VisitKnownDexFiles(Thread::Current(), [&](const art::DexFile* df) {
+    const OatDexFile* odf = df->GetOatDexFile();
+    if (odf == nullptr) {
+      return;
+    }
+    const OatFile* of = odf->GetOatFile();
+    if (of == nullptr || of->IsDebuggable()) {
+      // no Oat or already debuggable so no -quick.
+      return;
+    }
+    vdexs.insert(of->GetVdexFile());
+  });
+  LOG(INFO) << "Unquickening " << vdexs.size() << " vdex files!";
+  for (const VdexFile* vf : vdexs) {
+    vf->AllowWriting(true);
+    vf->UnquickenInPlace(/*decompile_return_instruction=*/true);
+    vf->AllowWriting(false);
+  }
 }
 
 Runtime::ScopedThreadPoolUsage::ScopedThreadPoolUsage()
diff --git a/runtime/vdex_file.cc b/runtime/vdex_file.cc
index e01d21ebe1..d67a968ae8 100644
--- a/runtime/vdex_file.cc
+++ b/runtime/vdex_file.cc
@@ -155,11 +155,13 @@ std::unique_ptr<VdexFile> VdexFile::OpenAtAddress(uint8_t* mmap_addr,
     mmap_reuse = false;
   }
   CHECK(!mmap_reuse || mmap_addr != nullptr);
+  CHECK(!(writable && unquicken)) << "We don't want to be writing unquickened files out to disk!";
+  // Start as PROT_WRITE so we can mprotect back to it if we want to.
   MemMap mmap = MemMap::MapFileAtAddress(
       mmap_addr,
       vdex_length,
-      (writable || unquicken) ? PROT_READ | PROT_WRITE : PROT_READ,
-      unquicken ? MAP_PRIVATE : MAP_SHARED,
+      PROT_READ | PROT_WRITE,
+      writable ? MAP_SHARED : MAP_PRIVATE,
       file_fd,
       /* start= */ 0u,
       low_4gb,
@@ -183,13 +185,19 @@ std::unique_ptr<VdexFile> VdexFile::OpenAtAddress(uint8_t* mmap_addr,
     if (!vdex->OpenAllDexFiles(&unique_ptr_dex_files, error_msg)) {
       return nullptr;
     }
+    // TODO: It would be nice to avoid doing the return-instruction stuff but then we end up not
+    // being able to tell if we need dequickening later. Instead just get rid of that too.
     vdex->Unquicken(MakeNonOwningPointerVector(unique_ptr_dex_files),
-                    /* decompile_return_instruction= */ false);
+                    /* decompile_return_instruction= */ true);
     // Update the quickening info size to pretend there isn't any.
     size_t offset = vdex->GetDexSectionHeaderOffset();
     reinterpret_cast<DexSectionHeader*>(vdex->mmap_.Begin() + offset)->quickening_info_size_ = 0;
   }
 
+  if (!writable) {
+    vdex->AllowWriting(false);
+  }
+
   return vdex;
 }
 
@@ -209,8 +217,12 @@ const uint8_t* VdexFile::GetNextDexFileData(const uint8_t* cursor) const {
   }
 }
 
+void VdexFile::AllowWriting(bool val) const {
+  CHECK(mmap_.Protect(val ? (PROT_READ | PROT_WRITE) : PROT_READ));
+}
+
 bool VdexFile::OpenAllDexFiles(std::vector<std::unique_ptr<const DexFile>>* dex_files,
-                               std::string* error_msg) {
+                               std::string* error_msg) const {
   const ArtDexFileLoader dex_file_loader;
   size_t i = 0;
   for (const uint8_t* dex_file_start = GetNextDexFileData(nullptr);
@@ -239,6 +251,23 @@ bool VdexFile::OpenAllDexFiles(std::vector<std::unique_ptr<const DexFile>>* dex_
   return true;
 }
 
+void VdexFile::UnquickenInPlace(bool decompile_return_instruction) const {
+  CHECK_NE(mmap_.GetProtect() & PROT_WRITE, 0)
+      << "File not mapped writable. Cannot unquicken! " << mmap_;
+  if (HasDexSection()) {
+    std::vector<std::unique_ptr<const DexFile>> unique_ptr_dex_files;
+    std::string error_msg;
+    if (!OpenAllDexFiles(&unique_ptr_dex_files, &error_msg)) {
+      return;
+    }
+    Unquicken(MakeNonOwningPointerVector(unique_ptr_dex_files),
+              decompile_return_instruction);
+    // Update the quickening info size to pretend there isn't any.
+    size_t offset = GetDexSectionHeaderOffset();
+    reinterpret_cast<DexSectionHeader*>(mmap_.Begin() + offset)->quickening_info_size_ = 0;
+  }
+}
+
 void VdexFile::Unquicken(const std::vector<const DexFile*>& target_dex_files,
                          bool decompile_return_instruction) const {
   const uint8_t* source_dex = GetNextDexFileData(nullptr);
@@ -279,7 +308,8 @@ static ArrayRef<const uint8_t> GetQuickeningInfoAt(const ArrayRef<const uint8_t>
 void VdexFile::UnquickenDexFile(const DexFile& target_dex_file,
                                 const DexFile& source_dex_file,
                                 bool decompile_return_instruction) const {
-  UnquickenDexFile(target_dex_file, source_dex_file.Begin(), decompile_return_instruction);
+  UnquickenDexFile(
+      target_dex_file, source_dex_file.Begin(), decompile_return_instruction);
 }
 
 void VdexFile::UnquickenDexFile(const DexFile& target_dex_file,
diff --git a/runtime/vdex_file.h b/runtime/vdex_file.h
index 102b73fdae..d2059042c1 100644
--- a/runtime/vdex_file.h
+++ b/runtime/vdex_file.h
@@ -309,7 +309,7 @@ class VdexFile {
 
   // Open all the dex files contained in this vdex file.
   bool OpenAllDexFiles(std::vector<std::unique_ptr<const DexFile>>* dex_files,
-                       std::string* error_msg);
+                       std::string* error_msg) const;
 
   // In-place unquicken the given `dex_files` based on `quickening_info`.
   // `decompile_return_instruction` controls if RETURN_VOID_BARRIER instructions are
@@ -319,6 +319,8 @@ class VdexFile {
   void Unquicken(const std::vector<const DexFile*>& target_dex_files,
                  bool decompile_return_instruction) const;
 
+  void UnquickenInPlace(bool decompile_return_instruction) const;
+
   // Fully unquicken `target_dex_file` based on `quickening_info`.
   void UnquickenDexFile(const DexFile& target_dex_file,
                         const DexFile& source_dex_file,
@@ -354,6 +356,10 @@ class VdexFile {
   // Returns true if the class loader context stored in the vdex matches `context`.
   bool MatchesClassLoaderContext(const ClassLoaderContext& context) const;
 
+  // Make the Vdex file & underlying dex-files RW or RO. Should only be used for in-place
+  // dequickening.
+  void AllowWriting(bool value) const;
+
  private:
   uint32_t GetQuickeningInfoTableOffset(const uint8_t* source_dex_begin) const;
 
@@ -382,7 +388,8 @@ class VdexFile {
     return DexBegin() + GetDexSectionHeader().GetDexSize();
   }
 
-  MemMap mmap_;
+  // mutable for AllowWriting()
+  mutable MemMap mmap_;
 
   DISALLOW_COPY_AND_ASSIGN(VdexFile);
 };
diff --git a/test/1995-final-virtual-structural-multithread/run b/test/1995-final-virtual-structural-multithread/run
index 421f7b0bf2..e912529e38 100755
--- a/test/1995-final-virtual-structural-multithread/run
+++ b/test/1995-final-virtual-structural-multithread/run
@@ -18,4 +18,4 @@
 # iget-object-quick during dex2dex compilation. This breaks the test since the
 # -quick opcode encodes the exact byte offset of fields. Since this test changes
 # the offset this causes problems.
-./default-run "$@" --jvmti --runtime-option -Xopaque-jni-ids:true -Xcompiler-option --debuggable
+./default-run "$@" --jvmti --runtime-option -Xopaque-jni-ids:true