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

[harold seigel]

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?

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;

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?

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;

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
>