RFR: 8211296: Remove HotSpot deprecation warning suppression for Mac/clang

[David Holmes]

On 3/10/2018 6:51 AM, Kim Barrett wrote:
>> On Oct 1, 2018, at 7:10 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> On 2/10/2018 4:35 AM, Kim Barrett wrote:
>>>> On Oct 1, 2018, at 2:27 AM, David Holmes <david.holmes at oracle.com> wrote:
>>>>
>>>> On 1/10/2018 1:21 PM, Kim Barrett wrote:
>>>>> The current library may have been previously only dlopen’ed with RTLD_LAZY.  Reopening with RTLD_NOW
>>>>> forces resolution of all undefined symbols in that shared object, which appears to be the purpose of the
>>>>> deprecated function.
>>>>
>>>> The current library is libjvm and it's already opened in the appropriate way:
>>>>
>>>>    libjvm = dlopen(jvmpath, RTLD_NOW + RTLD_GLOBAL);
>>>>
>>>> and it seems to have always been opened this way. So I can't see how this can fix the problem the workaround purports to fix??
>>>>
>>>> http://hg.openjdk.java.net/macosx-port/macosx-port/hotspot/rev/be94a5de4d32#l54.758
>>> The current version of LoadJavaVM (for Mac) contains:
>>> #ifndef STATIC_BUILD
>>>      libjvm = dlopen(jvmpath, RTLD_NOW + RTLD_GLOBAL);
>>> #else
>>>      libjvm = dlopen(NULL, RTLD_FIRST);
>>> #endif
>>> where STATIC_BUILD is controlled by the --enable-static-builds
>>> configure option. The --enable-static-builds option was added in JDK 9
>>> by 8136556: Add the ability to perform static builds of MacOSX x64
>>> binaries.
>>
>> Which "current version" is this? It is not what is in jdk/jdk repo, which has no STATIC_BUILD support in this area. ??
> 
> I think the MacOSX version of LoadJavaVM is here:
> ./java.base/macosx/native/libjli/java_md_macosx.m
> (Note that this appears to be an Obj-C source file.)
> (And no, I have no idea what build magic goes on to make this work.)
> This has the STATIC_BUILD stuff.

Hmmm okay. But I can't find that file in the original  mac port, or in 
7u at all. In 7u we have

jdk/src/macosx/bin/java_md_macosx.c

with

libjvm = dlopen(jvmpath, RTLD_NOW + RTLD_GLOBAL);

and that was created in 2012 when the launcher was refactored under 
JDK-7124089.

The archaeology is getting too hard. :(

> There are also definitions for other platforms.
> ./java.base/windows/native/libjli/java_md.c       // Windows
> ./java.base/unix/native/libjli/java_md_solinux.c  // Solaris/Linux/AIX
> (Note that java_md_solinux.c is excluded for macosx by the build
> system; see LIBJLI_EXCLUDE_FILES in CoreLibraries.gmk.)
> 
>> Although we may have not used RTLD_NOW with static builds (does this discussions re symbol resolution even make sense with a static build?)
> 
> I think symbol resolution can make sense with a static build, if there
> are other libraries that are not statically linked. I don't know
> whether that's the case here. I'm going to try doing a static build
> without this dlopen code and see what happens.
> 
>> in some version of the JDK, my point is that we have been using RTLD_NOW for as long as the workaround has been in place.
>> That strongly suggests to me that use of RTLD_NOW is not a solution to the problem the workaround was introduced for.
> 
> I think my replacement code is equivalent to the deprecated code.  I
> think the sequence of events may have been something like:
> 
> (1) dlopen java lib without RTLD_NOW
> (2) run into problems that led to workaround code
> (3) run into further problems that led to dlopen java lib with RTLD_NOW
>      but didn't remove the now superfluous workaround (perhaps it was
>      forgotten?)
> (4) merge

Perhaps. No way to know.

> 
>>> I thought this part of the change would be straight-forward. An
>>> alternative would be to leave the old deprecated call in place,
>>> locally disable the deprecation warning, and file an RFE for someone
>>> more knowledgable in this area and with Mac development to look at.
>>
>> Sorry for the obstructions on this but changing the code without knowing it even implements the same workaround is just wrong to me. I strongly suspect the code is not needed at all.
> 
> I agree that for the non-STATIC_BUILD case the workaround is
> superfluous. If a static build without any workaround code works then
> I think the workaround code should just be removed.  Is that okay with
> you?

Yes but you will need to go through a full round of testing, not just 
tier 1-3, before being reasonably confident the workaround is not 
needed. I'd hate to see this break things "in the wild" just because we 
don't have adequate test coverage.

Other opinions welcomed.

David

>