Re: RFR: 8240956: SEGV in DwarfParser::process_dwarf after JDK-8234624

[Kevin Walls]

Great, thanks Yasumasa.  Don't worry, the language is not just you - 
it's often unclear in other places. 8-)  Sorry maybe I should have said 
you didn't need to resubmit the webrev for that, but a retest is nice.


Thanks
Kevin


On 27/03/2020 02:49, Yasumasa Suenaga wrote:
All tests on submit repo has been passed. 
(mach5-one-ysuenaga-JDK-8240956-3-20200327-0003-9753265)


Yasumasa

On 2020/03/27 9:07, Yasumasa Suenaga wrote:

Thanks Kevin and Serguei! and sorry for my English...

I uploaded new webrev:

   http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.05/

Diff from webrev.04 is here:

   http://hg.openjdk.java.net/jdk/submit/rev/d5f400d70e94


Thanks,

Yasumasa


On 2020/03/27 2:53, serguei.spit...@oracle.com wrote:

Hi Kevin,

Nice catch with the name "lastFrame".
I was also confused when reviewed this but did not come up with 
something better.


Thanks,
Serguei

On 3/26/20 10:40, Kevin Walls wrote:

Hi Yasumasa,

Oops, didn't catch this - I also had done some manual testing and 
in mach5 but clearly not enough.


Generally I think this looks good.

"lastFrame" can mean last as in final, or last as in previous. 
"last" is one of those annoying English words. Here it means final, 
if we get an Exception during processDwarf, use this to flag that 
we should return null from sender().  "finalFrame" would be clearer 
to me, anything else probably gets more verbose than you wanted.


Yes I like having the limit on the while loop in process_dwarf(), 
always worried how sane the information is that we are parsing 
through.


Thanks!
Kevin


On 24/03/2020 23:47, Yasumasa Suenaga wrote:

Thanks Serguei!

I will push it when I get second reviewer.


Yasumasa


On 2020/03/25 1:39, serguei.spit...@oracle.com wrote:

Hi Yasumasa,

I'm okay with this update.
My mach5 test run for this patch is passed.

Thanks,
Serguei


On 3/23/20 17:08, Yasumasa Suenaga wrote:

Hi Serguei,

Thanks for your comment!
I uploaded new webrev:

http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.04/

Also I pushed it to submit repo:

http://hg.openjdk.java.net/jdk/submit/rev/fade6a949bd1

On 2020/03/24 7:39, serguei.spit...@oracle.com wrote:

Hi Yasumasa,

The mach5 tier5 testing looks good.
The serviceability/sa/ClhsdbPstack.java is failed without fix 
and is not failed with it.


Thanks,
Serguei


On 3/23/20 10:18, serguei.spit...@oracle.com wrote:

Hi Yasumasa,

I looked at you changes.
It is hard to understand if this fully solves the issue.

http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.03/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/amd64/LinuxAMD64CFrame.java.frames.html 



@@ -34,10 +34,11 @@
   public static LinuxAMD64CFrame 
getTopFrame(LinuxDebugger dbg, Address rip, ThreadContext 
context) {

    Address libptr = dbg.findLibPtrByAddress(rip);
    Address cfa = 
context.getRegisterAsAddress(AMD64ThreadContext.RBP);

    DwarfParser dwarf = null;
+ boolean unsupportedDwarf = false;
      if (libptr != null) { // Native frame
  try {
    dwarf = new DwarfParser(libptr);
    dwarf.processDwarf(rip);
-- 


@@ -45,24 +46,33 @@
   !dwarf.isBPOffsetAvailable())
  ? 
context.getRegisterAsAddress(AMD64ThreadContext.RBP)
  : 
context.getRegisterAsAddress(dwarf.getCFARegister())

.addOffsetTo(dwarf.getCFAOffset());
  } catch (DebuggerException e) {
- // Bail out to Java frame case
+ if (dwarf != null) {
+ // DWARF processing should succeed when the frame is native
+ // but it might fail if CIE has language personality routine
+ // and/or LSDA.
+ dwarf = null;
+ unsupportedDwarf = true;
+ } else {
+ throw e;
+ }
  }
    }
      return (cfa == null) ? null
- : new LinuxAMD64CFrame(dbg, cfa, rip, dwarf);
+ : new LinuxAMD64CFrame(dbg, cfa, rip, dwarf, 
!unsupportedDwarf);

 }

@@ -121,13 +131,25 @@
   }
     return is

Re: RFR: 8240956: SEGV in DwarfParser::process_dwarf after JDK-8234624

[Yasumasa Suenaga]

Thanks Kevin! I will push it.

Yasumasa

On 2020/03/27 16:42, Kevin Walls wrote:

Great, thanks Yasumasa.  Don't worry, the language is not just you - it's often 
unclear in other places. 8-)  Sorry maybe I should have said you didn't need to 
resubmit the webrev for that, but a retest is nice.

Thanks
Kevin


On 27/03/2020 02:49, Yasumasa Suenaga wrote:

All tests on submit repo has been passed. 
(mach5-one-ysuenaga-JDK-8240956-3-20200327-0003-9753265)

Yasumasa

On 2020/03/27 9:07, Yasumasa Suenaga wrote:

Thanks Kevin and Serguei! and sorry for my English...

I uploaded new webrev:

   http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.05/

Diff from webrev.04 is here:

   http://hg.openjdk.java.net/jdk/submit/rev/d5f400d70e94


Thanks,

Yasumasa


On 2020/03/27 2:53, serguei.spit...@oracle.com wrote:

Hi Kevin,

Nice catch with the name "lastFrame".
I was also confused when reviewed this but did not come up with something 
better.

Thanks,
Serguei

On 3/26/20 10:40, Kevin Walls wrote:

Hi Yasumasa,

Oops, didn't catch this - I also had done some manual testing and in mach5 but 
clearly not enough.

Generally I think this looks good.

"lastFrame" can mean last as in final, or last as in previous. "last" is one of those 
annoying English words. Here it means final, if we get an Exception during processDwarf, use this to flag 
that we should return null from sender().  "finalFrame" would be clearer to me, anything else 
probably gets more verbose than you wanted.

Yes I like having the limit on the while loop in process_dwarf(), always 
worried how sane the information is that we are parsing through.

Thanks!
Kevin


On 24/03/2020 23:47, Yasumasa Suenaga wrote:

Thanks Serguei!

I will push it when I get second reviewer.


Yasumasa


On 2020/03/25 1:39, serguei.spit...@oracle.com wrote:

Hi Yasumasa,

I'm okay with this update.
My mach5 test run for this patch is passed.

Thanks,
Serguei


On 3/23/20 17:08, Yasumasa Suenaga wrote:

Hi Serguei,

Thanks for your comment!
I uploaded new webrev:

http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.04/

Also I pushed it to submit repo:

http://hg.openjdk.java.net/jdk/submit/rev/fade6a949bd1

On 2020/03/24 7:39, serguei.spit...@oracle.com wrote:

Hi Yasumasa,

The mach5 tier5 testing looks good.
The serviceability/sa/ClhsdbPstack.java is failed without fix and is not failed 
with it.

Thanks,
Serguei


On 3/23/20 10:18, serguei.spit...@oracle.com wrote:

Hi Yasumasa,

I looked at you changes.
It is hard to understand if this fully solves the issue.

http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.03/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/amd64/LinuxAMD64CFrame.java.frames.html

