Request for reviews (M): 6939196: method handle signatures off the boot class path get linkage errors

John Rose John.Rose at Sun.COM
Tue Apr 6 14:54:21 PDT 2010

On Apr 6, 2010, at 6:41 AM, Christian Thalinger wrote:

> On Tue, 2010-03-30 at 04:06 -0700, John Rose wrote:
>> 6939196: method handle signatures off the boot class path get linkage errors
>> The bugtraq description is copied below.
>> - Adjust MethodType lookup logic to search off the BCP, but not to cache those results.
>> - Pass accessing class where it needs to be seen.
>> - A SignatureStream idiom "as_klass" for class lookup has been refactored.
> src/share/vm/classfile/systemDictionary.cpp:
> 2382   bool is_on_bcp;  // keep this true as long as we can materialize from the boot classloader
> Why do need an extra flag?  return_bcp_flag seems to always be set to
> the same value.

Just a style thing mainly:  Because return_bcp_flag is an out parameter, it is write-only.

Also, is_on_bcp is a local variable that the C++ optimizer can control fully.  (I don't like putting loop control in unknown reference variables.)

I'll move the write of return_bcp_flag out of the loop, to make its role more clear.

> src/share/vm/prims/methodHandles.cpp:
> 441       (//defc() == SystemDictionary::InvokeDynamic_klass() || //FIXME
> What is the problem here?

Oops, thanks for noticing that.  There is no problem at that point, so I'll remove the "//".  The note is to remind me of Lookup.findStatic(InvokeDynamic.class, "x", T). It's a corner case that needs some attention.  (There's a similar issue with Class.getDeclaredMethod on those magic classes.)

> Otherwise looks good.


>> Please review the JDK changes along with the JVM changes.  They are very simple:
>> - Remove workaround from MethodHandleImpl lookup code
>> - Add JUnit regression test to MethodHandlesTest
> These changes look good.

Good; thanks again.

Working on invokeGeneric now...

-- John

More information about the hotspot-compiler-dev mailing list