RFR (L) JDK-8154239: -Xbootclasspath/a breaks exploded build

[Lois Foltan]

On 7/21/2016 11:25 AM, harold seigel wrote:
> Hi Lois,
>
> This is a nice fix for the exploded build system class path issues!
>
> If the callers of methods such as add_to_exploded_build_list() already 
> have THREAD, can it be passed as a parameter?
Done.

>
> In classLoader.cpp, the comment at lines 1466 - 1468 say that the 
> starting classpath_index is  always 1.  Can you either add an assert 
> for this or change line 1469 to just: classpath_index = 1;
Done.

>
> In classLoader.hpp, could has_jimage()  be renamed to something like 
> has_jrt_entry so it's not confused with the has_jimage() method in 
> arguments.hpp?
Done.

>
> In filemap.cpp, can the code at lines 226 - 230 be changed to:
>
>    assert(strptr + name_bytes <= strptr_max, "miscalculated buffer 
> size");
>    strncpy(...)
>    strptr += name_bytes;
Done.

Thanks for the review Harold!  New webrev at 
http://cr.openjdk.java.net/~lfoltan/bug_jdk8154239.1/webrev/
Lois

>
> Thanks, Harold
>
>
> On 7/19/2016 2:09 PM, Lois Foltan wrote:
>> Hello,
>>
>> Please review the following fix:
>>
>> Webrev:
>>     http://cr.openjdk.java.net/~lfoltan/bug_jdk8154239/webrev/
>>
>> Bug: -Xbootclasspath/a breaks exploded build
>>     https://bugs.openjdk.java.net/browse/JDK-8154239
>>
>> Summary:
>> The JVM was incorrectly handling how to set up and search the 
>> ClassPathEntry's for directories representing the module definitions 
>> in an exploded module build situation for the boot class loader. The 
>> incorrect set up and search was particularly exposed when 
>> -Xbootclasspath/a was specified in conjunction with a exploded module 
>> build.  In an exploded module build, when Modules::define_modules was 
>> called to define a module to the boot loader, a ClassPathEntry was 
>> simply appended to the boot loader's search list.  At load class 
>> time, a class would then be searched by simply traversing that list 
>> without any regards to the module that class was defined in.  This is 
>> incorrect behavior.  The class should only be searched for in the 
>> location of the module's definition that it was defined within.  The 
>> fix now records each module and it's exploded build location in order 
>> to adhere to this rule.
>>
>> The other piece to this problem was that if -Xbootclasspath/a was 
>> specified, it was incorrectly injected prior to the majority of 
>> ClassPathEntry's for the exploded modules, yielding a broken and 
>> incorrect search order for the boot loader.  Introducing the concept 
>> of a "base" or "core" piece for the boot loader which is either the 
>> java runtime image or the exploded modules build and changing the 
>> ClassLoader::_first_append_entry to truly be only appended entries 
>> added via -Xbootclasspath/a or jvmti appended entries, introduces a 
>> clean way for how this information is stored in the ClassLoader data 
>> structure that maps directly to how ClassLoader::load_class() 
>> searches that information.
>>
>>  215   // The boot class path consists of 3 ordered pieces:
>>  216   //  1. the module/path pairs specified to -Xpatch
>>  217   // -Xpatch:<module>=<file>(<pathsep><file>)*
>>  218   //  2. the base piece
>>  219   //    [jimage | build with exploded modules]
>>  220   //  3. boot loader append path
>>  221   //    [-Xbootclasspath/a]; [jvmti appended entries]
>>  222   //
>>  223   // The boot loader must obey this order when attempting
>>  224   // to load a class.
>>
>>
>> Testing:
>> Full hotspot nightly testing with a java runtime image build
>> JDK java/io, java/lang, java/util with both a java runtime image and 
>> an exploded module build
>> JCK lang & vm with both a java runtime image and an exploded module 
>> build
>> Hotspot jtreg tests with both a java runtime image and an exploded 
>> module build
>>
>