RFR: 8003348: SA can not read core file on OS X

[Staffan Larsen]

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.qi at 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.qi at 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
>