Good.
Vladimir
Tom Rodriguez wrote:
> Yes that's better. I changed it slightly and copied the different step comments to the appropriate places. It's slightly verbose but very explicit.
>
> tom
>
> diff -r de6a837d75cf src/share/vm/compiler/disassembler.cpp
> --- a/src/share/vm/compiler/disassembler.cpp
> +++ b/src/share/vm/compiler/disassembler.cpp
> @@ -78,21 +78,46 @@
> char buf[JVM_MAXPATHLEN];
> os::jvm_path(buf, sizeof(buf));
> int jvm_offset = -1;
> + int lib_offset = -1;
> {
> // Match "jvm[^/]*" in jvm_path.
> const char* base = buf;
> const char* p = strrchr(buf, '/');
> + if (p != NULL) lib_offset = p - base + 1;
> p = strstr(p ? p : base, "jvm");
> if (p != NULL) jvm_offset = p - base;
> }
> + // 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)
> if (jvm_offset >= 0) {
> - // Find the disassembler next to libjvm.so.
> + // 1. <home>/jre/lib/<arch>/<vm>/libhsdis-<arch>.so
> strcpy(&buf[jvm_offset], hsdis_library_name);
> strcat(&buf[jvm_offset], os::dll_file_extension());
> _library = os::dll_load(buf, ebuf, sizeof ebuf);
> + if (_library == NULL) {
> + // 2. <home>/jre/lib/<arch>/<vm>/hsdis-<arch>.so
> + strcpy(&buf[lib_offset], hsdis_library_name);
> + strcat(&buf[lib_offset], os::dll_file_extension());
> + _library = os::dll_load(buf, ebuf, sizeof ebuf);
> + }
> + if (_library == NULL) {
> + // 3. <home>/jre/lib/<arch>/hsdis-<arch>.so
> + buf[lib_offset - 1] = '\0';
> + const char* p = strrchr(buf, '/');
> + if (p != NULL) {
> + lib_offset = p - buf + 1;
> + strcpy(&buf[lib_offset], hsdis_library_name);
> + strcat(&buf[lib_offset], os::dll_file_extension());
> + _library = os::dll_load(buf, ebuf, sizeof ebuf);
> + }
> + }
> }
> if (_library == NULL) {
> - // Try a free-floating lookup.
> + // 4. hsdis-<arch>.so (using LD_LIBRARY_PATH)
> strcpy(&buf[0], hsdis_library_name);
> strcat(&buf[0], os::dll_file_extension());
> _library = os::dll_load(buf, ebuf, sizeof ebuf);
> tom
>
> On Jul 5, 2011, at 3:15 PM, John Rose wrote:
>
>> Good. Thanks for fixing that.
>>
>> Instead of the comment "First try libhsdis for compatibility" I suggest laying out the whole search path, something like the following.
>>
>> // Search for several paths derived from libjvm, in this order:
>> // 1. <home>/jre/lib/<arch>/<vm>/libhsdis.so
>> // 2. <home>/jre/lib/<arch>/<vm>/hsdis.so
>> // 3. <home>/jre/lib/<arch>/hsdis.so
>> // 4. hsdis.so (no path)
>>
>> -- John
>>
>> On Jul 5, 2011, at 3:01 PM, Vladimir Kozlov wrote:
>>
>>> Looks good.
>>>
>>> Vladimir
>>>
>>> Tom Rodriguez wrote:
>>>> http://cr.openjdk.java.net/~never/7062856
>>>> 29 lines changed: 27 ins; 0 del; 2 mod; 487 unchg
>>>>
>>>> 7062856: Disassembler needs to be smarter about finding hsdis after 1.7 launcher changes
>>>> Summary: do explicit lookup emulating old LD_LIBRARY_PATH search
>