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="$?"