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
>