Closed dicej closed 9 years ago
Thanks for doing this so quickly @dicej...It looks good to me, but I am too nervous to merge it because I am not as familiar in the C portions. @joshuawarner32, do you want to give it a second opinion?
I'm a little uncertain how this works for the findVirtualMethod
case. (@dicej?)
However, I think these are pretty safe changes, and I'm going to assume that test failed before the corresponding vm changes. So I'm find with merging.
Yeah, I was wondering about findVirtualMethod
, too, and wrote a test for it, but couldn't make it fail. Then I looked for where the class was being loaded in GDB, and found that compileVirtualMethod2
loads the class at the last moment, which reminded me that bootstrap classes do have vtables before they are loaded, just not interface vtables (see bootJavaClass
for details). So we don't need to do anything special in findVirtualMethod
.
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.
Fixes #404