ART: Simple structural class check

Adds a simple check to class-loading when the embedded dex file in
an oat file and the dex file on the class path where we found the
class do not match.

We require that the number of methods and fields do not change, as
that will almost certainly mean that quickened and other compiled
offsets are wrong now. This is a reasonably lightweight change, but
we should investigate a full comparison including name and type of
members.

Bug: 17937814
Bug: 18708951

(cherry picked from commit 15a33b3f88546bce85dcb9d28caf200da51154d7)

Change-Id: Icb9638bebd369ab23822817f4a97c8dd8625fea5

12 files changed, 371 insertions, 7 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 438cebfff4..377a3c37df 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4486,6 +4486,171 @@ bool ClassLinker::LinkClass(Thread* self, const char* descriptor, Handle<mirror:
   return true;
 }
 
+static void CountMethodsAndFields(ClassDataItemIterator& dex_data,
+                                  size_t* virtual_methods,
+                                  size_t* direct_methods,
+                                  size_t* static_fields,
+                                  size_t* instance_fields) {
+  *virtual_methods = *direct_methods = *static_fields = *instance_fields = 0;
+
+  while (dex_data.HasNextStaticField()) {
+    dex_data.Next();
+    (*static_fields)++;
+  }
+  while (dex_data.HasNextInstanceField()) {
+    dex_data.Next();
+    (*instance_fields)++;
+  }
+  while (dex_data.HasNextDirectMethod()) {
+    (*direct_methods)++;
+    dex_data.Next();
+  }
+  while (dex_data.HasNextVirtualMethod()) {
+    (*virtual_methods)++;
+    dex_data.Next();
+  }
+  DCHECK(!dex_data.HasNext());
+}
+
+static void DumpClass(std::ostream& os,
+                      const DexFile& dex_file, const DexFile::ClassDef& dex_class_def,
+                      const char* suffix) {
+  ClassDataItemIterator dex_data(dex_file, dex_file.GetClassData(dex_class_def));
+  os << dex_file.GetClassDescriptor(dex_class_def) << suffix << ":\n";
+  os << " Static fields:\n";
+  while (dex_data.HasNextStaticField()) {
+    const DexFile::FieldId& id = dex_file.GetFieldId(dex_data.GetMemberIndex());
+    os << "  " << dex_file.GetFieldTypeDescriptor(id) << " " << dex_file.GetFieldName(id) << "\n";
+    dex_data.Next();
+  }
+  os << " Instance fields:\n";
+  while (dex_data.HasNextInstanceField()) {
+    const DexFile::FieldId& id = dex_file.GetFieldId(dex_data.GetMemberIndex());
+    os << "  " << dex_file.GetFieldTypeDescriptor(id) << " " << dex_file.GetFieldName(id) << "\n";
+    dex_data.Next();
+  }
+  os << " Direct methods:\n";
+  while (dex_data.HasNextDirectMethod()) {
+    const DexFile::MethodId& id = dex_file.GetMethodId(dex_data.GetMemberIndex());
+    os << "  " << dex_file.GetMethodName(id) << dex_file.GetMethodSignature(id).ToString() << "\n";
+    dex_data.Next();
+  }
+  os << " Virtual methods:\n";
+  while (dex_data.HasNextVirtualMethod()) {
+    const DexFile::MethodId& id = dex_file.GetMethodId(dex_data.GetMemberIndex());
+    os << "  " << dex_file.GetMethodName(id) << dex_file.GetMethodSignature(id).ToString() << "\n";
+    dex_data.Next();
+  }
+}
+
+static std::string DumpClasses(const DexFile& dex_file1, const DexFile::ClassDef& dex_class_def1,
+                               const DexFile& dex_file2, const DexFile::ClassDef& dex_class_def2) {
+  std::ostringstream os;
+  DumpClass(os, dex_file1, dex_class_def1, " (Compile time)");
+  DumpClass(os, dex_file2, dex_class_def2, " (Runtime)");
+  return os.str();
+}
+
+
+// Very simple structural check on whether the classes match. Only compares the number of
+// methods and fields.
+static bool SimpleStructuralCheck(const DexFile& dex_file1, const DexFile::ClassDef& dex_class_def1,
+                                  const DexFile& dex_file2, const DexFile::ClassDef& dex_class_def2,
+                                  std::string* error_msg) {
+  ClassDataItemIterator dex_data1(dex_file1, dex_file1.GetClassData(dex_class_def1));
+  ClassDataItemIterator dex_data2(dex_file2, dex_file2.GetClassData(dex_class_def2));
+
+  // Counters for current dex file.
+  size_t dex_virtual_methods1, dex_direct_methods1, dex_static_fields1, dex_instance_fields1;
+  CountMethodsAndFields(dex_data1, &dex_virtual_methods1, &dex_direct_methods1, &dex_static_fields1,
+                        &dex_instance_fields1);
+  // Counters for compile-time dex file.
+  size_t dex_virtual_methods2, dex_direct_methods2, dex_static_fields2, dex_instance_fields2;
+  CountMethodsAndFields(dex_data2, &dex_virtual_methods2, &dex_direct_methods2, &dex_static_fields2,
+                        &dex_instance_fields2);
+
+  if (dex_virtual_methods1 != dex_virtual_methods2) {
+    std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
+    *error_msg = StringPrintf("Virtual method count off: %zu vs %zu\n%s", dex_virtual_methods1,
+                              dex_virtual_methods2, class_dump.c_str());
+    return false;
+  }
+  if (dex_direct_methods1 != dex_direct_methods2) {
+    std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
+    *error_msg = StringPrintf("Direct method count off: %zu vs %zu\n%s", dex_direct_methods1,
+                              dex_direct_methods2, class_dump.c_str());
+    return false;
+  }
+  if (dex_static_fields1 != dex_static_fields2) {
+    std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
+    *error_msg = StringPrintf("Static field count off: %zu vs %zu\n%s", dex_static_fields1,
+                              dex_static_fields2, class_dump.c_str());
+    return false;
+  }
+  if (dex_instance_fields1 != dex_instance_fields2) {
+    std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
+    *error_msg = StringPrintf("Instance field count off: %zu vs %zu\n%s", dex_instance_fields1,
+                              dex_instance_fields2, class_dump.c_str());
+    return false;
+  }
+
+  return true;
+}
+
+// Checks whether a the super-class changed from what we had at compile-time. This would
+// invalidate quickening.
+static bool CheckSuperClassChange(Handle<mirror::Class> klass,
+                                  const DexFile& dex_file,
+                                  const DexFile::ClassDef& class_def,
+                                  mirror::Class* super_class)
+    SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+  // Check for unexpected changes in the superclass.
+  // Quick check 1) is the super_class class-loader the boot class loader? This always has
+  // precedence.
+  if (super_class->GetClassLoader() != nullptr &&
+      // Quick check 2) different dex cache? Breaks can only occur for different dex files,
+      // which is implied by different dex cache.
+      klass->GetDexCache() != super_class->GetDexCache()) {
+    // Now comes the expensive part: things can be broken if (a) the klass' dex file has a
+    // definition for the super-class, and (b) the files are in separate oat files. The oat files
+    // are referenced from the dex file, so do (b) first. Only relevant if we have oat files.
+    const OatFile* class_oat_file = dex_file.GetOatFile();
+    if (class_oat_file != nullptr) {
+      const OatFile* loaded_super_oat_file = super_class->GetDexFile().GetOatFile();
+      if (loaded_super_oat_file != nullptr && class_oat_file != loaded_super_oat_file) {
+        // Now check (a).
+        const DexFile::ClassDef* super_class_def = dex_file.FindClassDef(class_def.superclass_idx_);
+        if (super_class_def != nullptr) {
+          // Uh-oh, we found something. Do our check.
+          std::string error_msg;
+          if (!SimpleStructuralCheck(dex_file, *super_class_def,
+                                     super_class->GetDexFile(), *super_class->GetClassDef(),
+                                     &error_msg)) {
+            // Print a warning to the log. This exception might be caught, e.g., as common in test
+            // drivers. When the class is later tried to be used, we re-throw a new instance, as we
+            // only save the type of the exception.
+            LOG(WARNING) << "Incompatible structural change detected: " <<
+                StringPrintf(
+                    "Structural change of %s is hazardous (%s at compile time, %s at runtime): %s",
+                    PrettyType(super_class_def->class_idx_, dex_file).c_str(),
+                    class_oat_file->GetLocation().c_str(),
+                    loaded_super_oat_file->GetLocation().c_str(),
+                    error_msg.c_str());
+            ThrowIncompatibleClassChangeError(klass.Get(),
+                "Structural change of %s is hazardous (%s at compile time, %s at runtime): %s",
+                PrettyType(super_class_def->class_idx_, dex_file).c_str(),
+                class_oat_file->GetLocation().c_str(),
+                loaded_super_oat_file->GetLocation().c_str(),
+                error_msg.c_str());
+            return false;
+          }
+        }
+      }
+    }
+  }
+  return true;
+}
+
 bool ClassLinker::LoadSuperAndInterfaces(Handle<mirror::Class> klass, const DexFile& dex_file) {
   CHECK_EQ(mirror::Class::kStatusIdx, klass->GetStatus());
   const DexFile::ClassDef& class_def = dex_file.GetClassDef(klass->GetDexClassDefIndex());
@@ -4505,6 +4670,11 @@ bool ClassLinker::LoadSuperAndInterfaces(Handle<mirror::Class> klass, const DexF
     }
     CHECK(super_class->IsResolved());
     klass->SetSuperClass(super_class);
+
+    if (!CheckSuperClassChange(klass, dex_file, class_def, super_class)) {
+      DCHECK(Thread::Current()->IsExceptionPending());
+      return false;
+    }
   }
   const DexFile::TypeList* interfaces = dex_file.GetInterfacesList(class_def);
   if (interfaces != nullptr) {
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 3f6175f663..dc85f6c2c9 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -250,6 +250,7 @@ std::unique_ptr<const DexFile> DexFile::OpenMemory(const std::string& location,
                     location,
                     location_checksum,
                     mem_map,
+                    nullptr,
                     error_msg);
 }
 
