RFR(xxs, jdk10): 8171508: os::jvm_path -XXaltjvm processing error after 8066474

Thu Apr 27 17:34:41 UTC 2017

Hi Vladimir, David,

On Thu, Apr 27, 2017 at 7:07 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com
> wrote:

> I think we just need to remove -XXaltjvm.
>
> Originally we use it for testing locally built JVM (when we had ability to
> build Hotspot JVM separate). And for binary search of changes which may
> cause a failure by selecting different JVM for the same JDK. OR even
> different JDK releases (Hotspot Express).
>
> But since JDK 8 it become almost impossible since JVM code is tightly
> connected to JDK code.
>
> Regards,
> Vladimir
>
>
I am fine with killing it off in some form. As we already discussed before,
it has also made bad waves security wise, justly or unjustly.

Part of the mechanism is used for the gtestLauncher: it does not use
-XXaltjvm, but links against the gtest libjvm.so and then invokes java with
-Dsun.java.launcher.is_altjvm=true to make os::jvm_path() switch to a
different JDK. That mechanism at least must survive, or gtestLauncher must
be changed.

Kind Regards, Thomas

>
> On 4/27/17 5:21 AM, David Holmes wrote:
>
>> Hi Thomas,
>>
>> Thanks for the detailed investigation. I am very confused by all this.
>> I'm wondering whether we should just kill off -XXaltjvm altogether?
>> Otherwise I'm not at all sure what is the "right" thing to do as I'm not
>> clear what it is intended to do.
>>
>> David
>>
>> On 27/04/2017 7:25 PM, Thomas Stüfe wrote:
>>
>>> Hi David,
>>>
>>> You are right, this is more complicated.
>>>
>>> --
>>>
>>> Here is what I think was originally intended:
>>>
>>> Scenario a): I start java from jdk A and give it, via -XXaltjvm, a path
>>> pointing to a libjvm.so in jdk B. In this case, os::jvm_path() should
>>> detect that jdk B is a full JDK, and return the path to jdk B. So, all
>>> JDK libraries should be loaded from jdk B.
>>>
>>> Scenario b) I start java from jdk A and give it, via -XXaltjvm, a path
>>> pointing to a standalone libjvm.so. In this case, os::jvm_path() should
>>> see that there is no full valid JDK around the alternate libjvm.so, and
>>> uses a JDK from JAVA_HOME. Failing that (no JAVA_HOME set), it should
>>> use the path from the loaded alternative libjvm.so.
>>>
>>> --
>>>
>>> However, there are errors:
>>>
>>> Error 1: -XXaltjvm is currently not handled by os::jvm_path(): Arguments
>>> <http://ld8443:8080/source/s?defs=Arguments&project=openjdk-jdk10-hs
>>> >::sun_java_launcher_is_altjvm
>>>
>>> <http://ld8443:8080/source/s?defs=sun_java_launcher_is_altjv
>>> m&project=openjdk-jdk10-hs>()
>>>
>>> returns always false and we skip altjvm handling altogether. This is
>>> because -Dsun.java.launcher.is_altjvm=true is missing. The Launcher
>>> should set this property if XXaltjvm is set.
>>>
>>> Side note: the gtestLauncher sets -Dsun.java.launcher.is_altjvm=true. So
>>> starting gtests is currently the only way to hit the altjvm path in
>>> os::jvm_path().
>>>
>>> Side note 2: Your CDS scenario just works because the path you add to
>>> -XXaltjvm is inside a full JDK (actually the primary JDK). This needs no
>>> altjvm handling at all in os::jvm_path() - it just returns the path to
>>> the primary jdk. (I wonder why you even need to specify XXaltjvm? Is
>>> this to distinguish between server and client VM?)
>>>
>>> --
>>>
>>> So I do some trials with -XXaltjvm=<path>  together with explicitely
>>> setting -Dsun.java.launcher.is_altjvm=true.
>>>
>>> Scenario a): jdk A = ./images, jdk B = images-2
>>>
>>> without JAVA_HOME set:  ./images/jdk/bin/java
>>> -Dsun.java.launcher.is_altjvm=true -XXaltjvm=./images-2/jdk/lib/server
>>>
>>> We load libjvm.so and libjava.so from images-2. Looks correct? Yes, but
>>> I stepped thru with a debugger, what actually happens is that the
>>> full-JDK-recognition at os_linux.cpp:2348 fails. Then it checks for
>>> JAVA_HOME, which it is not set. Then it just gives up and falls back to
>>> the path to the alternate libjvm.so at os_linux.cpp:2364.Which happens
>>> to be the correct path anyway for jdk B,
>>>
>>> Now lets set JAVA_HOME to some completely unrelated JDK. os::jvm_path()
>>> should ignore the value and still return path to jdk b, yes? But no:
>>>
>>> - .../output $ export
>>> JAVA_HOME=/sapmnt/depot/tools/gen/linuxx86_64/licenseware/jse/1.5.0
>>> - .../output $ ./images/jdk/bin/java -Dsun.java.launcher.is_altjvm=true
>>> -XXaltjvm=./images-2/images/jdk/lib/server
>>> Error occurred during initialization of VM
>>> Unable to load native library:
>>> /net/sapmnt.depot/tools/gen/linuxx86_64/licenseware/jse/1.5.
>>> 0/jre/lib/libjava.so:
>>>
>>> cannot open shared object file: No such file or directory
>>>
>>> This is Error 2: os::jvm_path() does not recognize "images-2" to be a
>>> valid full JDK, so it falls back to JAVA_HOME, which in this case points
>>> to some unrelated older JDK. Here, in this case,  it fails to load the
>>> libjava.so. But it should never have attempted to load it.
>>>
>>> This is the bug my patch attempts to fix, admittedly you are right,
>>> there may be more to it (the "jre" for example).
>>>
>>> Scenario b) just works, if JAVA_HOME is set correctly. This is what the
>>> gtestLauncher does: It sets -Dsun.java.launcher.is_altjvm=true and sets
>>> JAVA_HOME to the value of the -jdk:<path> argument (see gtestMain.cpp).
>>>
>>> --
>>>
>>> So the first error is IMHO that when -XXaltjvm is passed it should set
>>> -Dsun.java.launcher.is_altjvm=true but it does not.
>>>
>>> The second error is that the half-baked full-jdk-detection in
>>> os::jvm_path() does not work. You are right, there are more errors than
>>> the 5-slashes-path-traversal. This makes Scenario a) misbehave if
>>> JAVA_HOME happens to be set to another JDK. You will either not load, as
>>> in my case, or load JDK libraries from another JDK.
>>>
>>> I honestly do not like this coding at all and would love to see this
>>> made simpler. This is very difficult to understand.
>>>
>>> Kind Regards, Thomas
>>>
>>>
>>> On Thu, Apr 27, 2017 at 1:31 AM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>>     Hi Thomas,
>>>
>>>     Backing up a few steps ...
>>>
>>>     On 26/04/2017 5:10 PM, Thomas Stüfe wrote:
>>>
>>>         Hi all,
>>>
>>>         may I please have a review for this tiny fix. 8066474 removed
>>>         the <arch>
>>>         directory from the images and since then -XXaltjvm was slightly
>>>         broken.
>>>         When handling XXaltjvm, os::jvm_path() examines the path of the
>>>         libjvm.so
>>>         to check if it is part of what it considers a standard JDK by
>>>         traversing a
>>>         number of slashes up the path and looking for "/jre/lib". That
>>>         number of
>>>         slashes was off since 8066474.
>>>
>>>
>>>     Hold on a moment! We don't have a jre subdirectory any more either!
>>>     The image for a jdk directly contains lib/server/libjvm.so (for
>>>     example).
>>>
>>>     Also jvm_path is used for dumping the shared archive using the
>>>     default location. If I do:
>>>
>>>     > ./images/jdk/bin/java -XXaltjvm=images/jdk/lib/server/
>>> -Xshare:dump
>>>
>>>     it works perfectly correctly:
>>>
>>>      > find . -name classes.jsa
>>>     ./images/jdk/lib/server/classes.jsa
>>>
>>>     So where's the bug ??
>>>
>>>     David
>>>     -----
>>>
>>>
>>>
>>>         webrev:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8171508-os_jvm_pa
>>> th_xxaltjvm_processing_error_after_8066474/jdk10-webrev.00/
>>> webrev/index.html
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8171508-os_jvm_p
>>> ath_xxaltjvm_processing_error_after_8066474/jdk10-webrev.00/
>>> webrev/index.html>
>>>
>>>
>>>         Bug: https://bugs.openjdk.java.net/browse/JDK-8171508
>>>         <https://bugs.openjdk.java.net/browse/JDK-8171508>
>>>
>>>         Note that this only affects cases where the alternate libjvm.so
>>>         is part of
>>>         a full jdk, so it does not affect the gtestLauncher.
>>>
>>>         Thanks & Regards, Thomas
>>>
>>>
>>>