RFR 8144931: Assert class signatures are correct and refer to valid classes
Hi All, Please review fix for the following bug- https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/ Thanks, Shilpi
Shilpi, _CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG Otherwise, looks fine. Best regards, Vladimir Ivanov On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
Thank You Vladimir! I have done the changes. Please review the updated patch- http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/ Regards, Shilpi On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
Reviewed. Best regards, Vladimir Ivanov On 2/18/16 2:18 PM, shilpi.rastogi@oracle.com wrote:
Thank You Vladimir!
I have done the changes. Please review the updated patch-
http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
Regards, Shilpi
On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
On 18 Feb 2016, at 12:18, shilpi.rastogi@oracle.com wrote:
Thank You Vladimir!
I have done the changes. Please review the updated patch-
Looking good. There are also a few other cases in InvokerBytecodeGenerator you can update: 679 mv.visitAnnotation("Ljava/lang/invoke/InjectedProfile;", true); 1328 // Suppress this method in backtraces displayed to the user. 1329 mv.visitAnnotation("Ljava/lang/invoke/LambdaForm$Hidden;", true); 1330 1331 // Don't inline the interpreter entry. 1332 mv.visitAnnotation("Ljdk/internal/vm/annotation/DontInline;", true); 1387 // Suppress this method in backtraces displayed to the user. 1388 mv.visitAnnotation("Ljava/lang/invoke/LambdaForm$Hidden;", true); 1389 1390 // Force inlining of this invoker method. 1391 mv.visitAnnotation("Ljdk/internal/vm/annotation/ForceInline;", true); 1392 That might be all the rest but i have not double checked. As a follow on task can you check other classes in j.l.i where this technique might be applied? if so we can log another issue for that e.g search using the regex \"L[\w\/]+;\”. Paul.
Regards, Shilpi
On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
Hi, just a stupid question from somebody outside the OpenJDK developers: You are already using ASM to generate the class files. Why not also use the Type class in ASM to generate the signatures of a class constant?: Instead of: static final String LF_HIDDEN_SIG = className("Ljava/lang/invoke/LambdaForm$Hidden;"); Use the following to define the constant: import jdk.internal.org.objectweb.asm.Type; import java.lang.invoke.LambdaForm.Hidden; static final String LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class); This is compile-time checked, because of the .class notation. Thanks, Uwe ----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of shilpi.rastogi@oracle.com Sent: Thursday, February 18, 2016 12:18 PM To: Vladimir Ivanov <vladimir.x.ivanov@oracle.com>; core-libs- dev@openjdk.java.net Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid classes
Thank You Vladimir!
I have done the changes. Please review the updated patch-
http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
Regards, Shilpi
On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
Hi Uwe Not a stupid question. We took the conservative approach to preserve the existing costs (avoid linkage and string generation). Paul.
On 18 Feb 2016, at 14:54, Uwe Schindler <uschindler@apache.org> wrote:
Hi,
just a stupid question from somebody outside the OpenJDK developers: You are already using ASM to generate the class files. Why not also use the Type class in ASM to generate the signatures of a class constant?:
Instead of:
static final String LF_HIDDEN_SIG = className("Ljava/lang/invoke/LambdaForm$Hidden;");
Use the following to define the constant:
import jdk.internal.org.objectweb.asm.Type; import java.lang.invoke.LambdaForm.Hidden; static final String LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);
This is compile-time checked, because of the .class notation.
Thanks, Uwe
----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of shilpi.rastogi@oracle.com Sent: Thursday, February 18, 2016 12:18 PM To: Vladimir Ivanov <vladimir.x.ivanov@oracle.com>; core-libs- dev@openjdk.java.net Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid classes
Thank You Vladimir!
I have done the changes. Please review the updated patch-
http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
Regards, Shilpi
On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
Hi, Thanks! I was expecting something like this. You just want to check the class name in an assertion - that’s fine. Kind regards, Uwe P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise broken descriptor strings (missing "L" or missing ";"): static boolean checkClassName(String cn) { Type tp = Type.getType(cn); // additional sanity so only valid "L;" descriptors work if (tp.getSort() != Type.OBJECT) { return false; } try { Class<?> c = Class.forName(tp.getClassName(), false, null); return true; } catch (ClassNotFoundException e) { return false; } } ----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of Paul Sandoz Sent: Thursday, February 18, 2016 3:05 PM Cc: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid classes
Hi Uwe
Not a stupid question.
We took the conservative approach to preserve the existing costs (avoid linkage and string generation).
Paul.
On 18 Feb 2016, at 14:54, Uwe Schindler <uschindler@apache.org> wrote:
Hi,
just a stupid question from somebody outside the OpenJDK developers: You are already using ASM to generate the class files. Why not also use the Type class in ASM to generate the signatures of a class constant?:
Instead of:
static final String LF_HIDDEN_SIG = className("Ljava/lang/invoke/LambdaForm$Hidden;");
Use the following to define the constant:
import jdk.internal.org.objectweb.asm.Type; import java.lang.invoke.LambdaForm.Hidden; static final String LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);
This is compile-time checked, because of the .class notation.
Thanks, Uwe
----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of shilpi.rastogi@oracle.com Sent: Thursday, February 18, 2016 12:18 PM To: Vladimir Ivanov <vladimir.x.ivanov@oracle.com>; core-libs- dev@openjdk.java.net Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid classes
Thank You Vladimir!
I have done the changes. Please review the updated patch-
http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
Regards, Shilpi
On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise broken descriptor strings (missing "L" or missing ";"): I like how it shapes out with Type. Thanks, Uwe!
Best regards, Vladimir Ivanov
static boolean checkClassName(String cn) { Type tp = Type.getType(cn); // additional sanity so only valid "L;" descriptors work if (tp.getSort() != Type.OBJECT) { return false; } try { Class<?> c = Class.forName(tp.getClassName(), false, null); return true; } catch (ClassNotFoundException e) { return false; } }
----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of Paul Sandoz Sent: Thursday, February 18, 2016 3:05 PM Cc: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid classes
Hi Uwe
Not a stupid question.
We took the conservative approach to preserve the existing costs (avoid linkage and string generation).
Paul.
On 18 Feb 2016, at 14:54, Uwe Schindler <uschindler@apache.org> wrote:
Hi,
just a stupid question from somebody outside the OpenJDK developers: You are already using ASM to generate the class files. Why not also use the Type class in ASM to generate the signatures of a class constant?:
Instead of:
static final String LF_HIDDEN_SIG = className("Ljava/lang/invoke/LambdaForm$Hidden;");
Use the following to define the constant:
import jdk.internal.org.objectweb.asm.Type; import java.lang.invoke.LambdaForm.Hidden; static final String LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);
This is compile-time checked, because of the .class notation.
Thanks, Uwe
----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of shilpi.rastogi@oracle.com Sent: Thursday, February 18, 2016 12:18 PM To: Vladimir Ivanov <vladimir.x.ivanov@oracle.com>; core-libs- dev@openjdk.java.net Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid classes
Thank You Vladimir!
I have done the changes. Please review the updated patch-
http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
Regards, Shilpi
On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
Shilpi,
_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF: LF_HIDDEN_SIG LF_COMPILED_SIG FORCEINLINE_SIG DONTINLINE_SIG
Otherwise, looks fine.
Best regards, Vladimir Ivanov
On 2/17/16 5:47 PM, shilpi rastogi wrote:
Hi All,
Please review fix for the following bug-
https://bugs.openjdk.java.net/browse/JDK-8144931 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
Thanks, Shilpi
On 18 Feb 2016, at 15:55, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise broken descriptor strings (missing "L" or missing ";"): I like how it shapes out with Type. Thanks, Uwe!
Me too! Paul.
Best regards, Vladimir Ivanov
static boolean checkClassName(String cn) { Type tp = Type.getType(cn); // additional sanity so only valid "L;" descriptors work if (tp.getSort() != Type.OBJECT) { return false; } try { Class<?> c = Class.forName(tp.getClassName(), false, null); return true; } catch (ClassNotFoundException e) { return false; } }
----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
Hi All, Please see the updated webrev http://cr.openjdk.java.net/~srastogi/8144931/webrev.03/ Regards, Shilpi On 2/18/2016 8:33 PM, Paul Sandoz wrote:
On 18 Feb 2016, at 15:55, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise broken descriptor strings (missing "L" or missing ";"): I like how it shapes out with Type. Thanks, Uwe!
Me too!
Paul.
Best regards, Vladimir Ivanov
static boolean checkClassName(String cn) { Type tp = Type.getType(cn); // additional sanity so only valid "L;" descriptors work if (tp.getSort() != Type.OBJECT) { return false; } try { Class<?> c = Class.forName(tp.getClassName(), false, null); return true; } catch (ClassNotFoundException e) { return false; } }
----- Uwe Schindler uschindler@apache.org ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/
Looks good. Best regards, Vladimir Ivanov On 2/19/16 8:32 AM, shilpi.rastogi@oracle.com wrote:
Please see the updated webrev http://cr.openjdk.java.net/~srastogi/8144931/webrev.03/
On 19 Feb 2016, at 06:32, shilpi.rastogi@oracle.com wrote:
Hi All,
Please see the updated webrev http://cr.openjdk.java.net/~srastogi/8144931/webrev.03/
+1 Paul.
All, I'll sponsor this. Best, Michael
Am 19.02.2016 um 12:37 schrieb Paul Sandoz <paul.sandoz@oracle.com>:
On 19 Feb 2016, at 06:32, shilpi.rastogi@oracle.com wrote:
Hi All,
Please see the updated webrev http://cr.openjdk.java.net/~srastogi/8144931/webrev.03/
+1
Paul.
-- <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
participants (6)
-
Michael Haupt
-
Paul Sandoz
-
shilpi rastogi
-
shilpi.rastogi＠oracle.com
-
Uwe Schindler
-
Vladimir Ivanov