RFR: Lambda: 8009130 JCK lambda test fails with IllegalAccessException
Karen Kinnear
karen.kinnear at oracle.com
Thu Oct 3 14:04:12 PDT 2013
Folks -
New webrev that includes feedback from Lois, Yumin and Sergeui:
http://cr.openjdk.java.net/~acorn/8009130.4/webrev/
I also removed the defaultMethods.cpp change I had made in the last update. I heard back from
the JVMS spec representative, and we are going to need further negotiations, so I will work
that one issue and one new JCK test in a future changeset.
Latest passes all the smaller tests and tests that have given me issues in the past. Kicking off
longer regression testing in parallel with review.
Yumin,
Thank you for the comments.I'm running gcc 4.4.4, I will adopt your suggestions. I need to do a jprt run as well to
sanity check builds on all platforms.
thanks,
Karen
On Oct 2, 2013, at 6:05 PM, Yumin Qi wrote:
> Karen,
>
> I applied the patch, compiled failed (see 7-b). Maybe I am using different compiler from re. Minor suggestions about format etc.
>
> 1) src/share/vm/classfile/classFileParser.cpp
> No comments.
>
> 2) src/share/vm/classfile/defaultMethods.cpp
> No comments.
>
> 3) src/share/vm/code/dependencies.cpp
> See 4)->b)
Made suggested change.
>
> 4) src/share/vm/interpreter/linkResolver.cpp
> a) 'look' and 'looks' better be consistent
> + // Look up method in klasses, including static methods
> + // Then looks up local default methods
Fixed.
> b) Two places with twice calls to
> InstanceKlass::cast(klass())->default_methods()
>
> I guess this will be optimized by compiler? If not sure (I did not check the created code), how about:
>
> + if (result_oop == NULL) {
> + Array<Method*>* default_methods = InstanceKlass::cast(klass())->default_methods();
> + if (default_methods != NULL) {
> + result_oop = InstanceKlass::find_method(default_methods, name, signature);
> + }
> + }
Made suggested change.
> c) line 1135: empty line added.
> d) line 1159: empty line added.
> e) line 1274: empty line added.
removed.
>
> 5) src/share/vm/interpreter/linkResolver.hpp
> No comments.
>
> 6) src/share/vm/memory/heapInspection.hpp
> No comments.
>
> 7) src/share/vm/oops/instanceKlass.cpp
> a) InstanceKlass::find_method
> can be replaced with
> return hit >= 0 ? methods->at(hit) : NULL;
done.
>
> b) this change
> - return (address)(offset + InstanceMirrorKlass::offset_of_static_fields() + cast_from_oop<intptr_t>(java_mirror()));
> + return (address)(offset + InstanceMirrorKlass::offset_of_static_fields() + (intptr_t)java_mirror());
>
> caused a compile error. Cannot cast from oop to intptr_t, I am using gcc 4.4.6 on Linux.
good catch - that is a merge error - I didn't mean to revert that line. Thank you for catching it.
( am using gcc 4.4.4 on Linux)
> 8) src/share/vm/oops/instanceKlass.hpp
> Could not make suggestion about format, but better to move 'bool*' to next line and add indent?
> + #if INCLUDE_JVMTI
> + void adjust_default_methods(Method** old_methods, Method** new_methods, int methods_length,
> + bool* trace_name_printed);
> + #endif // INCLUDE_JVMTI
>
I moved methods_length also to the next line and indented.
> 9) src/share/vm/oops/klassVtable.cpp
> wrap can have a nice format.
> --- 422,494 ----
> + assert(super_method->is_default_method() || super_method->is_overpass() ||
> + super_method->is_abstract(), "default override error");
improved wrap.
> 10) src/share/vm/oops/klassVtable.hpp
> No comments.
>
> 11) src/share/vm/oops/method.cpp
> No comments.
>
> 12) src/share/vm/oops/method.hpp
> No comments.
>
> 13) src/share/vm/prims/jni.cpp
> No comments.
>
> 14 src/share/vm/prims/jvmtiRedefineClasses.cpp
> No comments.
>
> 15) src/share/vm/prims/methodHandles.cpp
> No comments.
>
> 16) src/share/vm/runtime/reflectionUtils.cpp
> No comments.
>
> 17) src/share/vm/runtime/reflectionUtils.hpp
> No comments.
>
> 18) src/share/vm/runtime/vmStructs.cpp
> No comments.
>
>
> Thanks
> Yumin
>
> On 10/2/2013 10:24 AM, Karen Kinnear wrote:
>> I updated the webrev - just the defaultMethods.cpp. This failed a jck resolveMethod00705m2 test
>> so I am negotiating a minor specification correction which I think is the behavior we want and
>> matches the updated code (and creates an AME for the result of default method resolution as
>> Keith did originally if the resultant method is abstract).
>>
>> http://cr.openjdk.java.net/~acorn/8009103.3/webrev/
>>
>> thanks,
>> Karen
>>
>>
>>
>> On Oct 1, 2013, at 10:01 PM, Karen Kinnear wrote:
>>
>>>
>>> Please review: JCK lambda test fails with IllegalAccessException
>>> Also fixes: 8013085: Default method bridges have incorrect class loading constraints
>>> 8025288: Two JCK runtime tests failed with IllegalAccessError
>>>
>>> webrev: http://cr.openjdk.java.net/~acorn/8009130.2/webrev/
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8009130
>>>
>>> Summary:
>>> See bug report for further details.
>>>
>>> After several prototypes, current approach is to simplify how default methods are handled by:
>>> 1) not using overpasses (vm generated bridges) for invoking default methods, not using a second
>>> step invokespecial
>>> - this removes the problem with invokespecial's constant pool lookup that requires class access
>>> (of the original invocation's "selected" method)
>>> 2) creating default_methods_array in the inheriting instanceKlass with public default methods (and default_vtable_indices)
>>> 3) search order for method and interface method resolution (to match the 0.6.3+ JVMS spec update):
>>> local class methods, super class methods, default methods, transitive interfaces
>>> 4) adding the default method to the vtable directly, with the default method having the correct
>>> method holder, which allows the access permission and loader constraint tests run on
>>> the resolved method to work on the correct method holder/class loader
>>> 5) overpasses are still used for errors detected during default method processing, such
>>> as ICCE for conflicts with default methods, and abstract method error for no candidate methods found,
>>> these are still added to the inheriting method's local method array and constant pool at this time
>>>
>>> My goal is to check this in by the end of the week for next Tuesday's hotspot-rt deadline.
>>>
>>> thanks,
>>> Karen
>>>
>>> Tests run: (64 bit linux)
>>> 1. 8009130 reported bug
>>> 2. 8009103 jck tests:
>>> lang/INTF/intf024/intf02402m01/intf02402m01.html
>>> lang/INTF/intf024/intf02402m11/intf02402m11.html
>>> lang/CLSS/clss475/clss47501m21/clss47501m21
>>> lang/CLSS/clss475/clss47501m211/clss47501m211
>>> 3. 8013085: Loader Constraint example
>>> 4. default methods tests
>>> 5. intfbug
>>> 6. jtreg jdk: filteroptest
>>> 7. reflection modeling tests (Reflect2, ReflectInvokeprivatesmall)
>>> 8. jcmd <pid> GC.class_stats
>>> -- tests which ran prior to latest hotspot-rt merge and will rerun in parallel with code review
>>> 9. vm.quick.testlist
>>> 10. vm.mlvm.testlist
>>> 11. jtreg jdk java.util, java.lang
>>> 12. jck vm
>>> 13. jck lang
>>> 14. nsk.hprof.testlist
>>> 15. nsk.quick-jvmti.testlist
>>> 16. jck api/java_util/TreeSet/ParallelStream.html
>>> 17. jprt -stree .
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20131003/b0dd200d/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list