Merge "Add concurrent card graying for immune spaces"
diff --git a/cmdline/cmdline_parser.h b/cmdline/cmdline_parser.h
index d82fd48..32480dd 100644
--- a/cmdline/cmdline_parser.h
+++ b/cmdline/cmdline_parser.h
@@ -612,7 +612,7 @@
 template <typename TVariantMap,
           template <typename TKeyValue> class TVariantMapKey>
 template <typename TArg>
-CmdlineParser<TVariantMap, TVariantMapKey>::ArgumentBuilder<TArg>
+typename CmdlineParser<TVariantMap, TVariantMapKey>::template ArgumentBuilder<TArg>
 CmdlineParser<TVariantMap, TVariantMapKey>::CreateArgumentBuilder(
     CmdlineParser<TVariantMap, TVariantMapKey>::Builder& parent) {
   return CmdlineParser<TVariantMap, TVariantMapKey>::ArgumentBuilder<TArg>(
diff --git a/compiler/utils/swap_space.h b/compiler/utils/swap_space.h
index c286b82..0ff9fc6 100644
--- a/compiler/utils/swap_space.h
+++ b/compiler/utils/swap_space.h
@@ -78,7 +78,7 @@
     mutable FreeByStartSet::const_iterator free_by_start_entry;
   };
   struct FreeBySizeComparator {
-    bool operator()(const FreeBySizeEntry& lhs, const FreeBySizeEntry& rhs) {
+    bool operator()(const FreeBySizeEntry& lhs, const FreeBySizeEntry& rhs) const {
       if (lhs.size != rhs.size) {
         return lhs.size < rhs.size;
       } else {
diff --git a/runtime/dex_file_annotations.cc b/runtime/dex_file_annotations.cc
index 7d56bca..1397916 100644
--- a/runtime/dex_file_annotations.cc
+++ b/runtime/dex_file_annotations.cc
@@ -1135,7 +1135,7 @@
 bool GetParametersMetadataForMethod(ArtMethod* method,
                                     MutableHandle<mirror::ObjectArray<mirror::String>>* names,
                                     MutableHandle<mirror::IntArray>* access_flags) {
-  const DexFile::AnnotationSetItem::AnnotationSetItem* annotation_set =
+  const DexFile::AnnotationSetItem* annotation_set =
       FindAnnotationSetForMethod(method);
   if (annotation_set == nullptr) {
     return false;
diff --git a/runtime/dex_to_dex_decompiler.cc b/runtime/dex_to_dex_decompiler.cc
index 85d5784..c15c9ec 100644
--- a/runtime/dex_to_dex_decompiler.cc
+++ b/runtime/dex_to_dex_decompiler.cc
@@ -32,6 +32,7 @@
                 bool decompile_return_instruction)
     : code_item_(code_item),
       quickened_info_ptr_(quickened_info.data()),
+      quickened_info_start_(quickened_info.data()),
       quickened_info_end_(quickened_info.data() + quickened_info.size()),
       decompile_return_instruction_(decompile_return_instruction) {}
 
@@ -89,6 +90,7 @@
 
   const DexFile::CodeItem& code_item_;
   const uint8_t* quickened_info_ptr_;
+  const uint8_t* const quickened_info_start_;
   const uint8_t* const quickened_info_end_;
   const bool decompile_return_instruction_;
 
@@ -185,10 +187,15 @@
   }
 
   if (quickened_info_ptr_ != quickened_info_end_) {
-    LOG(FATAL) << "Failed to use all values in quickening info."
-               << " Actual: " << std::hex << quickened_info_ptr_
-               << " Expected: " << quickened_info_end_;
-    return false;
+    if (quickened_info_start_ == quickened_info_ptr_) {
+      LOG(WARNING) << "Failed to use any value in quickening info,"
+                   << " potentially due to duplicate methods.";
+    } else {
+      LOG(FATAL) << "Failed to use all values in quickening info."
+                 << " Actual: " << std::hex << reinterpret_cast<uintptr_t>(quickened_info_ptr_)
+                 << " Expected: " << reinterpret_cast<uintptr_t>(quickened_info_end_);
+      return false;
+    }
   }
 
   return true;
diff --git a/runtime/gc/accounting/mod_union_table.cc b/runtime/gc/accounting/mod_union_table.cc
index 34e30c1..c416b9c 100644
--- a/runtime/gc/accounting/mod_union_table.cc
+++ b/runtime/gc/accounting/mod_union_table.cc
@@ -391,7 +391,7 @@
     uintptr_t end = start + CardTable::kCardSize;
     live_bitmap->VisitMarkedRange(start,
                                   end,
-                                  [this, callback, arg](mirror::Object* obj) {
+                                  [callback, arg](mirror::Object* obj) {
       callback(obj, arg);
     });
   }
@@ -402,7 +402,7 @@
     uintptr_t end = start + CardTable::kCardSize;
     live_bitmap->VisitMarkedRange(start,
                                   end,
-                                  [this, callback, arg](mirror::Object* obj) {
+                                  [callback, arg](mirror::Object* obj) {
       callback(obj, arg);
     });
   }
@@ -560,7 +560,7 @@
             << start << " " << *space_;
         space_->GetLiveBitmap()->VisitMarkedRange(start,
                                                   start + CardTable::kCardSize,
-                                                  [this, callback, arg](mirror::Object* obj) {
+                                                  [callback, arg](mirror::Object* obj) {
           callback(obj, arg);
         });
       });
diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc
index cab293f..9d3d950 100644
--- a/runtime/gc/collector/mark_compact.cc
+++ b/runtime/gc/collector/mark_compact.cc
@@ -140,7 +140,7 @@
       }
     } else {
       DCHECK(!space_->HasAddress(obj));
-      auto slow_path = [this](const mirror::Object* ref)
+      auto slow_path = [](const mirror::Object* ref)
           REQUIRES_SHARED(Locks::mutator_lock_) {
         // Marking a large object, make sure its aligned as a sanity check.
         if (!IsAligned<kPageSize>(ref)) {
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 6799918..932d5ed 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -546,8 +546,8 @@
   std::vector<const DexFile*> dex_files_loaded;
 
   // Try to get dex files from the given class loader. If the class loader is null, or we do
-  // not support one of the class loaders in the chain, conservatively compare against all
-  // (non-boot) oat files.
+  // not support one of the class loaders in the chain, we do nothing and assume the collision
+  // check has succeeded.
   bool class_loader_ok = false;
   {
     ScopedObjectAccess soa(Thread::Current());
@@ -566,37 +566,20 @@
     } else if (h_class_loader != nullptr) {
       VLOG(class_linker) << "Something unsupported with "
                          << mirror::Class::PrettyClass(h_class_loader->GetClass());
+
+      // This is a class loader we don't recognize. Our earlier strategy would
+      // be to perform a global duplicate class check (with all loaded oat files)
+      // but that seems overly conservative - we have no way of knowing that
+      // those files are present in the same loader hierarchy. Among other
+      // things, it hurt GMS core and its filtering class loader.
     }
   }
 
-  // Dex files are registered late - once a class is actually being loaded. We have to compare
-  // against the open oat files. Take the oat_file_manager_lock_ that protects oat_files_ accesses.
-  ReaderMutexLock mu(Thread::Current(), *Locks::oat_file_manager_lock_);
-
-  // Vector that holds the newly opened dex files live, this is done to prevent leaks.
-  std::vector<std::unique_ptr<const DexFile>> opened_dex_files;
-
+  // Exit if we find a class loader we don't recognize. Proceed to check shared
+  // libraries and do a full class loader check otherwise.
   if (!class_loader_ok) {
-    // Add dex files from already loaded oat files, but skip boot.
-
-    // Clean up the dex files.
-    dex_files_loaded.clear();
-
-    std::vector<const OatFile*> boot_oat_files = GetBootOatFiles();
-    // The same OatFile can be loaded multiple times at different addresses. In this case, we don't
-    // need to check both against each other since they would have resolved the same way at compile
-    // time.
-    std::unordered_set<std::string> unique_locations;
-    for (const std::unique_ptr<const OatFile>& loaded_oat_file : oat_files_) {
-      DCHECK_NE(loaded_oat_file.get(), oat_file);
-      const std::string& location = loaded_oat_file->GetLocation();
-      if (std::find(boot_oat_files.begin(), boot_oat_files.end(), loaded_oat_file.get()) ==
-          boot_oat_files.end() && location != oat_file->GetLocation() &&
-          unique_locations.find(location) == unique_locations.end()) {
-        unique_locations.insert(location);
-        AddDexFilesFromOat(loaded_oat_file.get(), &dex_files_loaded, &opened_dex_files);
-      }
-    }
+      LOG(WARNING) << "Skipping duplicate class check due to unrecognized classloader";
+      return false;
   }
 
   // Exit if shared libraries are ok. Do a full duplicate classes check otherwise.
@@ -606,6 +589,9 @@
     return false;
   }
 
+  // Vector that holds the newly opened dex files live, this is done to prevent leaks.
+  std::vector<std::unique_ptr<const DexFile>> opened_dex_files;
+
   ScopedTrace st("Collision check");
   // Add dex files from the oat file to check.
   std::vector<const DexFile*> dex_files_unloaded;
@@ -677,21 +663,34 @@
     if (!accept_oat_file) {
       // Failed the collision check. Print warning.
       if (Runtime::Current()->IsDexFileFallbackEnabled()) {
-        LOG(WARNING) << "Found duplicate classes, falling back to interpreter mode for "
-                     << dex_location;
+        if (!oat_file_assistant.HasOriginalDexFiles()) {
+          // We need to fallback but don't have original dex files. We have to
+          // fallback to opening the existing oat file. This is potentially
+          // unsafe so we warn about it.
+          accept_oat_file = true;
+
+          LOG(WARNING) << "Dex location " << dex_location << " does not seem to include dex file. "
+                       << "Allow oat file use. This is potentially dangerous.";
+        } else {
+          // We have to fallback and found original dex files - extract them from an APK.
+          // Also warn about this operation because it's potentially wasteful.
+          LOG(WARNING) << "Found duplicate classes, falling back to extracting from APK : "
+                       << dex_location;
+          LOG(WARNING) << "NOTE: This wastes RAM and hurts startup performance.";
+        }
       } else {
+        // TODO: We should remove this. The fact that we're here implies -Xno-dex-file-fallback
+        // was set, which means that we should never fallback. If we don't have original dex
+        // files, we should just fail resolution as the flag intended.
+        if (!oat_file_assistant.HasOriginalDexFiles()) {
+          accept_oat_file = true;
+        }
+
         LOG(WARNING) << "Found duplicate classes, dex-file-fallback disabled, will be failing to "
                         " load classes for " << dex_location;
       }
-      LOG(WARNING) << error_msg;
 
-      // However, if the app was part of /system and preopted, there is no original dex file
-      // available. In that case grudgingly accept the oat file.
-      if (!oat_file_assistant.HasOriginalDexFiles()) {
-        accept_oat_file = true;
-        LOG(WARNING) << "Dex location " << dex_location << " does not seem to include dex file. "
-                     << "Allow oat file use. This is potentially dangerous.";
-      }
+      LOG(WARNING) << error_msg;
     }
 
     if (accept_oat_file) {
diff --git a/test/649-vdex-duplicate-method/classes.dex b/test/649-vdex-duplicate-method/classes.dex
new file mode 100644
index 0000000..8036a2f
--- /dev/null
+++ b/test/649-vdex-duplicate-method/classes.dex
Binary files differ
diff --git a/test/649-vdex-duplicate-method/expected.txt b/test/649-vdex-duplicate-method/expected.txt
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/test/649-vdex-duplicate-method/expected.txt
@@ -0,0 +1 @@
+0
diff --git a/test/649-vdex-duplicate-method/info.txt b/test/649-vdex-duplicate-method/info.txt
new file mode 100644
index 0000000..d2c9959
--- /dev/null
+++ b/test/649-vdex-duplicate-method/info.txt
@@ -0,0 +1 @@
+Regression test for unquickening a vdex that has duplicate methods.