Hi Mandy,
On 28/05/2020 2:13 pm, Mandy Chung wrote:
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8245432/webrev.01/
I modify this patch to check the class file version and throws CFE if
unsupported before creating ClassReader. This also fixes JDK-8245061
that it reads the value of `this_class` as a constant (as ASM will throw
an exception if it's not a constant) and also ensures that `this_class`
is a CONSTANT_Class_info by checking the descriptor string from Type.
I couldn't quite follow all the details so just a couple of comments:
src/java.base/share/classes/jdk/internal/misc/VM.java
+ public static boolean isSupportedClassFileVersion(int major, int
minor) {
+ if (major < 45 || major > classFileMajorVersion) return false;
+ if (major < 56) return minor == 0;
+ return minor == 0 || minor == PREVIEW_MINOR_VERSION;
+ }
that is only a partial validity test for preview versions - the major
version must match the current version as well.
+ s = props.get("java.class.version");
+ int index = s.indexOf('.');
+ try {
+ classFileMajorVersion = Integer.valueOf(s.substring(0,
index));
+ classFileMinorVersion =
Integer.valueOf(s.substring(index+1, s.length()));
+ } catch (NumberFormatException e) {
+ throw new InternalError(e);
+ }
Can you not access java.lang.VersionProps to get the version info?
Cheers,
David
Mandy
On 5/27/20 10:57 AM, Mandy Chung wrote:
I'm reconsidering this fix along with JDK-8245061 that may require to
do its own checking (a similar issue w.r.t. ASM validation but in this
case the constant pool entry of `this_class` item is not validated).
thanks
Mandy
On 5/27/20 10:39 AM, fo...@univ-mlv.fr wrote:
Hi Alan,
We (the ASM team) recommend to our users to check the byte 6 (and
perhaps 7) instead of relying on ASM throwing an exception,
because you may update the version of ASM but not the visitors your
are using in your code.
It's less brittle than catching the IAE thrown by ASM.
Rémi
----- Mail original -----
De: "Alan Bateman" <alan.bate...@oracle.com>
À: "mandy chung" <mandy.ch...@oracle.com>, "core-libs-dev"
<core-libs-dev@openjdk.java.net>, "Remi Forax"
<fo...@univ-mlv.fr>
Envoyé: Mercredi 27 Mai 2020 18:16:33
Objet: Re: RFR: JDK-8245432: Lookup::defineHiddenClass should throw
UnsupportedClassVersionError if the given bytes are
of an unsupported major or minor version
On 26/05/2020 22:46, Mandy Chung wrote:
Lookup::defineHiddenClass currently throws IAE by ASM if the given
bytes are of unsupported class file version. The implementation
should catch and throw UnsupportedClassVersionError instead.
webrev:
http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8245432/webrev.00/
This patch also includes a spec clarification of @throws IAE if the
the bytes has ACC_MODULE flag set to fix JDK-8245596.
Rémi - has there ever been any discussion in ASM about throwing more
specific exceptions? Only asking to see if we could avoid needing to
depend on the exception message here.
-Alan