@@ -337,9 +338,12 @@ std::unique_ptr<const DexFile> DexFile::OpenMemory(const uint8_t* base,
                                                    size_t size,
                                                    const std::string& location,
                                                    uint32_t location_checksum,
-                                                   MemMap* mem_map, std::string* error_msg) {
+                                                   MemMap* mem_map,
+                                                   const OatFile* oat_file,
+                                                   std::string* error_msg) {
   CHECK_ALIGNED(base, 4);  // various dex file structures must be word aligned
-  std::unique_ptr<DexFile> dex_file(new DexFile(base, size, location, location_checksum, mem_map));
+  std::unique_ptr<DexFile> dex_file(
+      new DexFile(base, size, location, location_checksum, mem_map, oat_file));
   if (!dex_file->Init(error_msg)) {
     dex_file.reset();
   }
@@ -349,7 +353,8 @@ std::unique_ptr<const DexFile> DexFile::OpenMemory(const uint8_t* base,
 DexFile::DexFile(const uint8_t* base, size_t size,
                  const std::string& location,
                  uint32_t location_checksum,
-                 MemMap* mem_map)
+                 MemMap* mem_map,
+                 const OatFile* oat_file)
     : begin_(base),
       size_(size),
       location_(location),
@@ -363,7 +368,8 @@ DexFile::DexFile(const uint8_t* base, size_t size,
       proto_ids_(reinterpret_cast<const ProtoId*>(base + header_->proto_ids_off_)),
       class_defs_(reinterpret_cast<const ClassDef*>(base + header_->class_defs_off_)),
       find_class_def_misses_(0),
-      class_def_index_(nullptr) {
+      class_def_index_(nullptr),
+      oat_file_(oat_file) {
   CHECK(begin_ != NULL) << GetLocation();
   CHECK_GT(size_, 0U) << GetLocation();
 }
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 019c8e6e9a..9b8f254448 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -44,6 +44,7 @@ namespace mirror {
 }  // namespace mirror
 class ClassLinker;
 class MemMap;
+class OatFile;
 class Signature;
 template<class T> class Handle;
 class StringPiece;
@@ -391,8 +392,9 @@ class DexFile {
   static std::unique_ptr<const DexFile> Open(const uint8_t* base, size_t size,
                                              const std::string& location,
                                              uint32_t location_checksum,
+                                             const OatFile* oat_file,
                                              std::string* error_msg) {
-    return OpenMemory(base, size, location, location_checksum, NULL, error_msg);
+    return OpenMemory(base, size, location, location_checksum, NULL, oat_file, error_msg);
   }
 
   // Open all classesXXX.dex files from a zip archive.
@@ -891,6 +893,10 @@ class DexFile {
   //     the dex_location where it's file name part has been made canonical.
   static std::string GetDexCanonicalLocation(const char* dex_location);
 
+  const OatFile* GetOatFile() const {
+    return oat_file_;
+  }
+
  private:
   // Opens a .dex file
   static std::unique_ptr<const DexFile> OpenFile(int fd, const char* location,
@@ -927,12 +933,14 @@ class DexFile {
                                                    const std::string& location,
                                                    uint32_t location_checksum,
                                                    MemMap* mem_map,
+                                                   const OatFile* oat_file,
                                                    std::string* error_msg);
 
   DexFile(const uint8_t* base, size_t size,
           const std::string& location,
           uint32_t location_checksum,
-          MemMap* mem_map);
+          MemMap* mem_map,
+          const OatFile* oat_file);
 
   // Top-level initializer that calls other Init methods.
   bool Init(std::string* error_msg);
@@ -1015,6 +1023,10 @@ class DexFile {
   };
   typedef HashMap<const char*, const ClassDef*, UTF16EmptyFn, UTF16HashCmp, UTF16HashCmp> Index;
   mutable Atomic<Index*> class_def_index_;
+
+  // The oat file this dex file was loaded from. May be null in case the dex file is not coming
+  // from an oat file, e.g., directly from an apk.
+  const OatFile* oat_file_;
 };
 std::ostream& operator<<(std::ostream& os, const DexFile& dex_file);
 
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 358519b7af..9061bb3d55 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -456,7 +456,7 @@ size_t OatFile::OatDexFile::FileSize() const {
 
 std::unique_ptr<const DexFile> OatFile::OatDexFile::OpenDexFile(std::string* error_msg) const {
   return DexFile::Open(dex_file_pointer_, FileSize(), dex_file_location_,
-                       dex_file_location_checksum_, error_msg);
+                       dex_file_location_checksum_, GetOatFile(), error_msg);
 }
 
 uint32_t OatFile::OatDexFile::GetOatClassOffset(uint16_t class_def_index) const {
diff --git a/test/131-structural-change/build b/test/131-structural-change/build
new file mode 100755
index 0000000000..7ddc81d9b8
--- /dev/null
+++ b/test/131-structural-change/build
@@ -0,0 +1,31 @@
+#!/bin/bash
+#
+# Copyright (C) 2015 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Stop if something fails.
+set -e
+
+mkdir classes
+${JAVAC} -d classes `find src -name '*.java'`
+
+mkdir classes-ex
+${JAVAC} -d classes-ex `find src-ex -name '*.java'`
+
+if [ ${NEED_DEX} = "true" ]; then
+  ${DX} -JXmx256m --debug --dex --dump-to=classes.lst --output=classes.dex --dump-width=1000 classes
+  zip $TEST_NAME.jar classes.dex
+  ${DX} -JXmx256m --debug --dex --dump-to=classes-ex.lst --output=classes.dex --dump-width=1000 classes-ex
+  zip ${TEST_NAME}-ex.jar classes.dex
+fi
diff --git a/test/131-structural-change/expected.txt b/test/131-structural-change/expected.txt
new file mode 100644
index 0000000000..cc7713d252
--- /dev/null
+++ b/test/131-structural-change/expected.txt
@@ -0,0 +1,2 @@
+Should really reach here.
+Done.
diff --git a/test/131-structural-change/info.txt b/test/131-structural-change/info.txt
new file mode 100644
index 0000000000..6d5817bcea
--- /dev/null
+++ b/test/131-structural-change/info.txt
@@ -0,0 +1 @@
+Check whether a structural change in a (non-native) multi-dex scenario is detected.
diff --git a/test/131-structural-change/run b/test/131-structural-change/run
new file mode 100755
index 0000000000..63fdb8c749
--- /dev/null
+++ b/test/131-structural-change/run
@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Copyright (C) 2015 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Use secondary switch to add secondary dex file to class path.
+exec ${RUN} "${@}" --secondary
diff --git a/test/131-structural-change/src-ex/A.java b/test/131-structural-change/src-ex/A.java
new file mode 100644
index 0000000000..800347b716
--- /dev/null
+++ b/test/131-structural-change/src-ex/A.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class A {
+    public void bar() {
+    }
+}
diff --git a/test/131-structural-change/src-ex/B.java b/test/131-structural-change/src-ex/B.java
new file mode 100644
index 0000000000..61369db32d
--- /dev/null
+++ b/test/131-structural-change/src-ex/B.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class B extends A {
+    public void test() {
+        System.out.println("Should not reach this!");
+    }
+}
diff --git a/test/131-structural-change/src/A.java b/test/131-structural-change/src/A.java
new file mode 100644
index 0000000000..b07de581d8
--- /dev/null
+++ b/test/131-structural-change/src/A.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class A {
+    public void foo() {
+    }
+
+    public void bar() {
+    }
+
+    public void baz() {
+    }
+}
diff --git a/test/131-structural-change/src/Main.java b/test/131-structural-change/src/Main.java
new file mode 100644
index 0000000000..8dfa2808a2
--- /dev/null
+++ b/test/131-structural-change/src/Main.java
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.File;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+
+/**
+ * Structural hazard test.
+ */
+public class Main {
+    public static void main(String[] args) {
+        new Main().run();
+    }
+
+    private void run() {
+        try {
+            Class<?> bClass = getClass().getClassLoader().loadClass("A");
+            System.out.println("Should really reach here.");
+        } catch (Exception e) {
+            e.printStackTrace(System.out);
+        }
+
+        boolean haveOatFile = hasOat();
+        boolean gotError = false;
+        try {
+            Class<?> bClass = getClass().getClassLoader().loadClass("B");
+        } catch (IncompatibleClassChangeError icce) {
+            gotError = true;
+        } catch (Exception e) {
+            e.printStackTrace(System.out);
+        }
+        if (haveOatFile ^ gotError) {
+            System.out.println("Did not get expected error.");
+        }
+        System.out.println("Done.");
+    }
+
+    static {
+        System.loadLibrary("arttest");
+    }
+
+    private native static boolean hasOat();
+}