[DKIM violation]Re: RFR: JDK-8230413: Support Pre JDK 6 class with CDS

Wed Jan 15 01:46:25 UTC 2020

A follow-up on this topic:

The survey for supporting archiving pre-JDK 6 classes received one
response, https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-November/037072.html,
which indicated a general pre-JDK 6 support (with verification enabled
as well) might be needed and beneficial in some use cases.

Based on the above, the proper next step is to withdraw my original
code review request for supporting archiving pre-JDK 6 classes with
-Xshare:off only. Harold will take over from here and investigate a
complete solution (including the support with older verifier), and
prioritize it appropriately along with the other items. Thanks!

Best regards,

Jiangli

---------- Forwarded message ---------
From: Jiangli Zhou <jianglizhou at google.com>
Date: Fri, Dec 6, 2019 at 2:35 PM
Subject: Re: [***DKIM violation***]Re: RFR: JDK-8230413: Support Pre
JDK 6 class with CDS
To: hotspot-runtime-dev <hotspot-runtime-dev at openjdk.java.net>

I'd like to check if there is any additional feedback on supporting
older class versions (<50) with CDS. Will give it some more time then
collect all feedback, and get to to Harold on what's the next step.
Thanks!

Best regards,
Jiangli

On Mon, Nov 18, 2019 at 7:40 PM Jiangli Zhou <jianglizhou at google.com> wrote:
>
> Hi Plizga,
>
> Thanks for the feedback! Inputs like yours are most helpful for deciding what is the right approach. Really appreciate it!
>
> Best,
> Jiangli
>
> On Mon, Nov 18, 2019, 7:21 PM Plizga Vladimir <v.plizga at cft.ru> wrote:
>>
>> Hi Jiangli,
>>
>> Thank you for proposal of this feature!
>>
>> I've tried to use AppCDS for a web application (microservice) based on Spring Boot framework. No ''-Xverify:none" option is used neither at dump time nor at run time.
>> There are ~10 000 classes in application at runtime (simply counted with 'grep' of JVM class loading log). During archive creation ~750 classes are skipped with warnings like this:
>> > Pre JDK 6 class not supported by CDS: 49.0 org/jrobin/core/RrdUpdater
>>
>> All the skipped classes are from 3rd party libraries. These libraries are usually involved as transitive dependencies of other libraries and frameworks. AFAIU, there are 2 reasons why the libraries have their byte code of so old version:
>> 1. The libraries that use them didn’t upgrade their dependencies for some reasons.
>> 2. They keep their byte code old intentionally to stay compatible with most runtime environments (for example slf4j logging façade).
>>
>> Of course this is not very significant (and for not the largest) part of all the skipped classes in this case. But we have other similar applications with a bigger number of old transitive dependencies (about 10%).
>> Hope this would help.
>>
>> Cheers,
>> Vladimir Plizga
>>
>>
>> -----Original Message-----
>> From: hotspot-runtime-dev <hotspot-runtime-dev-bounces at openjdk.java.net> On Behalf Of Jiangli Zhou
>> Sent: Tuesday, November 19, 2019 2:01 AM
>> To: hotspot-runtime-dev <hotspot-runtime-dev at openjdk.java.net>
>> Subject: [***DKIM violation***]Re: RFR: JDK-8230413: Support Pre JDK 6 class with CDS
>>
>> After off-mailing list discussions regarding the support for archiving
>> pre-JDK-6 classes, we would like to seek additional input and feedback on this topic. Please reply to this email thread if the general support for archiving pre-JDK-6 class is a requirement in your use case, both with verification enabled and disabled. Thanks!
>>
>> Best,
>>
>> Jiangli
>>
>> On Wed, Nov 13, 2019 at 9:23 AM Jiangli Zhou <jianglizhou at google.com> wrote:
>> >
>> > Hi David,
>> >
>> > On Tue, Nov 12, 2019 at 10:00 PM David Holmes <david.holmes at oracle.com> wrote:
>> > >
>> > > Hi Jiangli,
>> > >
>> > > On 13/11/2019 12:20 pm, Jiangli Zhou wrote:
>> > > > Hi Harold and Ioi,
>> > > >
>> > > > Thanks a lot for the additional feedback.
>> > > >
>> > > > I did some quick research today about -Xverify:none usages. My
>> > > > finding showed that the use of -Xverify:none is not very uncommon
>> > > > in some cases. Here are some of the usages:
>> > > >
>> > > > - trusted tools
>> > >
>> > > But what is the context? Is it:
>> > >
>> > > "I trust this tool, and all other classes, so I'll optimize by
>> > > disabling verification,"; or
>> >
>> > This is the case. For a tool that's developed by a user and properly
>> > compiled by javac, user may want to disable class verification when
>> > running the tool.
>> >
>> > >
>> > > "This tool produces non-verifiable classfiles, but I trust the tool
>> > > and so will disable verification" (which implicitly means all
>> > > classes/libraries have to be fully trusted)
>> > >
>> > > ?
>> > >
>> > > I'm not sure you can use any existing uses of -Xverify:none to infer
>> > > the applicability or not to what is being proposed here for CDS.
>> >
>> > In above example, CDS dump time forces verification for the tool's
>> > classes as long as they are placed in -cp path. Without CDS involved,
>> > users choice is honored. I feel this usage may be a lurking issue when
>> > more users start to use CDS/AppCDS.
>> >
>> > Harold, Ioi and I have a discussion for pre-jdk-6 verification off the
>> > mailing list, since verification is security related and may be
>> > sensitive. I'll loop you in. It's possible we may be able to separate
>> > the pre-jdk-6 class problem from the general CDS -Xverify:none topics.
>> >
>> > >
>> > > > - some limited testing environment
>> > > >
>> > > > CDS (particularly with dynamic archiving capability) may help
>> > > > avoid runtime verification overhead by verifying classes at dump
>> > > > time and reduce the needs for -Xverify:none. It would be good to
>> > > > have strategies for the following senators as well when removing
>> > > > -Xverify:none:
>> > > >
>> > > > 1) In cases when shared archive is disabled at runtime (I hope
>> > > > it's not common cases)
>> > >
>> > > I'm not quite sure what you are saying here. If a pre-verified
>> > > archive can't be used at runtime then normal verification should
>> > > occur as classes are not being loaded from a known pre-verified location.
>> >
>> > CDS/AppCDS are still not widely adopted yet. When users start to learn
>> > more about CDS/AppCDS capability, they may still choose to not use the
>> > feature based on their specific requirements. For example, a user may
>> > choose to not use AppCDS and also turn off the default CDS.
>> >
>> > >
>> > > > 2) When users want to reduce the overhead caused by verification
>> > > > during archiving dump time
>> > >
>> > > I would not expect dumping to be such a time critical activity that
>> > > users would care about the "overhead" of verification.
>> >
>> > With dynamic archiving, dump time performance can be more important to users.
>> >
>> > Best,
>> >
>> > Jiangli
>> >
>> > >
>> > > Cheers,
>> > > David
>> > >
>> > > > Thoughts?
>> > > >
>> > > > Best,
>> > > > Jiangli
>> > > >
>> > > > On Tue, Nov 12, 2019 at 4:16 PM Ioi Lam <ioi.lam at oracle.com> wrote:
>> > > >>
>> > > >> I am also a little worried that this might send the wrong message
>> > > >> -- "if you want to archive pre-JDK6 classes, you need to disable
>> > > >> verification altogether for all classes in your entire app".
>> > > >>
>> > > >> Thanks
>> > > >> - Ioi
>> > > >>
>> > > >> On 11/12/19 12:40 PM, Harold Seigel wrote:
>> > > >>> Hi Jiangli,
>> > > >>>
>> > > >>> I think this change is going in the wrong direction.  We are
>> > > >>> trying to discourage disabling verification, not encourage it.
>> > > >>> We also do not want to create more use-cases for preserving -Xverify:none.
>> > > >>>
>> > > >>> It looks like your change would allow archiving of unverified
>> > > >>> pre-JDK6 classes, but not allow archiving of verified pre-JDK6
>> > > >>> classes.  If so, that seems backward.
>> > > >>>
>> > > >>> Thanks, Harold
>> > > >>>
>> > > >>> On 11/11/2019 11:53 PM, Ioi Lam wrote:
>> > > >>>> I wonder if there's a safer alternative. Are there tools that
>> > > >>>> can add stackmaps to pre-JDK6 classes? That way they can be
>> > > >>>> verified with the split verifier during CDS dump time.
>> > > >>>>
>> > > >>>> Thanks
>> > > >>>> - Ioi
>> > > >>>>
>> > > >>>> On 11/11/19 4:25 PM, Jiangli Zhou wrote:
>> > > >>>>> Hi David,
>> > > >>>>>
>> > > >>>>> Thanks for quick response!
>> > > >>>>>
>> > > >>>>> On Mon, Nov 11, 2019 at 3:12 PM David Holmes
>> > > >>>>> <david.holmes at oracle.com> wrote:
>> > > >>>>>> Hi Jiangli,
>> > > >>>>>>
>> > > >>>>>> On 12/11/2019 8:12 am, Jiangli Zhou wrote:
>> > > >>>>>>> Please review the following change that allows archiving
>> > > >>>>>>> pre-JAVA_6_VERSION classes with -Xverify:none.
>> > > >>>>>>>
>> > > >>>>>>> webrev:
>> > > >>>>>>> http://cr.openjdk.java.net/~jiangli/8230413/webrev.00/
>> > > >>>>>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8230413
>> > > >>>>>>>
>> > > >>>>>>> Currently there are still large number of existing classes
>> > > >>>>>>> (pre-built)
>> > > >>>>>>> with older class versions (< 50) in real world applications.
>> > > >>>>>>> Those classes are missing the benefit of archiving.
>> > > >>>>>>> Particularly, in some use cases, class verification can be
>> > > >>>>>>> safely disabled. For those use cases, supporting archiving
>> > > >>>>>>> pre JDK 6 classes shows good performance benefit. We can
>> > > >>>>>>> re-evaluate this support when -Xverify:none is removed in
>> > > >>>>>>> the future, hopefully the needs for supporting class version
>> > > >>>>>>> < 50 is no longer significant at that time.
>> > > >>>>>>>
>> > > >>>>>>> This change brings back the pre-JDK-8198849 behavior.
>> > > >>>>>>> Runtime makes sure the dump-time verification mode must be
>> > > >>>>>>> the same or stronger than the current mode.
>> > > >>>>>>>
>> > > >>>>>>> A CSR may be needed for the change. Any thoughts on that?
>> > > >>>>>> A CSR request is definitely required given that you are
>> > > >>>>>> proposing to undo a change that was itself put in place via a
>> > > >>>>>> CSR request! And given this is relaxing a "defense-in-depth"
>> > > >>>>>> check which will result in increasing exploitability, I think
>> > > >>>>>> you will need a very strong argument to justify this.
>> > > >>>>> Thanks for confirming this! Will do.
>> > > >>>>>
>> > > >>>>>> Further this not only undoes JDK-8197972 but it also
>> > > >>>>>> invalidates
>> > > >>>>>> JDK-8155671 being closed as a duplicate of JDK-8197972.
>> > > >>>>>> JDK-8155671 requested a way to know if verification had been
>> > > >>>>>> disabled, to help with analyzing crash reports, but instead
>> > > >>>>>> we decided to not allow verification to be disabled.
>> > > >>>>> I had some concerns about JDK-8155671 initially before making
>> > > >>>>> the change, as it's a closed bug and my memory about the
>> > > >>>>> specific issue was flushed out. I brought up the question in
>> > > >>>>> the bug. My take on Ioi's response to my query about
>> > > >>>>> JDK-8155671 was that the
>> > > >>>>> pre-JDK-8197972 behavior would not cause any security hole.
>> > > >>>>>
>> > > >>>>> Re-evaluating this particular behavior, I think the
>> > > >>>>> pre-JDK-8155671 would actually matches user intention better.
>> > > >>>>> If user decides to turn off verification in safe use cases, it
>> > > >>>>> seems to be a good idea to honor that. With the new dynamic
>> > > >>>>> archiving capability, archive could be created at the first time when running a particular application.
>> > > >>>>> Not forcing verification when user decides to can avoid
>> > > >>>>> unnecessary/unwanted overhead.
>> > > >>>>>
>> > > >>>>> If verification is turned off at dump time for application
>> > > >>>>> classes, runtime does not allow execution without also turning
>> > > >>>>> off verification. We can determine a crash is not caused by
>> > > >>>>> relaxed dump time verification.
>> > > >>>>>
>> > > >>>>> Regards,
>> > > >>>>> Jiangli
>> > > >>>>>
>> > > >>>>>> David
>> > > >>>>>> -----
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>>> Tested with jtreg appcds tests.
>> > > >>>>>>>
>> > > >>>>>>> Best,
>> > > >>>>>>> Jiangli
>> > > >>>>>>>
>> > > >>>>
>> > > >>