@@ -34,10 +34,11 @@
   public static LinuxAMD64CFrame getTopFrame(LinuxDebugger dbg, Address 
rip, ThreadContext context) {
    Address libptr = dbg.findLibPtrByAddress(rip);
    Address cfa = context.getRegisterAsAddress(AMD64ThreadContext.RBP);
    DwarfParser dwarf = null;
+ boolean unsupportedDwarf = false;
      if (libptr != null) { // Native frame
  try {
    dwarf = new DwarfParser(libptr);
    dwarf.processDwarf(rip);
-- 


@@ -45,24 +46,33 @@
   !dwarf.isBPOffsetAvailable())
  ? context.getRegisterAsAddress(AMD64ThreadContext.RBP)
  : context.getRegisterAsAddress(dwarf.getCFARegister())
.addOffsetTo(dwarf.getCFAOffset());
  } catch (DebuggerException e) {
- // Bail out to Java frame case
+ if (dwarf != null) {
+ // DWARF processing should succeed when the frame is native
+ // but it might fail if CIE has language personality routine
+ // and/or LSDA.
+ dwarf = null;
+ unsupportedDwarf = true;
+ } else {
+ throw e;
+ }
  }
    }
      return (cfa == null) ? null
- : new LinuxAMD64CFrame(dbg, cfa, rip, dwarf);
+ : new LinuxAMD64CFrame(dbg, cfa, rip, dwarf, !unsupportedDwarf);
  

Re: RFR: 8241585: Remove unused _recursion_counter facility from PerfTraceTime

[Claes Redestad]

On 2020-03-27 03:18, David Holmes wrote:


Yeah they confuse me. Which makes it hard to see what impact your 
changes may have.


This patch removes some internal, unused code on the JVM end that is not
observable via jstat / jvmstat. I'm happy if serviceability can weigh in
though.

The other RFE[1] I've filed to remove StatSampler[1] might be more
contentious since it changes what gets periodically stored in the
perfdata shared file. I've not yet decided if it's worth the trouble to
move ahead with that at this point.

/Claes

[1] https://bugs.openjdk.java.net/browse/JDK-8241701

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Jan Lahoda]

Hi Mandy,

Regarding the javac changes - should those be switched on/off depending 
the Target? Or, if one compiles with e.g. --release 14, will the newly 
generated output still work on JDK 14?


Jan

On 27. 03. 20 0:57, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The main 
changes are in core-libs and hotspot runtime area.  Small changes are 
made in javac, VM compiler (intrinsification of Class::isHiddenClass), 
JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized 
state (see specdiff and javadoc below for reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 



Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point
of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not registered in 
any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final fields 
cannot be overriden via reflection.  setAccessible(true) can still be 
called on reflected objects representing final fields in a hidden class 
and its access check will be suppressed but only have read-access (i.e. 
can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a hidden 
class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

    can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden class.
4. Field::setXXX method will throw IAE on a final field of a hidden class
    regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for Lookup::defineClass
    and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one primary CLD
    that holds the classes strongly referenced by its defining loader. 
There

    can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. Access 
control

    check no longer throws LinkageError but instead it will throw IAE with
    a clear message if a class fails to resolve/validate the nest host 
declared

    in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
    and generate a bridge method to desuger a method reference to a 
protected

    method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to hidden 
class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError and 
intends

to have the newly created class linked.  However, the implementation in 14
does not link the class.  A separate CSR [2] proposes to update the
implementation to match the spec.  This patch fixes the implementation.

The spec update on JVM TI, JDI and Instrumentation will be done as
a separate RFE [3].  This patch includes new tests for JVM TI and
java.instrument that validates how the existing APIs work for hidden 
classes.


javadoc/specdiff
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/ 



JVMS 5.4.4 change:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf 



CSR:
https://bugs.openjdk.java.net/browse/JDK-8238359

Thanks
Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8238359
[2] https://bugs.openjdk.java.net/browse/JDK-8240338
[3] https://bugs.openjdk.java.net/browse/JDK-8230502

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Remi Forax]

Hi Mandy,
in ReflectionFactory, why in the case of a constructor the check to the 
anonymous class is removed ?

in BytecodeGenerator, the comment "// bootstrapping issue if using condy"
can be promoted on top of clinit, because i ask myself the same question seeing 
a static block was generated

in AbstractValidatingLambdaMetafactory.java, the field caller is not used after 
all ?

regards,
Rémi

- Mail original -
> De: "mandy chung" 
> À: "valhalla-dev" , "core-libs-dev" 
> ,
> "serviceability-dev" , "hotspot-dev" 
> 
> Envoyé: Vendredi 27 Mars 2020 00:57:39
> Objet: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

> Please review the implementation of JEP 371: Hidden Classes. The main
> changes are in core-libs and hotspot runtime area.  Small changes are
> made in javac, VM compiler (intrinsification of Class::isHiddenClass),
> JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized
> state (see specdiff and javadoc below for reference).
> 
> Webrev:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03
> 
> Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point
> of view, a hidden class is a normal class except the following:
> 
> - A hidden class has no initiating class loader and is not registered in
> any dictionary.
> - A hidden class has a name containing an illegal character
> `Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature`
> returns "Lp/Foo.0x1234;".
> - A hidden class is not modifiable, i.e. cannot be redefined or
> retransformed. JVM TI IsModifableClass returns false on a hidden.
> - Final fields in a hidden class is "final".  The value of final fields
> cannot be overriden via reflection.  setAccessible(true) can still be
> called on reflected objects representing final fields in a hidden class
> and its access check will be suppressed but only have read-access (i.e.
> can do Field::getXXX but not setXXX).
> 
> Brief summary of this patch:
> 
> 1. A new Lookup::defineHiddenClass method is the API to create a hidden
> class.
> 2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG
> option that
>    can be specified when creating a hidden class.
> 3. A new Class::isHiddenClass method tests if a class is a hidden class.
> 4. Field::setXXX method will throw IAE on a final field of a hidden class
>    regardless of the value of the accessible flag.
> 5. JVM_LookupDefineClass is the new JVM entry point for Lookup::defineClass
>    and defineHiddenClass to create a class from the given bytes.
> 6. ClassLoaderData implementation is not changed.  There is one primary CLD
>    that holds the classes strongly referenced by its defining loader.
> There
>    can be zero or more additional CLDs - one per weak class.
> 7. Nest host determination is updated per revised JVMS 5.4.4. Access control
>    check no longer throws LinkageError but instead it will throw IAE with
>    a clear message if a class fails to resolve/validate the nest host
> declared
>    in NestHost/NestMembers attribute.
> 8. JFR, jcmd, JDI are updated to support hidden classes.
> 9. update javac LambdaToMethod as lambda proxy starts using nestmates
>    and generate a bridge method to desuger a method reference to a
> protected
>    method in its supertype in a different package
> 
> This patch also updates StringConcatFactory, LambdaMetaFactory, and
> LambdaForms
> to use hidden classes.  The webrev includes changes in nashorn to hidden
> class
> and I will update the webrev if JEP 372 removes it any time soon.
> 
> We uncovered a bug in Lookup::defineClass spec throws LinkageError and
> intends
> to have the newly created class linked.  However, the implementation in 14
> does not link the class.  A separate CSR [2] proposes to update the
> implementation to match the spec.  This patch fixes the implementation.
> 
> The spec update on JVM TI, JDI and Instrumentation will be done as
> a separate RFE [3].  This patch includes new tests for JVM TI and
> java.instrument that validates how the existing APIs work for hidden
> classes.
> 
> javadoc/specdiff
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/
> 
> JVMS 5.4.4 change:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf
> 
> CSR:
> https://bugs.openjdk.java.net/browse/JDK-8238359
> 
> Thanks
> Mandy
> [1] https://bugs.openjdk.java.net/browse/JDK-8238359
> [2] https://bugs.openjdk.java.net/browse/JDK-8240338
> [3] https://bugs.openjdk.java.net/browse/JDK-8230502

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

On 3/27/20 5:00 AM, Remi Forax wrote:

Hi Mandy,
in ReflectionFactory, why in the case of a constructor the check to the 
anonymous class is removed ?


Good catch.  Fixed


in BytecodeGenerator, the comment "// bootstrapping issue if using condy"
can be promoted on top of clinit, because i ask myself the same question seeing 
a static block was generated


OK, that's clearer.


in AbstractValidatingLambdaMetafactory.java, the field caller is not used after 
all ?


Thanks.  Removed.  It was left behind from an early prototype.

Below is the patch.  I will send out a new webrev and delta webrev in 
the next revision.


thanks
Mandy

diff --git 
a/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java 
b/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
--- 
a/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
+++ 
b/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java

@@ -51,7 +51,6 @@
  * System.out.printf(">>> %s\n", iii.foo(44));
  * }}
  */
