RFR: JDK-8230413: Support Pre JDK 6 class with CDS

[David Holmes]

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 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.

> - 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.

> 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.

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
>>>>>>>
>>>>
>>