RFR (XS) 8219574: Minimal VM build failure after JDK-8219414

[David Holmes]

On 22/02/2019 9:12 pm, Aleksey Shipilev wrote:
> On 2/22/19 11:48 AM, David Holmes wrote:
>>> diff -r e94ed0236046 src/hotspot/os/linux/os_linux.cpp
>>> --- a/src/hotspot/os/linux/os_linux.cpp Fri Feb 22 09:23:37 2019 +0100
>>> +++ b/src/hotspot/os/linux/os_linux.cpp Fri Feb 22 10:51:11 2019 +0100
>>> @@ -1357,11 +1357,11 @@
>>>    // called from signal handler. Before adding something to os::abort(), make
>>>    // sure it is async-safe and can handle partially initialized VM.
>>>    void os::abort(bool dump_core, void* siginfo, const void* context) {
>>>      os::shutdown();
>>>      if (dump_core) {
>>> -#ifndef ZERO
>>> +#ifndef INCLUDE_CDS
>>>        if (DumpPrivateMappingsInCore) {
>>>          ClassLoader::close_jrt_image();
>>>        }
>>>    #endif
>>>    #ifndef PRODUCT
>>>
>>> JDK-8219414 incorrectly removed INCLUDE_CDS guard, while ClassLoader::close_jrt_image() is still
>>> only defined under INCLUDE_CDS.
>>
>> Right - though the function seems to be declared unconditionally, the definition is only under
>> INCLUDE_CDS. I recall seeing some discussion of this change but am left wondering how on earth ZERO
>> came into play! ??
> 
> With our current rich feature flags ifdef-ing the JVM variant should be considered the big red flag.
> The code should be guarded with feature flags. And indeed it was, until reviews got side-tracked:
>    https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-February/032756.html

Right. Still not sure why ZERO was raised here.

>> Your fix seems good, but I think there is more work related to close_jrt_image() needed (by others).
> 
> I agree. Not sure if close_jrt_image() should just be NOT_CDS_RETURN, if it is indeed CDS-specific.
> 
> Consider this fix trivial?

:) Sure.

Thanks,
David
-----

> -Aleksey
>