summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Nicolas Geoffray <ngeoffray@google.com> 2017-08-07 16:52:40 +0100
committer Nicolas Geoffray <ngeoffray@google.com> 2017-08-07 18:01:57 +0100
commit48b40cc97377fe67657b9936ad19395c4218b489 (patch)
tree6998d031e17120b8165e1f9202f433bcdda8dca1
parentc7c25d5b4cf243cbbf06f2e4302a0faa5eccb42a (diff)
Use the right class loader allocator in ReallocMethods.
Otherwise we would leak memory. Also complies with the JitCodeCache assumption that ArtMethods are allocated with the classloader that loaded the class where those ArtMethods hang. bug: 64241268 Test: 661-classloader-allocator Change-Id: I9e4ddbf8f40547084a1a4983459db48f68743cc7
-rw-r--r--runtime/class_linker.cc2
-rw-r--r--runtime/stack.cc13
-rw-r--r--test/661-classloader-allocator/expected.txt1
-rw-r--r--test/661-classloader-allocator/info.txt3
-rw-r--r--test/661-classloader-allocator/src-ex/OtherClass.java28
-rw-r--r--test/661-classloader-allocator/src/Main.java62
-rw-r--r--test/common/runtime_state.cc13
7 files changed, 116 insertions, 6 deletions
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 3ac87c5137..fc14da1067 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -7119,7 +7119,7 @@ void ClassLinker::LinkInterfaceMethodsHelper::ReallocMethods() {
method_alignment_);
const size_t old_methods_ptr_size = (old_methods != nullptr) ? old_size : 0;
auto* methods = reinterpret_cast<LengthPrefixedArray<ArtMethod>*>(
- Runtime::Current()->GetLinearAlloc()->Realloc(
+ class_linker_->GetAllocatorForClassLoader(klass_->GetClassLoader())->Realloc(
self_, old_methods, old_methods_ptr_size, new_size));
CHECK(methods != nullptr); // Native allocation failure aborts.
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 19df0d26a1..2e0653666f 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -634,7 +634,7 @@ static void AssertPcIsWithinQuickCode(ArtMethod* method, uintptr_t pc)
void StackVisitor::SanityCheckFrame() const {
if (kIsDebugBuild) {
ArtMethod* method = GetMethod();
- auto* declaring_class = method->GetDeclaringClass();
+ mirror::Class* declaring_class = method->GetDeclaringClass();
// Runtime methods have null declaring class.
if (!method->IsRuntimeMethod()) {
CHECK(declaring_class != nullptr);
@@ -647,11 +647,14 @@ void StackVisitor::SanityCheckFrame() const {
LinearAlloc* const linear_alloc = runtime->GetLinearAlloc();
if (!linear_alloc->Contains(method)) {
// Check class linker linear allocs.
- mirror::Class* klass = method->GetDeclaringClass();
+ // We get the canonical method as copied methods may have their declaring
+ // class from another class loader.
+ ArtMethod* canonical = method->GetCanonicalMethod();
+ mirror::Class* klass = canonical->GetDeclaringClass();
LinearAlloc* const class_linear_alloc = (klass != nullptr)
? runtime->GetClassLinker()->GetAllocatorForClassLoader(klass->GetClassLoader())
: linear_alloc;
- if (!class_linear_alloc->Contains(method)) {
+ if (!class_linear_alloc->Contains(canonical)) {
// Check image space.
bool in_image = false;
for (auto& space : runtime->GetHeap()->GetContinuousSpaces()) {
@@ -660,14 +663,14 @@ void StackVisitor::SanityCheckFrame() const {
const auto& header = image_space->GetImageHeader();
const ImageSection& methods = header.GetMethodsSection();
const ImageSection& runtime_methods = header.GetRuntimeMethodsSection();
- const size_t offset = reinterpret_cast<const uint8_t*>(method) - image_space->Begin();
+ const size_t offset = reinterpret_cast<const uint8_t*>(canonical) - image_space->Begin();
if (methods.Contains(offset) || runtime_methods.Contains(offset)) {
in_image = true;
break;
}
}
}
- CHECK(in_image) << method->PrettyMethod() << " not in linear alloc or image";
+ CHECK(in_image) << canonical->PrettyMethod() << " not in linear alloc or image";
}
}
if (cur_quick_frame_ != nullptr) {
diff --git a/test/661-classloader-allocator/expected.txt b/test/661-classloader-allocator/expected.txt
new file mode 100644
index 0000000000..6a5618ebc6
--- /dev/null
+++ b/test/661-classloader-allocator/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/661-classloader-allocator/info.txt b/test/661-classloader-allocator/info.txt
new file mode 100644
index 0000000000..872b5e0a5a
--- /dev/null
+++ b/test/661-classloader-allocator/info.txt
@@ -0,0 +1,3 @@
+Regression test for copied methods which used to not
+be (re-)allocated with the right class loader allocator,
+which crashed the JIT profiler.
diff --git a/test/661-classloader-allocator/src-ex/OtherClass.java b/test/661-classloader-allocator/src-ex/OtherClass.java
new file mode 100644
index 0000000000..e59cb953fc
--- /dev/null
+++ b/test/661-classloader-allocator/src-ex/OtherClass.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+package p1;
+
+interface Itf {
+ public default int defaultMethod() {
+ return 42;
+ }
+}
+
+public class OtherClass implements Itf {
+ public void foo() {
+ }
+}
diff --git a/test/661-classloader-allocator/src/Main.java b/test/661-classloader-allocator/src/Main.java
new file mode 100644
index 0000000000..40f8f7a4bf
--- /dev/null
+++ b/test/661-classloader-allocator/src/Main.java
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2017 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.lang.reflect.Constructor;
+
+public class Main {
+ static final String DEX_FILE =
+ System.getenv("DEX_LOCATION") + "/661-classloader-allocator-ex.jar";
+ static final String LIBRARY_SEARCH_PATH = System.getProperty("java.library.path");
+
+ private static void doUnloading() {
+ // Stop the JIT to ensure its threads and work queue are not keeping classes
+ // artifically alive.
+ stopJit();
+ // Do multiple GCs to prevent rare flakiness if some other thread is keeping the
+ // classloader live.
+ for (int i = 0; i < 5; ++i) {
+ Runtime.getRuntime().gc();
+ }
+ startJit();
+ }
+
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+ loadClass();
+ doUnloading();
+ // fetchProfiles iterate over the ProfilingInfo, we used to crash in the presence
+ // of unloaded copied methods.
+ fetchProfiles();
+ }
+
+ public static void loadClass() throws Exception {
+ Class<?> pathClassLoader = Class.forName("dalvik.system.PathClassLoader");
+ if (pathClassLoader == null) {
+ throw new AssertionError("Couldn't find path class loader class");
+ }
+ Constructor<?> constructor =
+ pathClassLoader.getDeclaredConstructor(String.class, String.class, ClassLoader.class);
+ ClassLoader loader = (ClassLoader) constructor.newInstance(
+ DEX_FILE, LIBRARY_SEARCH_PATH, ClassLoader.getSystemClassLoader());
+ Class<?> otherClass = loader.loadClass("p1.OtherClass");
+ ensureJitCompiled(otherClass, "foo");
+ }
+
+ public static native void ensureJitCompiled(Class<?> cls, String methodName);
+ public static native void fetchProfiles();
+ public static native void stopJit();
+ public static native void startJit();
+}
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index 7c0ed691b6..60c7315b6f 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -254,4 +254,17 @@ extern "C" JNIEXPORT int JNICALL Java_Main_numberOfDeoptimizations(JNIEnv*, jcla
return Runtime::Current()->GetNumberOfDeoptimizations();
}
+extern "C" JNIEXPORT void JNICALL Java_Main_fetchProfiles(JNIEnv*, jclass) {
+ jit::Jit* jit = GetJitIfEnabled();
+ if (jit == nullptr) {
+ return;
+ }
+ jit::JitCodeCache* code_cache = jit->GetCodeCache();
+ std::vector<ProfileMethodInfo> unused_vector;
+ std::set<std::string> unused_locations;
+ unused_locations.insert("fake_location");
+ ScopedObjectAccess soa(Thread::Current());
+ code_cache->GetProfiledMethods(unused_locations, unused_vector);
+}
+
} // namespace art