Request for review (XS): JDK-8006758: LinkResolver assertion (caused by @Contended changes)

[Krystal Mo]

Thank you, Vladimir.

- Kris

On 01/24/2013 04:31 PM, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 1/24/13 12:06 AM, Krystal Mo wrote:
>> Hi all,
>>
>> Could anyone review this patch, please?
>> It's causing a lot of failures in nightlies, so I'd like to get it
>> reviewed quickly and push it ASAP.
>>
>> Webrev: http://cr.openjdk.java.net/~kmo/8006758/webrev.00/
>> Bug: https://jbs.oracle.com/bugs/browse/JDK-8006758
>> (There's no public bug for this yet. The description is copied below.)
>>
>> Bug to cover recent nightly failures with:
>>
>> # To suppress the following error report, specify this argument
>> # after -XX: or in .hotspotrc: SuppressErrorAt=/linkResolver.cpp:99
>> #
>> # A fatal error has been detected by the Java Runtime Environment:
>> #
>> # Internal Error
>> (XXX/hotspot/src/share/vm/interpreter/linkResolver.cpp:99), pid=7993,
>> tid=2731850608
>> # assert(resolved_method->intrinsic_id() == vmIntrinsics::_invokeBasic
>> || resolved_method->is_compiled_lambda_form()) failed: linkMethod must
>> return one of these
>> #
>> # JRE version: Java(TM) SE Runtime Environment (8.0) (build
>> 1.8.0-internal-fastdebug-201301181518.amurillo.hs25-b16-jdk8-b74--b00)
>> # Java VM: Java HotSpot(TM) Client VM (25.0-b16-fastdebug compiled mode,
>> sharing linux-x86 )
>> # Core dump written. Default location: XXX/core or core.7993
>> #
>> # An error report file with more information is saved as:
>> # XXX/hs_err_pid7993.log
>> #
>> # If you would like to submit a bug report, please visit:
>> # http://bugreport.sun.com/bugreport/crash.jsp
>> #
>>
>> This assertion is caused by a predicate on code privilege in
>> ClassFileParser::AnnotationCollector::annotation_index():
>>
>>    bool privileged = false;
>>    if (loader_data->is_the_null_class_loader_data()) {
>>      // Privileged code can use all annotations. Other code silently
>> drops some.
>>      privileged = true;
>>    }
>>
>> The problematic code was introduced in the @Contended change,
>> JDK-8003985: Support @Contended Annotation - JEP 142
>> It didn't take into account that, with recent NPG changes for 292, every
>> anonymous class would have a dedicated ClassLoaderData which is not the
>> null_CLD, so the condition for privileged code was not complete.
>>
>> The way how this code triggers the assertion far away goes like this:
>>
>> (1) The new MethodHandle implementation for 292 via LambdaForm uses
>> anonymous classes to represent "compiled lambda forms".
>> (2) The sole method in compiled lambda forms is marked with three
>> special marker annotations: @Hidden, @Compiled and @ForceInline.
>> (3) There's special handling in the VM to recognized these annotations
>> while parsing the class file; but these annotations are handled
>> specially only when the class is privileged, otherwise the special
>> handling for them is ignored silently.
>> (4) That information is later used to tell if a Method is a compiled
>> lambda form.
>>
>> This bug introduced an incomplete predicate for code privilege in (3),
>> silently missed the special handling, and then triggered an assertion in
>> (4).
>>
>> The fix is to treat all anonymous classes as privileged code. This is
>> assumed to be safe in an internal discussion thread.
>>
>> --- a/src/share/vm/classfile/classFileParser.cpp
>> +++ b/src/share/vm/classfile/classFileParser.cpp
>> @@ -1802,11 +1802,9 @@
>> ClassFileParser::AnnotationCollector::annotation_index(ClassLoaderData* 
>> loader_data,
>> Symbol* name) {
>>     vmSymbols::SID sid = vmSymbols::find_sid(name);
>> -  bool privileged = false;
>> -  if (loader_data->is_the_null_class_loader_data()) {
>> -    // Privileged code can use all annotations.  Other code silently
>> drops some.
>> -    privileged = true;
>> -  }
>> +  // Privileged code can use all annotations.  Other code silently
>> drops some.
>> +  bool privileged = loader_data->is_the_null_class_loader_data() ||
>> +                    loader_data->is_anonymous();
>>     switch (sid) {
>>     case
>> vmSymbols::VM_SYMBOL_ENUM_NAME(java_lang_invoke_ForceInline_signature):
>>       if (_location != _in_method)  break;  // only allow for methods
>>
>> Thanks,
>> Kris