-    final MethodHandles.Lookup caller;
 final Class targetClass;   // The class calling the 
meta-factory via invokedynamic "class X"
 final MethodType invokedType; // The type of the 
invoked method "(CC)II"
 final Class samBase;   // The type of the 
returned instance "interface JJ"

@@ -121,7 +120,6 @@
 "Invalid caller: %s",
 caller.lookupClass().getName()));
 }
-    this.caller = caller;
 this.targetClass = caller.lookupClass();
 this.invokedType = invokedType;

diff --git 
a/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java 
b/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
--- 
a/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
+++ 
b/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java

@@ -363,6 +363,10 @@
 clinit(cw, className(), classData);
 }

+    /*
+ *  to initialize the static final fields with the live 
class data

+ * LambdaForms can't use condy due to bootstrapping issue.
+ */
 static void clinit(ClassWriter cw, String className, 
List classData) {

 if (classData.isEmpty())
 return;
@@ -375,7 +379,6 @@

 MethodVisitor mv = cw.visitMethod(Opcodes.ACC_STATIC, 
"", "()V", null, null);

 mv.visitCode();
-    // bootstrapping issue if using condy
 mv.visitLdcInsn(Type.getType("L" + className + ";"));
 mv.visitMethodInsn(Opcodes.INVOKESTATIC, 
"java/lang/invoke/MethodHandleNatives",
    "classData", 
"(Ljava/lang/Class;)Ljava/lang/Object;", false);
diff --git 
a/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java 
b/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
--- 
a/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
+++ 
b/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java

@@ -245,7 +245,8 @@
 return new BootstrapConstructorAccessorImpl(c);
 }

-    if (noInflation && !c.getDeclaringClass().isHiddenClass()) {
+    if (noInflation && !c.getDeclaringClass().isHiddenClass()
+    && 
!ReflectUtil.isVMAnonymousClass(c.getDeclaringClass())) {

 return new MethodAccessorGenerator().
 generateConstructor(c.getDeclaringClass(),
 c.getParameterTypes(),

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[forax]

> De: "mandy chung" 
> À: "Remi Forax" 
> Cc: "valhalla-dev" , "core-libs-dev"
> , "serviceability-dev"
> , "hotspot-dev"
> 
> Envoyé: Vendredi 27 Mars 2020 16:50:55
> Objet: Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

> On 3/27/20 5:00 AM, Remi Forax wrote:

>> Hi Mandy,
>> in ReflectionFactory, why in the case of a constructor the check to the
>> anonymous class is removed ?

> Good catch. Fixed

>> in BytecodeGenerator, the comment "// bootstrapping issue if using condy"
>> can be promoted on top of clinit, because i ask myself the same question 
>> seeing
>> a static block was generated

> OK, that's clearer.

>> in AbstractValidatingLambdaMetafactory.java, the field caller is not used 
>> after
>> all ?

> Thanks. Removed. It was left behind from an early prototype.

> Below is the patch. I will send out a new webrev and delta webrev in the next
> revision.
Thanks Mandy, 
Looks good. 

Rémi 

> thanks
> Mandy

> diff --git
> a/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
> b/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
> ---
> a/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
> +++
> b/src/java.base/share/classes/java/lang/invoke/AbstractValidatingLambdaMetafactory.java
> @@ -51,7 +51,6 @@
> * System.out.printf(">>> %s\n", iii.foo(44));
> * }}
> */
> - final MethodHandles.Lookup caller;
> final Class targetClass; // The class calling the meta-factory via
> invokedynamic "class X"
> final MethodType invokedType; // The type of the invoked method "(CC)II"
> final Class samBase; // The type of the returned instance "interface JJ"
> @@ -121,7 +120,6 @@
> "Invalid caller: %s",
> caller.lookupClass().getName()));
> }
> - this.caller = caller;
> this.targetClass = caller.lookupClass();
> this.invokedType = invokedType;

> diff --git
> a/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
> b/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
> --- 
> a/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
> +++ 
> b/src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
> @@ -363,6 +363,10 @@
> clinit(cw, className(), classData);
> }

> + /*
> + *  to initialize the static final fields with the live class data
> + * LambdaForms can't use condy due to bootstrapping issue.
> + */
> static void clinit(ClassWriter cw, String className, List 
> classData)
> {
> if (classData.isEmpty())
> return;
> @@ -375,7 +379,6 @@

> MethodVisitor mv = cw.visitMethod(Opcodes.ACC_STATIC, "", "()V", null,
> null);
> mv.visitCode();
> - // bootstrapping issue if using condy
> mv.visitLdcInsn(Type.getType("L" + className + ";"));
> mv.visitMethodInsn(Opcodes.INVOKESTATIC, 
> "java/lang/invoke/MethodHandleNatives",
> "classData", "(Ljava/lang/Class;)Ljava/lang/Object;", false);
> diff --git
> a/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
> b/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
> --- a/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
> +++ b/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
> @@ -245,7 +245,8 @@
> return new BootstrapConstructorAccessorImpl(c);
> }

> - if (noInflation && !c.getDeclaringClass().isHiddenClass()) {
> + if (noInflation && !c.getDeclaringClass().isHiddenClass()
> + && !ReflectUtil.isVMAnonymousClass(c.getDeclaringClass())) {
> return new MethodAccessorGenerator().
> generateConstructor(c.getDeclaringClass(),
> c.getParameterTypes(),

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

Hi Jan,

Good point.  The javac change only applies to JDK 15 and later and the 
lambda proxy class is not a nestmate when running on JDK 14 or earlier.


I probably need the help from langtools team to fix this.  I'll give it 
a try.


Mandy

On 3/27/20 4:31 AM, Jan Lahoda wrote:

Hi Mandy,

Regarding the javac changes - should those be switched on/off 
depending the Target? Or, if one compiles with e.g. --release 14, will 
the newly generated output still work on JDK 14?


Jan

On 27. 03. 20 0:57, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The main 
changes are in core-libs and hotspot runtime area.  Small changes are 
made in javac, VM compiler (intrinsification of 
Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been reviewed 
and is in the finalized state (see specdiff and javadoc below for 
reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 



Hidden class is created via `Lookup::defineHiddenClass`. From JVM's 
point

of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not registered 
in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection.  setAccessible(true) can 
still be called on reflected objects representing final fields in a 
hidden class and its access check will be suppressed but only have 
read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

    can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden class.
4. Field::setXXX method will throw IAE on a final field of a hidden 
class

    regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

    and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
    that holds the classes strongly referenced by its defining 
loader. There

    can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. Access 
control
    check no longer throws LinkageError but instead it will throw IAE 
with
    a clear message if a class fails to resolve/validate the nest 
host declared

    in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
    and generate a bridge method to desuger a method reference to a 
protected

    method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError 
and intends
to have the newly created class linked.  However, the implementation 
in 14

does not link the class.  A separate CSR [2] proposes to update the
implementation to match the spec.  This patch fixes the implementation.

The spec update on JVM TI, JDI and Instrumentation will be done as
a separate RFE [3].  This patch includes new tests for JVM TI and
java.instrument that validates how the existing APIs work for hidden 
classes.


javadoc/specdiff
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/ 



JVMS 5.4.4 change:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf 



CSR:
https://bugs.openjdk.java.net/browse/JDK-8238359

Thanks
Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8238359
[2] https://bugs.openjdk.java.net/browse/JDK-8240338
[3] https://bugs.openjdk.java.net/browse/JDK-8230502

RFR (XS) 8241750: x86_32 build failure after JDK-8227269

[Aleksey Shipilev]

Bug:
  https://bugs.openjdk.java.net/browse/JDK-8241750

Fix:

diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c
--- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
15:33:24 2020 +0100
+++ b/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
17:47:31 2020 +0100
@@ -70,5 +70,5 @@
   return;
 }
-*(char**)bagAdd(deletedSignatures) = (char*)tag;
+*(char**)bagAdd(deletedSignatures) = (char*)jlong_to_ptr(tag);

 debugMonitorExit(classTrackLock);
@@ -118,5 +118,5 @@
 EXIT_ERROR(error,"signature");
 }
-error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, (jlong)signature);
+error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
ptr_to_jlong(signature));
 if (error != JVMTI_ERROR_NONE) {
 jvmtiDeallocate(signature);

Testing: Linux {x86_64, x86_32} x {builds, vmTestbase_nsk_jdwp}; jdk-submit 
(running)

-- 
Thanks,
-Aleksey



signature.asc
Description: OpenPGP digital signature

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

[Roman Kennke]

Looks good to me, thanks!

Roman

> Bug:
>   https://bugs.openjdk.java.net/browse/JDK-8241750
> 
> Fix:
> 
> diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
> 15:33:24 2020 +0100
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
> 17:47:31 2020 +0100
> @@ -70,5 +70,5 @@
>return;
>  }
> -*(char**)bagAdd(deletedSignatures) = (char*)tag;
> +*(char**)bagAdd(deletedSignatures) = (char*)jlong_to_ptr(tag);
> 
>  debugMonitorExit(classTrackLock);
> @@ -118,5 +118,5 @@
>  EXIT_ERROR(error,"signature");
>  }
> -error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
> (jlong)signature);
> +error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
> ptr_to_jlong(signature));
>  if (error != JVMTI_ERROR_NONE) {
>  jvmtiDeallocate(signature);
> 
> Testing: Linux {x86_64, x86_32} x {builds, vmTestbase_nsk_jdwp}; jdk-submit 
> (running)
> 



signature.asc
Description: OpenPGP digital signature

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

[Chris Plummer]

+1

Chris

On 3/27/20 10:03 AM, Roman Kennke wrote:

Looks good to me, thanks!

Roman


Bug:
   https://bugs.openjdk.java.net/browse/JDK-8241750

Fix:

diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c
--- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
15:33:24 2020 +0100
+++ b/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
17:47:31 2020 +0100
@@ -70,5 +70,5 @@
return;
  }
