RFR(XS): 8043264: hsdis library not picked up correctly on expected paths

[serguei.spitsyn at oracle.com]

On 5/15/14 1:19 PM, Krystal Mok wrote:
> Hi everyone,
>
> May I have a couple of review for this small patch, please?
>
> *Bug*: https://bugs.openjdk.java.net/browse/JDK-8043264
>
> *Patch*: (against jdk9/hs)
>
> diff -r 466b58fa837b src/share/vm/compiler/disassembler.cpp
> --- a/src/share/vm/compiler/disassembler.cppThu May 15 11:35:26 2014 -0700
> +++ b/src/share/vm/compiler/disassembler.cppThu May 15 13:14:58 2014 -0700
> @@ -86,7 +86,7 @@
>    {
>      // Match "jvm[^/]*" in jvm_path.
>      const char* base = buf;
> -    const char* p = strrchr(buf, '/');
> +    const char* p = strrchr(buf, *os::file_separator());
>      if (p != NULL) lib_offset = p - base + 1;
>      p = strstr(p ? p : base, "jvm");
>      if (p != NULL)  jvm_offset = p - base;
> @@ -111,7 +111,7 @@
>      if (_library == NULL) {
>        // 3. <home>/jre/lib/<arch>/hsdis-<arch>.so
>        buf[lib_offset - 1] = '\0';
> -      const char* p = strrchr(buf, '/');
> +      const char* p = strrchr(buf, *os::file_separator());
>        if (p != NULL) {
>          lib_offset = p - buf + 1;
>          strcpy(&buf[lib_offset], hsdis_library_name);

The fix looks good.

Thanks,
Serguei

>
> *Description*:
>
> (Copied from the bug)
>
> On Windows, the hsdis library isn't picked up correctly in all 
> expected paths described in Disassembler::load_library():
>
>   // Find the disassembler shared library.
>   // Search for several paths derived from libjvm, in this order:
>   // 1. <home>/jre/lib/<arch>/<vm>/libhsdis-<arch>.so (for compatibility)
>   // 2. <home>/jre/lib/<arch>/<vm>/hsdis-<arch>.so
>   // 3. <home>/jre/lib/<arch>/hsdis-<arch>.so
>   // 4. hsdis-<arch>.so (using LD_LIBRARY_PATH)
>
> The reason is that the code that concatenates the paths doesn't take 
> os::file_separator() into account, and always uses '/' instead, like:
>
> const char* p = strrchr(buf, '/');
>
> The fix is to change the use of '/' into *os::file_separator() on two 
> lines in Disassembler::load_library().
>
> *Testing*
>
> The change was tested by a friend of mine on Windows and he said 
> hsdis.dll could be picked up from expected paths after the change. I 
> don't have a Windows box to test it myself, so I'd like someone from 
> Oracle to help me test it.
>
> P.S. I wanted to upload a webrev, but somehow I couldn't get the the 
> connection to work. Could it be that my publickey is missing on 
> cr.openjdk.java.net <http://cr.openjdk.java.net>?
>
> The authenticity of host 'cr.openjdk.java.net 
> <http://cr.openjdk.java.net> (137.254.56.61)' can't be established.
> RSA key fingerprint is d2:d1:ee:03:4b:11:45:49:ec:46:99:8c:e4:c3:00:c4.
> Are you sure you want to continue connecting (yes/no)? yes
> Warning: Permanently added 'cr.openjdk.java.net 
> <http://cr.openjdk.java.net>,137.254.56.61' (RSA) to the list of known 
> hosts.
> Permission denied (publickey).
> lost connection
>
> Best regards,
> Kris (OpenJDK username: kmo)