Yumin, This looks much better. Thank you for spending the extra time on this. The SA code in general (and the native code in particular) is quite messy and could use a lot of cleanup, but we'll have to leave that for later.
A few comments below: General comment: Please use "#ifdef __APPLE__" consistently (instead of sometimes using "#ifndef __APPLE__"). I think this makes it easier to read the code. MacosxDebuggerLocal.m getThreadIntegerRegisterSet(): - Should be renamed getThreadIntegerRegisterSetFromCore() or something similar to indicate that this is used only for core files. readBytesFromProcess() - Should also be renamed tom include FromCore. Java_sun_jvm_hotspot_debugger_bsd_BsdDebuggerLocal_attach0__I() - It looks like the code for ptrace_attach (lines 528-534) were accidentally removed fill_java_threads - CHECK_EXCEPTION_(0) should be CHECK_EXCEPTION_(false) - I think cinfos should be of type uint64_t* since it holds some (more or less) untyped values. - The inner loop can be made easier to read by breaking out the reading of cinfos[]. Something like: for (j = 0; j < len; j += 3) { lwpid_t lwp = cinfos[j]; uint64_t beg = cinfos[j + 1]; uint64_t end = cinfos[j + 2]; if ((regs.r_rsp < end && regs.r_rsp >= beg) || (regs.r_rbp < end && regs.r_rbp >= beg)) { set_lwp_id(ph, i, lwp); break; } } - Also note in the above code that rsp and rbp cannot be equal to end, they have to be strictly smaller. BsbDebuggerLocal.java getThreadForIdentifierAddress(Address addr) - I had intentionally made this throw an exception when fixing JDK-8006423. This was so that a BsdThread could not be created without setting the unique_thread_id. Is that no longer valid? Or was this method resurrected by mistake? getJavaThreadsInfo - nit: no need to cast the value from t.getStackBaseValue(), it's already a long BsdThread.java BsdThread(BsdDebugger debugger, Address threadIdAddr) - This constructor was removed in JDK-8006423 - see above. HotspotAgent.java connectRemoteDebugger() - The if-clause is missing a case for "darwin". (Unfortunately remote debugging does not work even after this fix because of the changes I recently made to BsdAMD64JavaThreadPDAccess.getThreadProxy(), so that will have to be revisted.) Thanks, /Staffan On 3 mar 2013, at 08:57, Yumin Qi <yumin...@oracle.com> wrote: > Hi, all > > Please review at the new changes. Include > 1) use unique_thread_id (which is a 64 bit integer on Macosx) to identify > thread. Add a function in BsdDebuggerLocal to call the newly added function > BsdThread.getUniqueThreadId to get this id. Meanwhile, move the code of > forming threadList in native part to BsdDebuggerLocal.java since the thread > ids of all java threads can be obtained from Threads and coding is much > easier and clear. > > 2) To have better performance, get all java thread ids, stack infos (stack > begin, stack end) into one array of long which is decoded in native code and > used to set thread ids. This save much more time first time to fill java > thread ids. > > 3) remove DarwinVtblAccess.java which added in last version , its > functionality moved to BsdVtblAccess.java > > 4) BugSpotAgent.java no long exists, remove the changes. > > 5) remove unsupported platform defs. > > http://cr.openjdk.java.net/~minqi/8003348/ > > Thanks > Yumin > > On 1/22/2013 10:35 PM, Yumin Qi wrote: >> Hi, Staffan (and Serguei) >> >> Made some clean for code. >> 1) added mach-o file fat header parsing as you suggested. >> 2) modified get_real_path as you indicated it could run with jre/bin/java >> 3) moved output information from CommandProcessor.java to PStack.java for >> printing out pstack not available for Darwin. >> 4) code clean, file header update. >> >> Please take a look at same location. >> >> Thanks >> Yumin >> >> On 1/18/2013 3:58 AM, Staffan Larsen wrote: >>> Thanks for doing this Yumin! >>> >>> I tried to apply you patch and run it, but I can't get SA to open a core >>> file. You can see the exception I get below. Is there some kind of setup I >>> need to do? This is against a jvmg build of Hotspot. >>> >>> I also noticed that you forgot to update BugSpotAgent.java with the same >>> changes as in HotspotAgent.java. This makes the jstack tool fail. >>> >>> I will look at the changes more closely. >>> >>> Thanks, >>> /Staffan >>> >>> >>> >>> Opening core file, please wait... >>> Unable to open core file >>> /cores/core.79028: >>> >>> Doesn't appear to be a HotSpot VM (could not find symbol "gHotSpotVMTypes" >>> in >>> remote process) >>> sun.jvm.hotspot.debugger.DebuggerException: Doesn't appear to be a HotSpot >>> VM (could not find symbol "gHotSpotVMTypes" in remote process) >>> at sun.jvm.hotspot.HotSpotAgent.setupVM(HotSpotAgent.java:385) >>> at sun.jvm.hotspot.HotSpotAgent.go(HotSpotAgent.java:287) >>> at sun.jvm.hotspot.HotSpotAgent.attach(HotSpotAgent.java:146) >>> at sun.jvm.hotspot.CLHSDB.attachDebugger(CLHSDB.java:188) >>> at sun.jvm.hotspot.CLHSDB.run(CLHSDB.java:55) >>> at sun.jvm.hotspot.CLHSDB.main(CLHSDB.java:35) >>> hsdb> Input stream closed. >>> >>> >>> On 17 jan 2013, at 22:23, Yumin Qi<yumin...@oracle.com> wrote: >>> >>>> Hi, >>>> >>>> Please review for the changes for SA to read core file on Mac OS X, >>>> this is feature is not implemented on previous releases. >>>> This change made for SA to work on core file on Darwin, but still some >>>> function not fixed, such as 'pstack'. This is intended to integrate into 8. >>>> >>>> http://cr.openjdk.java.net/~minqi/8003348/ >>>> >>>> Please take some time since the code change is a little bigger. >>>> >>>> Thanks very much >>>> Yumin >