-*(char**)bagAdd(deletedSignatures) = (char*)tag;
+*(char**)bagAdd(deletedSignatures) = (char*)jlong_to_ptr(tag);

  debugMonitorExit(classTrackLock);
@@ -118,5 +118,5 @@
  EXIT_ERROR(error,"signature");
  }
-error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, (jlong)signature);
+error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
ptr_to_jlong(signature));
  if (error != JVMTI_ERROR_NONE) {
  jvmtiDeallocate(signature);

Testing: Linux {x86_64, x86_32} x {builds, vmTestbase_nsk_jdwp}; jdk-submit 
(running)

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

[Aleksey Shipilev]

Thanks! Trivial, right?

If so, I'll push as soon as jdk-submit clears.

-Aleksey

On 3/27/20 6:08 PM, Chris Plummer wrote:
> +1
> 
> Chris
> 
> On 3/27/20 10:03 AM, Roman Kennke wrote:
>> Looks good to me, thanks!
>>
>> Roman
>>
>>> Bug:
>>>https://bugs.openjdk.java.net/browse/JDK-8241750
>>>
>>> Fix:
>>>
>>> diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c
>>> --- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
>>> 15:33:24 2020 +0100
>>> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
>>> 17:47:31 2020 +0100
>>> @@ -70,5 +70,5 @@
>>> return;
>>>   }
>>> -*(char**)bagAdd(deletedSignatures) = (char*)tag;
>>> +*(char**)bagAdd(deletedSignatures) = (char*)jlong_to_ptr(tag);
>>>
>>>   debugMonitorExit(classTrackLock);
>>> @@ -118,5 +118,5 @@
>>>   EXIT_ERROR(error,"signature");
>>>   }
>>> -error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
>>> (jlong)signature);
>>> +error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
>>> ptr_to_jlong(signature));
>>>   if (error != JVMTI_ERROR_NONE) {
>>>   jvmtiDeallocate(signature);
>>>
>>> Testing: Linux {x86_64, x86_32} x {builds, vmTestbase_nsk_jdwp}; jdk-submit 
>>> (running)



signature.asc
Description: OpenPGP digital signature

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

[Chris Plummer]

Yeah, I think given that it fixes a broken build it should be fine to 
push right away.


Chris

On 3/27/20 10:10 AM, Aleksey Shipilev wrote:

Thanks! Trivial, right?

If so, I'll push as soon as jdk-submit clears.

-Aleksey

On 3/27/20 6:08 PM, Chris Plummer wrote:

+1

Chris

On 3/27/20 10:03 AM, Roman Kennke wrote:

Looks good to me, thanks!

Roman


Bug:
https://bugs.openjdk.java.net/browse/JDK-8241750

Fix:

diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c
--- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
15:33:24 2020 +0100
+++ b/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c  Fri Mar 27 
17:47:31 2020 +0100
@@ -70,5 +70,5 @@
 return;
   }
-*(char**)bagAdd(deletedSignatures) = (char*)tag;
+*(char**)bagAdd(deletedSignatures) = (char*)jlong_to_ptr(tag);

   debugMonitorExit(classTrackLock);
@@ -118,5 +118,5 @@
   EXIT_ERROR(error,"signature");
   }
-error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, (jlong)signature);
+error = JVMTI_FUNC_PTR(trackingEnv, SetTag)(env, klass, 
ptr_to_jlong(signature));
   if (error != JVMTI_ERROR_NONE) {
   jvmtiDeallocate(signature);

Testing: Linux {x86_64, x86_32} x {builds, vmTestbase_nsk_jdwp}; jdk-submit 
(running)

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

[Aleksey Shipilev]

On 3/27/20 6:15 PM, Chris Plummer wrote:
> Yeah, I think given that it fixes a broken build it should be fine to 
> push right away.
jdk-submit came clean, pushed.

-- 
Thanks,
-Aleksey



signature.asc
Description: OpenPGP digital signature

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Paul Sandoz]

Hi Mandy,

Very thorough, bravo!

Minor suggestions below.

Paul.

MethodHandleNatives.java
—

 142 
 143 /**
 144  * Flags for Lookup.ClassOptions
 145  */
 146 static final int
 147 NESTMATE_CLASS= 0x0001,
 148 HIDDEN_CLASS  = 0x0002,
 149 STRONG_LOADER_LINK= 0x0004,
 150 ACCESS_VM_ANNOTATIONS = 0x0008;
 151 }
 

Suggest you add a comment to keep the values in sync with the VM component.


MethodHandles.java
—

