Merge "ART: Simple structural class check" into lmp-mr1-dev
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index cd1a93a..702e901 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4507,6 +4507,171 @@
   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());
@@ -4526,6 +4691,11 @@
     }
     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 31d79bf..9d835f4 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -241,6 +241,7 @@
                     location,
                     location_checksum,
                     mem_map,
+                    nullptr,
                     error_msg);
 }
 
@@ -326,9 +327,12 @@
                                    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)) {
     return nullptr;
   } else {
@@ -339,7 +343,8 @@
 DexFile::DexFile(const byte* 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),
@@ -354,7 +359,8 @@
       class_defs_(reinterpret_cast<const ClassDef*>(base + header_->class_defs_off_)),
       find_class_def_misses_(0),
       class_def_index_(nullptr),
-      build_class_def_index_mutex_("DexFile index creation mutex") {
+      build_class_def_index_mutex_("DexFile index creation mutex"),
+      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 54c4b09..060562b 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -43,6 +43,7 @@
 }  // namespace mirror
 class ClassLinker;
 class MemMap;
+class OatFile;
 class Signature;
 template<class T> class Handle;
 class StringPiece;
@@ -390,8 +391,9 @@
   static 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.
@@ -889,6 +891,10 @@
   //     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 const DexFile* OpenFile(int fd, const char* location, bool verify, std::string* error_msg);
@@ -924,12 +930,14 @@
                                    const std::string& location,
                                    uint32_t location_checksum,
                                    MemMap* mem_map,
+                                   const OatFile* oat_file,
                                    std::string* error_msg);
 
   DexFile(const byte* 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);
@@ -1013,6 +1021,10 @@
   typedef HashMap<const char*, const ClassDef*, UTF16EmptyFn, UTF16HashCmp, UTF16HashCmp> Index;
   mutable Atomic<Index*> class_def_index_;
   mutable Mutex build_class_def_index_mutex_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+
+  // 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 9c06ebe..8bf7ad4 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -458,7 +458,7 @@
 
 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 0000000..7ddc81d
--- /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 0000000..79facd5
--- /dev/null
+++ b/test/131-structural-change/expected.txt
@@ -0,0 +1,2 @@
+Should really reach here.
+Got expected error.
diff --git a/test/131-structural-change/info.txt b/test/131-structural-change/info.txt
new file mode 100644
index 0000000..6d5817b
--- /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 0000000..63fdb8c
--- /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 0000000..800347b
--- /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 0000000..61369db
--- /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 0000000..b07de58
--- /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 0000000..c94843e
--- /dev/null
+++ b/test/131-structural-change/src/Main.java
@@ -0,0 +1,48 @@
+/*
+ * 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);
+        }
+
+        try {
+            Class<?> bClass = getClass().getClassLoader().loadClass("B");
+            System.out.println("Should not reach here.");
+        } catch (IncompatibleClassChangeError icce) {
+            System.out.println("Got expected error.");
+        } catch (Exception e) {
+            e.printStackTrace(System.out);
+        }
+
+    }
+
+}
diff --git a/test/run-test b/test/run-test
index e48681a..3582628 100755
--- a/test/run-test
+++ b/test/run-test
@@ -420,6 +420,7 @@
             good="yes"
         fi
     fi
+    exit
 elif [ "$update_mode" = "yes" ]; then
     "./${build}" >"$build_output" 2>&1
     build_exit="$?"