----- Mail original -----
> De: "daniel smith" <daniel.sm...@oracle.com>
> À: "valhalla-spec-experts" <valhalla-spec-experts@openjdk.java.net>
> Envoyé: Vendredi 4 Juin 2021 19:11:11
> Objet: Re: JEP draft: Better-defined JVM class file validation

>> On Jun 4, 2021, at 10:41 AM, Dan Smith <daniel.sm...@oracle.com> wrote:
>> 
>> Posted a new JEP draft, here:
>> 
>> http://openjdk.java.net/jeps/8267650

Hi Dan,
that's a lot of nice cleanups.

If i can emit a critic, i would have preferred to have the text of the JEP to 
clearly separate spec issues from Hotspot issues,
to the point where i wonder if it's not better to have two JEPs.

> 
> I'll draw your attention to this section, in case it raises any red flags:
> 
> ---
> 
> Risks and Assumptions
> 
> Changing JVM validation behavior is often a risk, because it may cause legacy
> classfiles to fail with new errors, or, more subtly, new class files with old
> version numbers to be accepted, but then fail on older JVMs.
> 
> In general, the HotSpot changes proposed in this JEP are narrow in scope, 
> often
> in corner cases that real world code is unlikely to probe. And many of the
> changes only modify the type of error being thrown or the timing of an error
> check. That said, the most likely areas of concern are:
> 
>       • New errors caused by improper appearances of the Module, 
> ModulePackages,
>       ModuleMainClass, and ConstantValue attributes.

yes, those attributes should be checked by the VM an not only in Java code.

> 
>       • New errors caused by pre-51 class files that declare a useless method 
> with
>       name <clinit> and 1 or more parameters.

I don't see the point of this change, unlike all the others, this one will 
reject classes that were valid before.

> 
>       • Accepting class files with malformed optional attributes, even though 
> those
>       class files could fail to load on an older JVM.

It's not clear to me if this change is only for classfile version >= V18 or not.
I would vote for having that change applied on all classes given it's an 
Hotspot bug, not a spec bug.

> 
> Besides the risk to JVM users, there is some risk that, by relaxing the
> constraints on optional attributes, downstream tools will be surprised by
> unvalidated attribute contents in class files that can be successfully loaded.

Actually, the fact that the InnerClasses attribute is validated by the VM is 
confusing because it implies that the VM uses the values of that attributes to 
check access which it does not.
For me, the spec says that those attributes are optionals so Hotspot should 
behave as the spec says.

And i'm a little worry about rejecting the ConstantValue attribute if the field 
is not static because i think i've seen a code using ConstantValue on all 
fields but i'm not able to recall where so i've not idea if it was in library 
used or not. I suppose this behavior will be gated with a V18 checks so it 
should not be an issue anyway.

> 
> These risks need to be balanced against the cost of the extra complexity
> required to fully specify and maintain longstanding, often ad hoc HotSpot
> behavior.
> 
> ---

regards,
Rémi

Reply via email to