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

[Krystal Mo]

The fix is being tested with (1) JPRT, (2) jtreg compiler and (3) jtreg 
java/lang/invoke

I'm seeing some javac compilation timeouts in (3), but the tests that 
built fine are passing.
Will update on test results when all three tests finish.

- Kris

On 01/24/2013 04:06 PM, 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