1786  * (Given the {@code Lookup} object returned this method, its 
lookup class
1787  * is a {@code Class} object for which {@link Class#getName()} 
returns a string
1788  * that is not a binary name.)

“
(The {@code Lookup} object returned from this method has a lookup class that is 
a {@code Class} object whose {@link Class#getName()} returns a string
that is not a binary name.)
“


1902 Set opts = options.length > 0 ? Set.of(options) : 
Set.of();

You can just do:

  Set opts = Set.of(options)

And/or inline it into the subsequent method call.  The implementation of Set.of 
checks the array length.


2001 ClassDefiner makeHiddenClassDefiner(byte[] bytes,

I think you can telescope the methods for non-name and name accepting since 
IIUC the name is derived from the byte[].  Thereby you can remove some code 
duplication. i.e. pull ClassDefiner.className out from ClassDefiner and place 
the logic in the factory methods.  Alternative push the factory methods into 
ClassDefiner to keep all the logic together.



3797 public enum ClassOption {

Shuffle up to be closer to the defineHiddenClass


3798 /**
3799  * This class option specifies the hidden class be added to
3800  * {@linkplain Class#getNestHost nest} of a lookup class as
3801  * a nestmate.

Suggest:

"This class option specifies the hidden class “ -> “Specifies that a hidden 
class 


3812  * This class option specifies the hidden class to have a 
strong

“Specifies that a hidden class have a …"


3813  * relationship with the class loader marked as its defining 
loader,
3814  * as a normal class or interface has with its own defining 
loader.
3815  * This means that the hidden class may be unloaded if and 
only if
3816  * its defining loader is not reachable and thus may be 
reclaimed
3817  * by a garbage collector (JLS 12.7).


StringConcatFactory.java
—

 861 // use of @ForceInline no longer has any effect

?

 862 mv.visitAnnotation("Ljdk/internal/vm/annotation/ForceInline;", 
true);
 863 mv.visitCode();





> On Mar 26, 2020, at 4:57 PM, Mandy Chung  wrote:
> 
> Please review the implementation of JEP 371: Hidden Classes. The main changes 
> are in core-libs and hotspot runtime area.  Small changes are made in javac, 
> VM compiler (intrinsification of Class::isHiddenClass), JFR, JDI, and jcmd.  
> CSR [1]has been reviewed and is in the finalized state (see specdiff and 
> javadoc below for reference).
> 
> Webrev:
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03
> 
> Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point
> of view, a hidden class is a normal class except the following:
> 
> - A hidden class has no initiating class loader and is not registered in any 
> dictionary.
> - A hidden class has a name containing an illegal character `Class::getName` 
> returns `p.Foo/0x1234` whereas `GetClassSignature` returns "Lp/Foo.0x1234;".
> - A hidden class is not modifiable, i.e. cannot be redefined or 
> retransformed. JVM TI IsModifableClass returns false on a hidden.
> - Final fields in a hidden class is "final".  The value of final fields 
> cannot be overriden via reflection.  setAccessible(true) can still be called 
> on reflected objects representing final fields in a hidden class and its 
> access check will be suppressed but only have read-access (i.e. can do 
> Field::getXXX but not setXXX).
> 
> Brief summary of this patch:
> 
> 1. A new Lookup::defineHiddenClass method is the API to create a hidden class.
> 2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG option that
>can be specified when creating a hidden class.
> 3. A new Class::isHiddenClass method tests if a class is a hidden class.
> 4. Field::setXXX method will throw IAE on a final field of a hidden class
>regardless of the value of the accessible flag.
> 5. JVM_LookupDefineClass is the new JVM entry point for Lookup::defineClass
>and defineHiddenClass to create a class from the given bytes.
> 6. ClassLoaderData implementation is not changed.  There is one primary CLD
>that holds the classes strongly referenced by its defining loader.  There
>can be zero or more additional CLDs - one per weak class.
> 7. Nest host determination is updated pe

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

On 3/27/20 11:59 AM, Paul Sandoz wrote:

Hi Mandy,

Very thorough, bravo!


Thanks.

Minor suggestions below.

Paul.

MethodHandleNatives.java
—

  142
  143 /**
  144  * Flags for Lookup.ClassOptions
  145  */
  146 static final int
  147 NESTMATE_CLASS= 0x0001,
  148 HIDDEN_CLASS  = 0x0002,
  149 STRONG_LOADER_LINK= 0x0004,
  150 ACCESS_VM_ANNOTATIONS = 0x0008;
  151 }
  


Suggest you add a comment to keep the values in sync with the VM component.


Already in the class spec of this Constants class.  The values of all 
constants defined in this Constants class are verified in sync with VM 
(see verifyConstants).




MethodHandles.java
—

1786  * (Given the {@code Lookup} object returned this method, its 
lookup class
1787  * is a {@code Class} object for which {@link Class#getName()} 
returns a string
1788  * that is not a binary name.)

“
(The {@code Lookup} object returned from this method has a lookup class that is
a {@code Class} object whose {@link Class#getName()} returns a string
that is not a binary name.)
“


1902 Set opts = options.length > 0 ? Set.of(options) : 
Set.of();

You can just do:

   Set opts = Set.of(options)

And/or inline it into the subsequent method call.  The implementation of Set.of 
checks the array length.


Great to know.  Thanks.


2001 ClassDefiner makeHiddenClassDefiner(byte[] bytes,

I think you can telescope the methods for non-name and name accepting since 
IIUC the name is derived from the byte[].  Thereby you can remove some code 
duplication. i.e. pull ClassDefiner.className out from ClassDefiner and place 
the logic in the factory methods.  Alternative push the factory methods into 
ClassDefiner to keep all the logic together.


Ok.  I will move the className out.


3797 public enum ClassOption {

Shuffle up to be closer to the defineHiddenClass


Moved before defineHiddenClass.



3798 /**
3799  * This class option specifies the hidden class be added to
3800  * {@linkplain Class#getNestHost nest} of a lookup class as
3801  * a nestmate.

Suggest:

"This class option specifies the hidden class “ -> “Specifies that a hidden 
class

3812  * This class option specifies the hidden class to have a 
strong

“Specifies that a hidden class have a …"


Specifies that a hidden class has a...



3813  * relationship with the class loader marked as its defining 
loader,
3814  * as a normal class or interface has with its own defining 
loader.
3815  * This means that the hidden class may be unloaded if and 
only if
3816  * its defining loader is not reachable and thus may be 
reclaimed
3817  * by a garbage collector (JLS 12.7).


StringConcatFactory.java
—

  861 // use of @ForceInline no longer has any effect

?


Right, I should have explained this [1].

This @ForceInline is used by BytecodeStringBuilderStrategy that 
generates code to have the same StringBuilder chain javac would emit. It 
uses `@ForceInline` annotation which may probably be for performance.  
It's believed people rarely uses this non-default strategy.  This patch 
changes StringConcatFactory to the standard defineHiddenClass method and 
hence `@ForceInline` has no effect in the generated class for this 
non-default strategy.  If it turns out to be an issue, then we will 
determine if it should enable the access to VM annotations (I doubt this 
is supported strategy).


[1] https://bugs.openjdk.java.net/browse/JDK-8241548

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Vicente Romero]

Hi Mandy,

The patch for nestmates [1] could be used as a reference. There a new 
method was added to class `com.sun.tools.javac.jvm.Target`, named: 
`hasNestmateAccess` which checks if a target is ready for nestmates or 
not. I think that you can follow a similar approach here.


Thanks,
Vicente

[1] http://hg.openjdk.java.net/jdk/jdk/rev/2f2af62dfac7

On 3/27/20 12:29 PM, Mandy Chung wrote:

Hi Jan,

Good point.  The javac change only applies to JDK 15 and later and the 
lambda proxy class is not a nestmate when running on JDK 14 or earlier.


I probably need the help from langtools team to fix this.  I'll give 
it a try.


Mandy

On 3/27/20 4:31 AM, Jan Lahoda wrote:

Hi Mandy,

Regarding the javac changes - should those be switched on/off 
depending the Target? Or, if one compiles with e.g. --release 14, 
will the newly generated output still work on JDK 14?


Jan

On 27. 03. 20 0:57, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The 
main changes are in core-libs and hotspot runtime area.  Small 
changes are made in javac, VM compiler (intrinsification of 
Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been reviewed 
and is in the finalized state (see specdiff and javadoc below for 
reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 



Hidden class is created via `Lookup::defineHiddenClass`. From JVM's 
point

of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not 
registered in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection. setAccessible(true) can 
still be called on reflected objects representing final fields in a 
hidden class and its access check will be suppressed but only have 
read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

    can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden 
class.
4. Field::setXXX method will throw IAE on a final field of a hidden 
class

    regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

    and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
    that holds the classes strongly referenced by its defining 
loader. There

    can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. Access 
control
    check no longer throws LinkageError but instead it will throw 
IAE with
    a clear message if a class fails to resolve/validate the nest 
host declared

    in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
    and generate a bridge method to desuger a method reference to a 
protected

    method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError 
and intends
to have the newly created class linked.  However, the implementation 
in 14

does not link the class.  A separate CSR [2] proposes to update the
implementation to match the spec.  This patch fixes the implementation.

The spec update on JVM TI, JDI and Instrumentation will be done as
a separate RFE [3].  This patch includes new tests for JVM TI and
java.instrument that validates how the existing APIs work for hidden 
classes.


javadoc/specdiff
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/ 



JVMS 5.4.4 change:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf 



CSR:
https://bugs.openjdk.java.net/browse/JDK-8238359

Thanks
Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8238359
[2] https://bugs.openjdk.java.net/browse/JDK-8240338
[3] https://bugs.openjdk.java.net/browse/JDK-8230502

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

Hi Paul,

This is the delta incorporating your comment:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-psandoz/

This patch also took Alex's comment to make it clear that the hidden 
class is the lookup class of the returned Lookup object and drops the 
sentence you commented on:


On 3/27/20 1:18 PM, Mandy Chung wrote:

MethodHandles.java
—

1786  * (Given the {@code Lookup} object returned this 
method, its lookup class
1787  * is a {@code Class} object for which {@link 
Class#getName()} returns a string

1788  * that is not a binary name.)

“
(The {@code Lookup} object returned from this method has a lookup 
class that is

a {@code Class} object whose {@link Class#getName()} returns a string
that is not a binary name.)
“



Mandy

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

Hi Vicente,

hasNestmateAccess is about VM supports static nestmates on JDK release 
>= 11.


However this is about javac --release 14 and the compiled classes may 
run on JDK 14 that lambda and string concat spin classes that are not 
nestmates. I have a patch with Jan's help:


http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/index.html

(you can apply the above patch on valhalla repo "nestmates" branch)

About testing, I wanted to run BridgeMethodsForLambdaTest and 
TestLambdaBytecode test with --release 14 but it turns out not 
straight-forward.  Any help would be appreciated.


thanks
Mandy

On 3/27/20 2:15 PM, Vicente Romero wrote:

Hi Mandy,

The patch for nestmates [1] could be used as a reference. There a new 
method was added to class `com.sun.tools.javac.jvm.Target`, named: 
`hasNestmateAccess` which checks if a target is ready for nestmates or 
not. I think that you can follow a similar approach here.


Thanks,
Vicente

[1] http://hg.openjdk.java.net/jdk/jdk/rev/2f2af62dfac7

On 3/27/20 12:29 PM, Mandy Chung wrote:

Hi Jan,

Good point.  The javac change only applies to JDK 15 and later and 
the lambda proxy class is not a nestmate when running on JDK 14 or 
earlier.


I probably need the help from langtools team to fix this.  I'll give 
it a try.


Mandy

On 3/27/20 4:31 AM, Jan Lahoda wrote:

Hi Mandy,

Regarding the javac changes - should those be switched on/off 
depending the Target? Or, if one compiles with e.g. --release 14, 
will the newly generated output still work on JDK 14?


Jan

On 27. 03. 20 0:57, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The 
main changes are in core-libs and hotspot runtime area.  Small 
changes are made in javac, VM compiler (intrinsification of 
Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been 
reviewed and is in the finalized state (see specdiff and javadoc 
below for reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 



Hidden class is created via `Lookup::defineHiddenClass`. From JVM's 
point

of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not 
registered in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection. setAccessible(true) can 
still be called on reflected objects representing final fields in a 
hidden class and its access check will be suppressed but only have 
read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

    can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden 
class.
4. Field::setXXX method will throw IAE on a final field of a hidden 
class

    regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

    and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
    that holds the classes strongly referenced by its defining 
loader. There

    can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. 
Access control
    check no longer throws LinkageError but instead it will throw 
IAE with
    a clear message if a class fails to resolve/validate the nest 
host declared

    in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
    and generate a bridge method to desuger a method reference to a 
protected

    method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError 
and intends
to have the newly created class linked.  However, the 
implementation in 14

does not link the class.  A separate CSR [2] proposes to update the
implementation to match the spec.  This patch fixes the 
implementation.


The spec update on JVM TI, JDI and Instrumentation will be done as
a separate RFE [3].  This patch includes new tests for JVM TI and
java.instrument that validates how the existing APIs work for 
hidden classes.


javadoc/specdiff
http://cr

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Vicente Romero]

Hi Mandy,

On 3/27/20 6:29 PM, Mandy Chung wrote:

Hi Vicente,

hasNestmateAccess is about VM supports static nestmates on JDK release 
>= 11.


I was not suggesting the use of `hasNestmateAccess` but to follow the 
same approach which is adding a new method at class `Target` to check if 
the new goodies were in the given target


However this is about javac --release 14 and the compiled classes may 
run on JDK 14 that lambda and string concat spin classes that are not 
nestmates. I have a patch with Jan's help:


http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/index.html


which is what the patch above is doing



(you can apply the above patch on valhalla repo "nestmates" branch)

About testing, I wanted to run BridgeMethodsForLambdaTest and 
TestLambdaBytecode test with --release 14 but it turns out not 
straight-forward.  Any help would be appreciated.


thanks
Mandy


Vicente


On 3/27/20 2:15 PM, Vicente Romero wrote:

Hi Mandy,

The patch for nestmates [1] could be used as a reference. There a new 
method was added to class `com.sun.tools.javac.jvm.Target`, named: 
`hasNestmateAccess` which checks if a target is ready for nestmates 
or not. I think that you can follow a similar approach here.


Thanks,
Vicente

[1] http://hg.openjdk.java.net/jdk/jdk/rev/2f2af62dfac7

On 3/27/20 12:29 PM, Mandy Chung wrote:

Hi Jan,

Good point.  The javac change only applies to JDK 15 and later and 
the lambda proxy class is not a nestmate when running on JDK 14 or 
earlier.


I probably need the help from langtools team to fix this. I'll give 
it a try.


Mandy

On 3/27/20 4:31 AM, Jan Lahoda wrote:

Hi Mandy,

Regarding the javac changes - should those be switched on/off 
depending the Target? Or, if one compiles with e.g. --release 14, 
will the newly generated output still work on JDK 14?


Jan

On 27. 03. 20 0:57, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The 
main changes are in core-libs and hotspot runtime area.  Small 
changes are made in javac, VM compiler (intrinsification of 
Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been 
reviewed and is in the finalized state (see specdiff and javadoc 
below for reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 



Hidden class is created via `Lookup::defineHiddenClass`. From 
JVM's point

of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not 
registered in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas 
`GetClassSignature` returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection. setAccessible(true) can 
still be called on reflected objects representing final fields in 
a hidden class and its access check will be suppressed but only 
have read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

    can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden 
class.
4. Field::setXXX method will throw IAE on a final field of a 
hidden class

    regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

    and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
    that holds the classes strongly referenced by its defining 
loader. There

    can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. 
Access control
    check no longer throws LinkageError but instead it will throw 
IAE with
    a clear message if a class fails to resolve/validate the nest 
host declared

    in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
    and generate a bridge method to desuger a method reference to 
a protected

    method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, 
and LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError 
and intends
to have the newly created class linked.  However, the 
implementation in 14

does not link the class.  A separate CSR [2] proposes to update the
implementation to match

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[David Holmes]

Hi Mandy,

On 28/03/2020 8:29 am, Mandy Chung wrote:

Hi Vicente,

hasNestmateAccess is about VM supports static nestmates on JDK release 
 >= 11.


However this is about javac --release 14 and the compiled classes may 
run on JDK 14 that lambda and string concat spin classes that are not 
nestmates. I have a patch with Jan's help:


http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/index.html 


+ /**
+  * The VM does not support access across nested classes 
(8010319).

+  * Were that ever to change, this should be removed.
+  */
+ boolean isPrivateInOtherClass() {

I'm not at all sure what this means - access across different nests? 
(I'm not even sure what that means.)


Thanks,
David
-



(you can apply the above patch on valhalla repo "nestmates" branch)

About testing, I wanted to run BridgeMethodsForLambdaTest and 
TestLambdaBytecode test with --release 14 but it turns out not 
straight-forward.  Any help would be appreciated.


thanks
Mandy

On 3/27/20 2:15 PM, Vicente Romero wrote:

Hi Mandy,

The patch for nestmates [1] could be used as a reference. There a new 
method was added to class `com.sun.tools.javac.jvm.Target`, named: 
`hasNestmateAccess` which checks if a target is ready for nestmates or 
not. I think that you can follow a similar approach here.


Thanks,
Vicente

[1] http://hg.openjdk.java.net/jdk/jdk/rev/2f2af62dfac7

On 3/27/20 12:29 PM, Mandy Chung wrote:

Hi Jan,

Good point.  The javac change only applies to JDK 15 and later and 
the lambda proxy class is not a nestmate when running on JDK 14 or 
earlier.


I probably need the help from langtools team to fix this.  I'll give 
it a try.


Mandy

On 3/27/20 4:31 AM, Jan Lahoda wrote:

Hi Mandy,

Regarding the javac changes - should those be switched on/off 
depending the Target? Or, if one compiles with e.g. --release 14, 
will the newly generated output still work on JDK 14?


Jan

On 27. 03. 20 0:57, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The 
main changes are in core-libs and hotspot runtime area.  Small 
changes are made in javac, VM compiler (intrinsification of 
Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been 
reviewed and is in the finalized state (see specdiff and javadoc 
below for reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03 



Hidden class is created via `Lookup::defineHiddenClass`. From JVM's 
point

of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not 
registered in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection. setAccessible(true) can 
still be called on reflected objects representing final fields in a 
hidden class and its access check will be suppressed but only have 
read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

    can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden 
class.
4. Field::setXXX method will throw IAE on a final field of a hidden 
class

    regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

    and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
    that holds the classes strongly referenced by its defining 
loader. There

    can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. 
Access control
    check no longer throws LinkageError but instead it will throw 
IAE with
    a clear message if a class fails to resolve/validate the nest 
host declared

    in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
    and generate a bridge method to desuger a method reference to a 
protected

    method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError 
and intends
to have the newly created class link

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Remi Forax]

- Mail original -
> De: "David Holmes" 
> À: "mandy chung" , "Vicente Romero" 
> , "jan lahoda"
> 
> Cc: "serviceability-dev" , "hotspot-dev" 
> ,
> "core-libs-dev" , "valhalla-dev" 
> 
> Envoyé: Samedi 28 Mars 2020 00:01:58
> Objet: Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

> Hi Mandy,

Hi David,

> 
> On 28/03/2020 8:29 am, Mandy Chung wrote:
>> Hi Vicente,
>> 
>> hasNestmateAccess is about VM supports static nestmates on JDK release
>>  >= 11.
>> 
>> However this is about javac --release 14 and the compiled classes may
>> run on JDK 14 that lambda and string concat spin classes that are not
>> nestmates. I have a patch with Jan's help:
>> 
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/index.html
> 
> + /**
> +  * The VM does not support access across nested classes
> (8010319).
> +  * Were that ever to change, this should be removed.
> +  */
> + boolean isPrivateInOtherClass() {
> 
> I'm not at all sure what this means - access across different nests?
> (I'm not even sure what that means.)

Access inside the same nest.
As you know, until now, a lambda proxy is a VM anonymous class that can only 
see the private fields of the class declaring the lambda (the host class) and 
not the private fields of a class of the nest (the enclosing classes in term of 
Java the language).

Rémi

> 
> Thanks,
> David
> -
> 
>> 
>> (you can apply the above patch on valhalla repo "nestmates" branch)
>> 
>> About testing, I wanted to run BridgeMethodsForLambdaTest and
>> TestLambdaBytecode test with --release 14 but it turns out not
>> straight-forward.  Any help would be appreciated.
>> 
>> thanks
>> Mandy
>> 
>> On 3/27/20 2:15 PM, Vicente Romero wrote:
>>> Hi Mandy,
>>>
>>> The patch for nestmates [1] could be used as a reference. There a new
>>> method was added to class `com.sun.tools.javac.jvm.Target`, named:
>>> `hasNestmateAccess` which checks if a target is ready for nestmates or
>>> not. I think that you can follow a similar approach here.
>>>
>>> Thanks,
>>> Vicente
>>>
>>> [1] http://hg.openjdk.java.net/jdk/jdk/rev/2f2af62dfac7
>>>
>>> On 3/27/20 12:29 PM, Mandy Chung wrote:
 Hi Jan,

 Good point.  The javac change only applies to JDK 15 and later and
 the lambda proxy class is not a nestmate when running on JDK 14 or
 earlier.

 I probably need the help from langtools team to fix this.  I'll give
 it a try.

 Mandy

 On 3/27/20 4:31 AM, Jan Lahoda wrote:
> Hi Mandy,
>
> Regarding the javac changes - should those be switched on/off
> depending the Target? Or, if one compiles with e.g. --release 14,
> will the newly generated output still work on JDK 14?
>
> Jan
>
> On 27. 03. 20 0:57, Mandy Chung wrote:
>> Please review the implementation of JEP 371: Hidden Classes. The
>> main changes are in core-libs and hotspot runtime area.  Small
>> changes are made in javac, VM compiler (intrinsification of
>> Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been
>> reviewed and is in the finalized state (see specdiff and javadoc
>> below for reference).
>>
>> Webrev:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03
>>
>>
>> Hidden class is created via `Lookup::defineHiddenClass`. From JVM's
>> point
>> of view, a hidden class is a normal class except the following:
>>
>> - A hidden class has no initiating class loader and is not
>> registered in any dictionary.
>> - A hidden class has a name containing an illegal character
>> `Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature`
>> returns "Lp/Foo.0x1234;".
>> - A hidden class is not modifiable, i.e. cannot be redefined or
>> retransformed. JVM TI IsModifableClass returns false on a hidden.
>> - Final fields in a hidden class is "final".  The value of final
>> fields cannot be overriden via reflection. setAccessible(true) can
>> still be called on reflected objects representing final fields in a
>> hidden class and its access check will be suppressed but only have
>> read-access (i.e. can do Field::getXXX but not setXXX).
>>
>> Brief summary of this patch:
>>
>> 1. A new Lookup::defineHiddenClass method is the API to create a
>> hidden class.
>> 2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG
>> option that
>>     can be specified when creating a hidden class.
>> 3. A new Class::isHiddenClass method tests if a class is a hidden
>> class.
>> 4. Field::setXXX method will throw IAE on a final field of a hidden
>> class
>>     regardless of the value of the accessible flag.
>> 5. JVM_LookupDefineClass is the new JVM entry point for
>> Lookup::defineClass
>>     and defineHiddenClass to create a class from the

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

On 3/27/20 4:01 PM, David Holmes wrote:

Hi Mandy,

On 28/03/2020 8:29 am, Mandy Chung wrote:

Hi Vicente,

hasNestmateAccess is about VM supports static nestmates on JDK 
release  >= 11.


However this is about javac --release 14 and the compiled classes may 
run on JDK 14 that lambda and string concat spin classes that are not 
nestmates. I have a patch with Jan's help:


http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/index.html 



+ /**
+  * The VM does not support access across nested classes 
(8010319).

+  * Were that ever to change, this should be removed.
+  */
+ boolean isPrivateInOtherClass() {

I'm not at all sure what this means - access across different nests? 
(I'm not even sure what that means.)


This just reverts  the old code that I removed.

What this method is trying to determine if it accesses a private in 
another class in the same nest (nested classes) that needs a synthetic 
bridge method to access.


Mandy

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[David Holmes]

Hi Mandy,

On 28/03/2020 9:46 am, Mandy Chung wrote:



On 3/27/20 4:01 PM, David Holmes wrote:

Hi Mandy,

On 28/03/2020 8:29 am, Mandy Chung wrote:

Hi Vicente,

hasNestmateAccess is about VM supports static nestmates on JDK 
release  >= 11.


However this is about javac --release 14 and the compiled classes may 
run on JDK 14 that lambda and string concat spin classes that are not 
nestmates. I have a patch with Jan's help:


http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/index.html 



+ /**
+  * The VM does not support access across nested classes 
(8010319).

+  * Were that ever to change, this should be removed.
+  */
+ boolean isPrivateInOtherClass() {

I'm not at all sure what this means - access across different nests? 
(I'm not even sure what that means.)


This just reverts  the old code that I removed.


Ah I see. This is ancient pre-nestmate code. Can we at least fix the 
comment as it really doesn't make any sense


What this method is trying to determine if it accesses a private in 
another class in the same nest (nested classes) that needs a synthetic 
bridge method to access.


That would be a good comment to add. Something like:

If compiling for a release where the VM does not support access between
nested classes, this method indicates if a synthetic bridge method is
needed for access.

Thanks,
David


Mandy

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Paul Sandoz]

+1
Paul.

> On Mar 27, 2020, at 3:22 PM, Mandy Chung  wrote:
> 
> Hi Paul,
> 
> This is the delta incorporating your comment:
>   
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-psandoz/
>  
> 
> 
> This patch also took Alex's comment to make it clear that the hidden class is 
> the lookup class of the returned Lookup object and drops the sentence you 
> commented on:
> 
> On 3/27/20 1:18 PM, Mandy Chung wrote:
>>> MethodHandles.java 
>>> — 
>>> 
>>> 1786  * (Given the {@code Lookup} object returned this method, its 
>>> lookup class 
>>> 1787  * is a {@code Class} object for which {@link Class#getName()} 
>>> returns a string 
>>> 1788  * that is not a binary name.) 
>>> 
>>> “ 
>>> (The {@code Lookup} object returned from this method has a lookup class 
>>> that is 
>>> a {@code Class} object whose {@link Class#getName()} returns a string 
>>> that is not a binary name.) 
>>> “ 
> 
> 
> Mandy

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Dean Long]

I looked at the AOT, C2, and JVMCI changes and I didn't find any issues.

dl

On 3/26/20 4:57 PM, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The main 
changes are in core-libs and hotspot runtime area. Small changes are 
made in javac, VM compiler (intrinsification of Class::isHiddenClass), 
JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized 
state (see specdiff and javadoc below for reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03

Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point
of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not registered 
in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection.  setAccessible(true) can 
still be called on reflected objects representing final fields in a 
hidden class and its access check will be suppressed but only have 
read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

   can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden class.
4. Field::setXXX method will throw IAE on a final field of a hidden class
   regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

   and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
   that holds the classes strongly referenced by its defining loader.  
There

   can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. Access 
control

   check no longer throws LinkageError but instead it will throw IAE with
   a clear message if a class fails to resolve/validate the nest host 
declared

   in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
   and generate a bridge method to desuger a method reference to a 
protected

   method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError and 
intends

to have the newly created class linked.  However, the implementation in 14
does not link the class.  A separate CSR [2] proposes to update the
implementation to match the spec.  This patch fixes the implementation.

The spec update on JVM TI, JDI and Instrumentation will be done as
a separate RFE [3].  This patch includes new tests for JVM TI and
java.instrument that validates how the existing APIs work for hidden 
classes.


javadoc/specdiff
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/

JVMS 5.4.4 change:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf

CSR:
https://bugs.openjdk.java.net/browse/JDK-8238359

Thanks
Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8238359
[2] https://bugs.openjdk.java.net/browse/JDK-8240338
[3] https://bugs.openjdk.java.net/browse/JDK-8230502

Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Chris Plummer]

Hi Mandy,

A couple of very minor nits in the jvmtiRedefineClasses.cpp comments:

 153 // classes for primitives, arrays, hidden and vm unsafe 
anonymous classes
 154 // cannot be redefined.  Check here so following code can 
assume these classes

 155 // are InstanceKlass.
 156 if (!is_modifiable_class(mirror)) {
 157   _res = JVMTI_ERROR_UNMODIFIABLE_CLASS;
 158   return false;
 159 }

I think this code and comment predate anonymous classes. Probably before 
anonymous classes the check was not for !is_modifiable_class() but 
instead was just a check for primitive or array class types since they 
are not an InstanceKlass, and would cause issues when cast to one in the 
code that lies below this section. When anonymous classes were added, 
the code got changed to use !is_modifiable_class() and the comment was 
not correctly updated (anonymous classes are an InstanceKlass). Then 
with this webrev the mention of hidden classes was added, also 
incorrectly implying they are not an InstanceKlass. I think you should 
just leave off the last sentence of the comment.


There's some ambiguity in the application of adjectives in the following:

 297   // Cannot redefine or retransform a hidden or an unsafe 
anonymous class.


I'd suggest:

 297   // Cannot redefine or retransform a hidden class or an unsafe 
anonymous class.


There are some places in libjdwp that need to be fixed. I spoke to 
Serguei about those this afternoon. Basically the 
convertSignatureToClassname() function needs to be fixed to handle 
hidden classes. Without the fix classname filtering will have problems 
if the filter contains a pattern with a '/' to filter on hidden classes. 
Also CLASS_UNLOAD events will not properly convert hidden class names. 
We also need tests for these cases. I think these are all things that 
can be addressed later.


I still need to look over the JVMTI tests.

thanks,

Chris

On 3/26/20 4:57 PM, Mandy Chung wrote:
Please review the implementation of JEP 371: Hidden Classes. The main 
changes are in core-libs and hotspot runtime area. Small changes are 
made in javac, VM compiler (intrinsification of Class::isHiddenClass), 
JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized 
state (see specdiff and javadoc below for reference).


Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03

Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point
of view, a hidden class is a normal class except the following:

- A hidden class has no initiating class loader and is not registered 
in any dictionary.
- A hidden class has a name containing an illegal character 
`Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` 
returns "Lp/Foo.0x1234;".
- A hidden class is not modifiable, i.e. cannot be redefined or 
retransformed. JVM TI IsModifableClass returns false on a hidden.
- Final fields in a hidden class is "final".  The value of final 
fields cannot be overriden via reflection.  setAccessible(true) can 
still be called on reflected objects representing final fields in a 
hidden class and its access check will be suppressed but only have 
read-access (i.e. can do Field::getXXX but not setXXX).


Brief summary of this patch:

1. A new Lookup::defineHiddenClass method is the API to create a 
hidden class.
2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG 
option that

   can be specified when creating a hidden class.
3. A new Class::isHiddenClass method tests if a class is a hidden class.
4. Field::setXXX method will throw IAE on a final field of a hidden class
   regardless of the value of the accessible flag.
5. JVM_LookupDefineClass is the new JVM entry point for 
Lookup::defineClass

   and defineHiddenClass to create a class from the given bytes.
6. ClassLoaderData implementation is not changed.  There is one 
primary CLD
   that holds the classes strongly referenced by its defining loader.  
There

   can be zero or more additional CLDs - one per weak class.
7. Nest host determination is updated per revised JVMS 5.4.4. Access 
control

   check no longer throws LinkageError but instead it will throw IAE with
   a clear message if a class fails to resolve/validate the nest host 
declared

   in NestHost/NestMembers attribute.
8. JFR, jcmd, JDI are updated to support hidden classes.
9. update javac LambdaToMethod as lambda proxy starts using nestmates
   and generate a bridge method to desuger a method reference to a 
protected

   method in its supertype in a different package

This patch also updates StringConcatFactory, LambdaMetaFactory, and 
LambdaForms
to use hidden classes.  The webrev includes changes in nashorn to 
hidden class

and I will update the webrev if JEP 372 removes it any time soon.

We uncovered a bug in Lookup::defineClass spec throws LinkageError and 
intends

to have the newly created class linked.  However, the implementation in 14
does not link the class.  A separate CSR [2]