From 7a4cae0dde2e97f0e5d85ece7f252b0685963aa3 Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Fri, 6 Feb 2015 13:50:59 -0700 Subject: [PATCH] load bootstrap classes in findInterfaceMethod In afbd4ff, I made a low-risk, but very specific fix for a more general problem: "bootstrap" classes (i.e. classes which the VM has built-in knowledge of) need to be loaded from the classpath before any of their methods are called. Based on recent testing, I found there were more cases than I previously thought where the VM tries to call methods on "unloaded" bootstrap classes, so we needed a more general solution to the problem. This commit addresses it by closing the last (known) loophole by which methods might be called on bootstrap classes: invokeinterface, and its helper method findInterfaceMethod. The fix is to check for bootstrap classes in findInterfaceMethod and load the full versions if necessary. This process may lead to garbage collection and/or thrown exceptions, which made me nervous about cases of direct or indirect calls to findInterfaceMethod not expecting those events, which is why I hadn't used that approach earlier. However, it turns out there were only a few places that made non-GC-safe calls to findInterfaceMethod, and a bit of code rearrangement fixed that. --- src/avian/machine.h | 7 ++++++- src/classpath-openjdk.cpp | 13 ------------- src/compile.cpp | 12 ++++++------ test/Threads.java | 7 +++++-- 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/avian/machine.h b/src/avian/machine.h index 8caa7ca847..837e9a7afc 100644 --- a/src/avian/machine.h +++ b/src/avian/machine.h @@ -2675,7 +2675,12 @@ inline GcMethod* findInterfaceMethod(Thread* t, GcMethod* method, GcClass* class_) { - assertT(t, (class_->vmFlags() & BootstrapFlag) == 0); + if (UNLIKELY(class_->vmFlags() & BootstrapFlag)) { + PROTECT(t, method); + PROTECT(t, class_); + + resolveSystemClass(t, roots(t)->bootLoader(), class_->name()); + } GcClass* interface = method->class_(); GcArray* itable = cast(t, class_->interfaceTable()); diff --git a/src/classpath-openjdk.cpp b/src/classpath-openjdk.cpp index c19c7a6353..4ec3c3c423 100644 --- a/src/classpath-openjdk.cpp +++ b/src/classpath-openjdk.cpp @@ -2452,19 +2452,10 @@ object makeJconstructor(Thread* t, } #endif // HAVE_JexecutableHasRealParameterData -void resolveBootstrap(Thread* t, Gc::Type type) -{ - if (vm::type(t, type)->vmFlags() & BootstrapFlag) { - resolveSystemClass(t, roots(t)->bootLoader(), vm::type(t, type)->name()); - } -} - object makeJmethod(Thread* t, GcMethod* vmMethod, int index) { PROTECT(t, vmMethod); - resolveBootstrap(t, GcJmethod::Type); - object name = intern(t, t->m->classpath->makeString( @@ -2567,8 +2558,6 @@ object makeJconstructor(Thread* t, GcMethod* vmMethod, int index) { PROTECT(t, vmMethod); - resolveBootstrap(t, GcJconstructor::Type); - unsigned parameterCount; unsigned returnTypeSpec; object parameterTypes = resolveParameterJTypes(t, @@ -2649,8 +2638,6 @@ object makeJfield(Thread* t, GcField* vmField, int index) { PROTECT(t, vmField); - resolveBootstrap(t, GcJfield::Type); - object name = intern(t, t->m->classpath->makeString( diff --git a/src/compile.cpp b/src/compile.cpp index 51790bb0b3..497b961b58 100644 --- a/src/compile.cpp +++ b/src/compile.cpp @@ -8639,8 +8639,6 @@ class MyProcessor : public Processor { assertT(t, ((method->flags() & ACC_STATIC) == 0) xor (this_ == 0)); - method = findMethod(t, method, this_); - const char* spec = reinterpret_cast(method->spec()->body().begin()); unsigned size = method->parameterFootprint(); @@ -8656,6 +8654,8 @@ class MyProcessor : public Processor { PROTECT(t, method); + method = findMethod(t, method, this_); + compile(static_cast(t), local::codeAllocator(static_cast(t)), 0, @@ -8677,8 +8677,6 @@ class MyProcessor : public Processor { assertT(t, ((method->flags() & ACC_STATIC) == 0) xor (this_ == 0)); - method = findMethod(t, method, this_); - const char* spec = reinterpret_cast(method->spec()->body().begin()); unsigned size = method->parameterFootprint(); @@ -8694,6 +8692,8 @@ class MyProcessor : public Processor { PROTECT(t, method); + method = findMethod(t, method, this_); + compile(static_cast(t), local::codeAllocator(static_cast(t)), 0, @@ -8716,8 +8716,6 @@ class MyProcessor : public Processor { assertT(t, ((method->flags() & ACC_STATIC) == 0) xor (this_ == 0)); - method = findMethod(t, method, this_); - const char* spec = reinterpret_cast(method->spec()->body().begin()); unsigned size = method->parameterFootprint(); @@ -8734,6 +8732,8 @@ class MyProcessor : public Processor { PROTECT(t, method); + method = findMethod(t, method, this_); + compile(static_cast(t), local::codeAllocator(static_cast(t)), 0, diff --git a/test/Threads.java b/test/Threads.java index 5fa4b780c7..5f4860bd7b 100644 --- a/test/Threads.java +++ b/test/Threads.java @@ -1,4 +1,4 @@ -public class Threads implements Runnable { +public class Threads implements Runnable { private static boolean success = false; private static void expect(boolean v) { @@ -6,6 +6,9 @@ public class Threads implements Runnable { } public static void main(String[] args) throws Exception { + ((Thread.UncaughtExceptionHandler) Thread.currentThread().getThreadGroup()) + .uncaughtException(Thread.currentThread(), new Exception()); + { Threads test = new Threads(); Thread thread = new Thread(test); @@ -41,7 +44,7 @@ public class Threads implements Runnable { // do nothing } }; - + thread.start(); thread.join(); }