RFR: 8199807 & 8202738: AppCDS performs overly restrictive path matching check

Fri May 11 21:44:21 UTC 2018

Hi Erik,

Thank you so much for investigating a proper fix for the vm_version.o issue. I will withdraw the build change from my original webrev.

Thanks again for taking over the bug!
Jiangli 

> On May 11, 2018, at 2:33 PM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
> 
> Received: from [10.132.185.167] (/10.132.185.167)
>    by default (Oracle Beehive Gateway v4.0)
>    with ESMTP ; Fri, 11 May 2018 14:33:20 -0700
> Subject: Re: RFR: 8199807 & 8202738: AppCDS performs overly restrictive path
> matching check
> To: Jiangli Zhou <jiangli.zhou at oracle.com>,
> "hotspot-runtime-dev at openjdk.java.net runtime"
> <hotspot-runtime-dev at openjdk.java.net>,
> build-dev <build-dev at openjdk.java.net>
> References: <B173B2EF-9908-418A-9DCC-EE6D9133DC10 at oracle.com>
> From: Erik Joelsson <erik.joelsson at oracle.com>
> Organization: Oracle Corporation
> Message-ID: <ae4c66a1-ad7d-00f1-ced1-729704d56a9a at oracle.com>
> Date: Fri, 11 May 2018 14:33:20 -0700
> User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0)
> Gecko/20100101 Thunderbird/52.7.0
> MIME-Version: 1.0
> In-Reply-To: <B173B2EF-9908-418A-9DCC-EE6D9133DC10 at oracle.com>
> Content-Type: text/plain; charset=utf-8; format=flowed
> Content-Transfer-Encoding: 8bit
> Content-Language: en-US
> 
> Hello,
> 
> For the build change, it's very undesirable to always have to relink libjvm on every incremental build. Such a change cannot be accepted.
> 
> I have a counter suggestion, which is still a bit of a hack, but it will cause vm_version.cpp to be recompiled (almost) every time libjvm.so needs to be relinked. The drawback is that compiling vm_version.cpp is now bound to happen absolutely last and so cannot happen in parallel with other compilations.
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8202738/webrev.01/index.html
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8202738
> 
> /Erik
> 
> 
>> On 2018-05-10 16:11, Jiangli Zhou wrote:
>> Hi,
>> 
>> Please review the following webrev that addresses the issue of copied/moved JDK image after generating a CDS archive. Thanks Karen Kinnear and Alan Baterman for initiating the investigation & discussions in this area (especially the ease of usage). Thanks Ioi for implementing a test case for moved JDK (JDK-8202935).
>> 
>> webrev: http://cr.openjdk.java.net/~jiangli/8199807_8202738/webrev.00/
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8199807?filter=14921
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8202738?filter=14921
>> 
>> The webrev includes the following three main parts:
>> 
>> 1. Reduced check for JDK ‘modules’ image file at runtime. The runtime path to the ‘modules’ image is no longer required to the the same as dump time path. Runtime performs file size check only for the ‘modules’ image, which must match with the dump time ‘modules’ size. Invalidation of an outdated archive is achieved by the existing vm_version string check (the archived vm_version string must match with the runtime vm_version string).
>> 
>> 2. Boot path check are now performed based on the content of the archive. Also added a new test case in BootClassPathMismatch.java and add more comments for existing test cases.
>> 
>> 3. Fixed the stale vm_version string issue with incremental build. The issue was discovered during the work of 8199807. CDS uses vm_version string as part of the runtime validation check for archive. A stale vm_version string causes the CDS runtime to mistakenly accept an outdated archive. The fix is to make sure vm_version.o is recompiled properly when the library/vm is rebuilt.
>> 
>> Tested with hs-tier1-4 and jdk-tier1-2. Tested by relocating the JDK image manually after generating an archive. Also tested with Ioi’s test both locally and via Mach5.
>> 
>> Thanks,
>> Jiangli
>> 
> 
> Hello,
> 
> For the build change, it's very undesirable to always have to relink libjvm on every incremental build. Such a change cannot be accepted.
> 
> I have a counter suggestion, which is still a bit of a hack, but it will cause vm_version.cpp to be recompiled (almost) every time libjvm.so needs to be relinked. The drawback is that compiling vm_version.cpp is now bound to happen absolutely last and so cannot happen in parallel with other compilations.
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8202738/webrev.01/index.html
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8202738
> 
> /Erik
> 
> 
>> On 2018-05-10 16:11, Jiangli Zhou wrote:
>> Hi,
>> 
>> Please review the following webrev that addresses the issue of copied/moved JDK image after generating a CDS archive. Thanks Karen Kinnear and Alan Baterman for initiating the investigation & discussions in this area (especially the ease of usage). Thanks Ioi for implementing a test case for moved JDK (JDK-8202935).
>> 
>> webrev: http://cr.openjdk.java.net/~jiangli/8199807_8202738/webrev.00/
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8199807?filter=14921
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8202738?filter=14921
>> 
>> The webrev includes the following three main parts:
>> 
>> 1. Reduced check for JDK ‘modules’ image file at runtime. The runtime path to the ‘modules’ image is no longer required to the the same as dump time path. Runtime performs file size check only for the ‘modules’ image, which must match with the dump time ‘modules’ size. Invalidation of an outdated archive is achieved by the existing vm_version string check (the archived vm_version string must match with the runtime vm_version string).
>> 
>> 2. Boot path check are now performed based on the content of the archive. Also added a new test case in BootClassPathMismatch.java and add more comments for existing test cases.
>> 
>> 3. Fixed the stale vm_version string issue with incremental build. The issue was discovered during the work of 8199807. CDS uses vm_version string as part of the runtime validation check for archive. A stale vm_version string causes the CDS runtime to mistakenly accept an outdated archive. The fix is to make sure vm_version.o is recompiled properly when the library/vm is rebuilt.
>> 
>> Tested with hs-tier1-4 and jdk-tier1-2. Tested by relocating the JDK image manually after generating an archive. Also tested with Ioi’s test both locally and via Mach5.
>> 
>> Thanks,
>> Jiangli
>> 
> 