[RFA] JDWP implment getFrames, getFrame, getFrameCount

[RFA] JDWP implment getFrames, getFrame, getFrameCount

[Kyle Galloway]

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

Hey,

This patch implments the functions getFrames, getFrame, and 
getFrameCount in natVMVirtualMachine.cc.  getFrames determines the id's 
of the frames it wants, then calls getFrame to generate VMFrame objects. 
  getFrame handles the actual call to jvmti->GetStackTrace, and retrievs 
information one frame at a time, then creates a new VMFrame and returns 
it.  getFrameCount simply calls jvmti-GetFrameCount and returns the count.

I also changed ThreadReferenceCommandSet to pass jlongs to getFrame 
since the ByteBuffer seemed like overkill since all that would be done 
is a bb.getLong call.

Comments, questions?

-Kyle

ChangeLog
2007-01-31  Kyle Galloway  <kgallowa@redhat.com>

	* classpath/gnu/classpath/jdwp/processor/
	StackFrameCommandSet.java (executeGetValues): Pass jlong instead
	of ByteBuffer.
	(executeSetValues): Ditto.
	(executeThisObject): Ditto.
	* classpath/gnu/classpath/jdwp/processor/
	StackFrameCommandSet.class: Rebuilt.
	* classpath/lib/gnu/classpath/jdwp/VMVirtualMachine.class:
	Rebuilt.
	* classpath/lib/gnu/classpath/jdwp/VMFrame.class: Rebuilt.
	* classpath/lib/gnu/classpath/jdwp/exception/
	InvalidFrameException.java: New file.
	* gnu/classpath/jdwp/VMFrame.java: Added field for thread of
	frame.
	(Constructor): New method.
	* gnu/classpath/jdwp/VMFrame.h: Regenerated.
	* gnu/classpath/jdwp/VMVirtualMachine.java
	(getFrame): Changed ByteBuffer to jlong.
	* gnu/classpath/jdwp/natVMVirtualMachine.cc
	(getFrames): Implmented.
	(getFrame): Implmented.
	(getFrameCount): Implmented.
	* gnu/classpath/jdwp/VMVirtualMachine.h: Regenerated.


[-- Attachment #2: stacktrace-jdwp.patch --]
[-- Type: text/x-patch, Size: 17271 bytes --]

Index: libjava/classpath/gnu/classpath/jdwp/processor/StackFrameCommandSet.java
===================================================================
--- libjava/classpath/gnu/classpath/jdwp/processor/StackFrameCommandSet.java	(revision 121296)
+++ libjava/classpath/gnu/classpath/jdwp/processor/StackFrameCommandSet.java	(working copy)
@@ -107,7 +107,8 @@
     // has a reference to them. Furthermore they are not ReferenceTypeIds since
     // these are held permanently and we want these to be held only as long as
     // the Thread is suspended.
-    VMFrame frame = VMVirtualMachine.getFrame(thread, bb);
+    long frameID = bb.getLong();
+    VMFrame frame = VMVirtualMachine.getFrame(thread, frameID);
     int slots = bb.getInt();
     os.writeInt(slots); // Looks pointless but this is the protocol
     for (int i = 0; i < slots; i++)
@@ -125,7 +126,8 @@
     ObjectId tId = idMan.readObjectId(bb);
     Thread thread = (Thread) tId.getObject();
 
-    VMFrame frame = VMVirtualMachine.getFrame(thread, bb);
+    long frameID = bb.getLong();
+    VMFrame frame = VMVirtualMachine.getFrame(thread, frameID);
 
     int slots = bb.getInt();
     for (int i = 0; i < slots; i++)
@@ -142,7 +144,8 @@
     ObjectId tId = idMan.readObjectId(bb);
     Thread thread = (Thread) tId.getObject();
 
-    VMFrame frame = VMVirtualMachine.getFrame(thread, bb);
+    long frameID = bb.getLong();
+    VMFrame frame = VMVirtualMachine.getFrame(thread, frameID);
 
     Object thisObject = frame.getObject();
     Value.writeTaggedValue(os, thisObject);
Index: libjava/classpath/gnu/classpath/jdwp/exception/InvalidFrameException.java
===================================================================
--- libjava/classpath/gnu/classpath/jdwp/exception/InvalidFrameException.java	(revision 0)
+++ libjava/classpath/gnu/classpath/jdwp/exception/InvalidFrameException.java	(revision 0)
@@ -0,0 +1,63 @@
+/* InvalidFrameException.java -- invalid jframeid
+   Copyright (C) 2007 Free Software Foundation
+
+This file is part of GNU Classpath.
+
+GNU Classpath is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2, or (at your option)
+any later version.
+
+GNU Classpath is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Classpath; see the file COPYING.  If not, write to the
+Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version. */
+
+
+package gnu.classpath.jdwp.exception;
+
+import gnu.classpath.jdwp.JdwpConstants;
+
+/**
+ * An exception thrown when the debugger requests an invalid frame in the call
+ * stack.
+ *
+ * @author Kyle Galloway  (kgallowa@redhat.com)
+ */
+public class InvalidFrameException
+  extends JdwpException
+{
+  public InvalidFrameException(long id)
+  {
+    super(JdwpConstants.Error.INVALID_FRAMEID, 
+          "invalid frame id (" + id + ")");
+  }
+
+  public InvalidFrameException(String msg)
+  {
+    super(JdwpConstants.Error.INVALID_FRAMEID, msg);
+  }
+}
Index: libjava/classpath/lib/gnu/classpath/jdwp/processor/StackFrameCommandSet.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: libjava/classpath/lib/gnu/classpath/jdwp/VMVirtualMachine.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: libjava/classpath/lib/gnu/classpath/jdwp/VMFrame.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: libjava/gnu/classpath/jdwp/VMVirtualMachine.h
===================================================================
--- libjava/gnu/classpath/jdwp/VMVirtualMachine.h	(revision 121296)
+++ libjava/gnu/classpath/jdwp/VMVirtualMachine.h	(working copy)
@@ -1,4 +1,3 @@
-
 // DO NOT EDIT THIS FILE - it is machine generated -*- c++ -*-
 
 #ifndef __gnu_classpath_jdwp_VMVirtualMachine__
@@ -17,59 +16,52 @@
     {
       namespace jdwp
       {
-          class VMFrame;
-          class VMMethod;
-          class VMVirtualMachine;
+        class VMVirtualMachine;
         namespace event
         {
-            class EventRequest;
+          class EventRequest;
         }
         namespace util
         {
-            class MethodResult;
+          class MethodResult;
         }
+        class VMFrame;
+        class VMMethod;
       }
     }
   }
-  namespace java
-  {
-    namespace nio
-    {
-        class ByteBuffer;
-    }
-  }
 }
 
 class gnu::classpath::jdwp::VMVirtualMachine : public ::java::lang::Object
 {
-
 public:
-  VMVirtualMachine();
-  static void initialize();
-  static void suspendThread(::java::lang::Thread *);
-  static void suspendAllThreads();
-  static void resumeThread(::java::lang::Thread *);
-  static void resumeAllThreads();
-  static jint getSuspendCount(::java::lang::Thread *);
-  static jint getAllLoadedClassesCount();
-  static ::java::util::Iterator * getAllLoadedClasses();
-  static jint getClassStatus(::java::lang::Class *);
-  static JArray< ::gnu::classpath::jdwp::VMMethod * > * getAllClassMethods(::java::lang::Class *);
-  static ::gnu::classpath::jdwp::VMMethod * getClassMethod(::java::lang::Class *, jlong);
-  static ::java::util::ArrayList * getFrames(::java::lang::Thread *, jint, jint);
-  static ::gnu::classpath::jdwp::VMFrame * getFrame(::java::lang::Thread *, ::java::nio::ByteBuffer *);
-  static jint getFrameCount(::java::lang::Thread *);
-  static jint getThreadStatus(::java::lang::Thread *);
-  static ::java::util::ArrayList * getLoadRequests(::java::lang::ClassLoader *);
-  static ::gnu::classpath::jdwp::util::MethodResult * executeMethod(::java::lang::Object *, ::java::lang::Thread *, ::java::lang::Class *, ::java::lang::reflect::Method *, JArray< ::java::lang::Object * > *, jboolean);
-  static ::java::lang::String * getSourceFile(::java::lang::Class *);
-  static void registerEvent(::gnu::classpath::jdwp::event::EventRequest *);
-  static void unregisterEvent(::gnu::classpath::jdwp::event::EventRequest *);
-  static void clearEvents(jbyte);
+  VMVirtualMachine ();
+  static void initialize ();
+  static void suspendThread (::java::lang::Thread *);
+  static void suspendAllThreads ();
+  static void resumeThread (::java::lang::Thread *);
+  static void resumeAllThreads ();
+  static jint getSuspendCount (::java::lang::Thread *);
+  static jint getAllLoadedClassesCount ();
+  static ::java::util::Iterator *getAllLoadedClasses ();
+  static jint getClassStatus (::java::lang::Class *);
+  static JArray< ::gnu::classpath::jdwp::VMMethod *> *getAllClassMethods (::java::lang::Class *);
+  static ::gnu::classpath::jdwp::VMMethod *getClassMethod (::java::lang::Class *, jlong);
+  static ::java::util::ArrayList *getFrames (::java::lang::Thread *, jint, jint);
+  static ::gnu::classpath::jdwp::VMFrame *getFrame (::java::lang::Thread *, jlong);
+  static jint getFrameCount (::java::lang::Thread *);
+  static jint getThreadStatus (::java::lang::Thread *);
+  static ::java::util::ArrayList *getLoadRequests (::java::lang::ClassLoader *);
+  static ::gnu::classpath::jdwp::util::MethodResult *executeMethod (::java::lang::Object *, ::java::lang::Thread *, ::java::lang::Class *, ::java::lang::reflect::Method *, JArray< ::java::lang::Object *> *, jboolean);
+  static ::java::lang::String *getSourceFile (::java::lang::Class *);
+  static void registerEvent (::gnu::classpath::jdwp::event::EventRequest *);
+  static void unregisterEvent (::gnu::classpath::jdwp::event::EventRequest *);
+  static void clearEvents (jbyte);
 private:
-  static ::java::util::Hashtable * _jdwp_suspend_counts;
+  static ::java::util::Hashtable *_jdwp_suspend_counts;
 public:
+
   static ::java::lang::Class class$;
 };
 
-#endif // __gnu_classpath_jdwp_VMVirtualMachine__
+#endif /* __gnu_classpath_jdwp_VMVirtualMachine__ */
Index: libjava/gnu/classpath/jdwp/VMFrame.java
===================================================================
--- libjava/gnu/classpath/jdwp/VMFrame.java	(revision 121296)
+++ libjava/gnu/classpath/jdwp/VMFrame.java	(working copy)
@@ -54,7 +54,10 @@
    */
   public static final int SIZE = 8;
 
-  // The object this frame resides in
+  // The thread this frame resides in
+  private Thread thread;
+   
+  //The object of this frame
   private Object obj;
   
   // The current location of this frame
@@ -64,6 +67,20 @@
   private long id;
   
   /**
+   * Create a new VMFrame object.
+   * 
+   * @param thr a Thread, the thread this frame is in
+   * @param frame_id a long, the jframeID of this frame
+   * @param frame_loc a Location, the location of this frame
+   */
+  public VMFrame(Thread thr, long frame_id, Location frame_loc)
+  {
+    thread = thr;
+    id = frame_id;
+    loc = frame_loc;
+  }
+  
+  /**
    * Gets the current location of the frame.
    */
   public Location getLocation()
Index: libjava/gnu/classpath/jdwp/VMFrame.h
===================================================================
--- libjava/gnu/classpath/jdwp/VMFrame.h	(revision 121296)
+++ libjava/gnu/classpath/jdwp/VMFrame.h	(working copy)
@@ -29,7 +29,7 @@
 {
 
 public:
-  VMFrame();
+  VMFrame(::java::lang::Thread *, jlong, ::gnu::classpath::jdwp::util::Location *);
   virtual ::gnu::classpath::jdwp::util::Location * getLocation();
   virtual ::java::lang::Object * getValue(jint);
   virtual void setValue(jint, ::java::lang::Object *);
@@ -37,7 +37,8 @@
   virtual jlong getId();
   static const jint SIZE = 8;
 private:
-  ::java::lang::Object * __attribute__((aligned(__alignof__( ::java::lang::Object)))) obj;
+  ::java::lang::Thread * __attribute__((aligned(__alignof__( ::java::lang::Object)))) thread;
+  ::java::lang::Object * obj;
   ::gnu::classpath::jdwp::util::Location * loc;
   jlong id;
 public:
Index: libjava/gnu/classpath/jdwp/VMVirtualMachine.java
===================================================================
--- libjava/gnu/classpath/jdwp/VMVirtualMachine.java	(revision 121296)
+++ libjava/gnu/classpath/jdwp/VMVirtualMachine.java	(working copy)
@@ -240,10 +240,10 @@
    * I don't like this.
    *
    * @param  thread  the frame's thread
-   * @param  bb      buffer containing the frame's ID
+   * @param  id the frame's id
    * @return the desired frame
    */
-  public static native VMFrame getFrame (Thread thread, ByteBuffer bb)
+  public static native VMFrame getFrame (Thread thread, long id)
     throws JdwpException;
 
   /**
Index: libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(revision 121296)
+++ libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(working copy)
@@ -13,6 +13,8 @@
 #include <jvm.h>
 #include <jvmti.h>
 
+#include <java-interp.h>
+
 #include <java/lang/Class.h>
 #include <java/lang/ClassLoader.h>
 #include <java/lang/Integer.h>
@@ -20,6 +22,7 @@
 #include <java/lang/StringBuilder.h>
 #include <java/lang/Thread.h>
 #include <java/nio/ByteBuffer.h>
+#include <java/nio/ByteBufferImpl.h>
 #include <java/util/ArrayList.h>
 #include <java/util/Hashtable.h>
 #include <java/util/Iterator.h>
@@ -34,12 +37,17 @@
 #include <gnu/classpath/jdwp/event/ThreadStartEvent.h>
 #include <gnu/classpath/jdwp/event/VmDeathEvent.h>
 #include <gnu/classpath/jdwp/event/VmInitEvent.h>
+#include <gnu/classpath/jdwp/exception/InvalidFrameException.h>
 #include <gnu/classpath/jdwp/exception/InvalidMethodException.h>
+#include <gnu/classpath/jdwp/exception/JdwpIllegalArgumentException.h>
 #include <gnu/classpath/jdwp/exception/JdwpInternalErrorException.h>
+#include <gnu/classpath/jdwp/util/Location.h>
 #include <gnu/classpath/jdwp/util/MethodResult.h>
 
 using namespace java::lang;
+using namespace gnu::classpath::jdwp;
 using namespace gnu::classpath::jdwp::event;
+using namespace gnu::classpath::jdwp::exception;
 using namespace gnu::classpath::jdwp::util;
 
 // Forward declarations
@@ -355,21 +363,122 @@
 						   MAYBE_UNUSED jint start,
 						   MAYBE_UNUSED jint length)
 {
-  return NULL;
+  jint frame_count = getFrameCount (thread);
+  ::java::util::ArrayList *frame_list; 
+     
+  frame_list = new ::java::util::ArrayList (frame_count);
+  
+  _Jv_Frame *vm_frame = (_Jv_Frame *) thread->frame;
+  
+  while (vm_frame != NULL)
+    {
+      // Take start frames off the top of the stack.
+      if (start-- > 0)
+        continue;
+      
+      // Only return length frames.
+      if (length != -1 && length-- <= 0)
+        break; 
+      
+      jlong frameId = reinterpret_cast<jlong> (vm_frame);
+      
+      VMFrame *frame = getFrame (thread, frameId);
+      frame_list->add (frame);
+      vm_frame = vm_frame->next;
+    }
+  
+  return frame_list;
 }
 
 gnu::classpath::jdwp::VMFrame *
 gnu::classpath::jdwp::VMVirtualMachine::
-getFrame (MAYBE_UNUSED Thread *thread, MAYBE_UNUSED::java::nio::ByteBuffer *bb)
+getFrame (MAYBE_UNUSED Thread *thread, jlong id)
 {
-  return NULL;
+  _Jv_Frame *vm_frame = (_Jv_Frame *) thread->frame;
+  jint depth = 0;
+  _Jv_Frame *frameID = reinterpret_cast<_Jv_Frame *> (id); 
+  
+  // We need to find the stack depth of the frame, so search through the call
+  // stack to find it.  This also checks for a valid frameID.
+  while (vm_frame != frameID)
+    {
+      vm_frame = vm_frame->next;
+      depth++;
+      if (vm_frame == NULL)
+        throw new InvalidFrameException (reinterpret_cast<jlong> (frameID));
+    }
+  
+  Location *loc = NULL;
+  jvmtiFrameInfo info;
+  jvmtiError jerr;
+  jint num_frames;
+  jclass klass;
+  jthread thr = reinterpret_cast<jthread> (thread);
+  
+  // Get the info for the frame of interest
+  jerr = _jdwp_jvmtiEnv->GetStackTrace (thr, depth, 1, &info, &num_frames);
+   
+  if (jerr != JVMTI_ERROR_NONE)
+    {
+      using namespace gnu::classpath::jdwp::exception;
+      char *error;
+      _jdwp_jvmtiEnv->GetErrorName (jerr, &error);
+      String *msg = reinterpret_cast<String *> (JvNewStringUTF (error));
+      _jdwp_jvmtiEnv->Deallocate (reinterpret_cast<unsigned char *> (error));
+      
+      if (jerr == JVMTI_ERROR_ILLEGAL_ARGUMENT) 
+        throw new JdwpIllegalArgumentException (msg);
+      else
+        throw new JdwpInternalErrorException (msg);
+    }
+  
+  jerr = _jdwp_jvmtiEnv->GetMethodDeclaringClass (info.method, &klass);
+      
+  if (jerr != JVMTI_ERROR_NONE)
+    {
+      using namespace gnu::classpath::jdwp::exception;
+      char *error;
+      _jdwp_jvmtiEnv->GetErrorName (jerr, &error);
+      String *msg = reinterpret_cast<String *> (JvNewStringUTF (error));
+      _jdwp_jvmtiEnv->Deallocate (reinterpret_cast<unsigned char *> (error));
+      throw new JdwpInternalErrorException (msg);
+    }
+
+  VMMethod *meth 
+    = getClassMethod (klass, reinterpret_cast<jlong> (info.method));
+  
+  if (info.location == -1)
+    loc = new Location (meth, 0);
+  else
+    loc = new Location (meth, static_cast<jlong> (info.location));
+  
+  return new VMFrame (thread, reinterpret_cast<jlong> (vm_frame), loc); 
 }
 
 jint
 gnu::classpath::jdwp::VMVirtualMachine::
 getFrameCount (MAYBE_UNUSED Thread *thread)
 {
-  return 0;
+  jint frame_count;
+  jthread thr = reinterpret_cast<jthread> (thread);
+  
+  jvmtiError jerr = _jdwp_jvmtiEnv->GetFrameCount (thr, &frame_count);
+  
+  if (jerr != JVMTI_ERROR_NONE)
+    {
+      using namespace gnu::classpath::jdwp::exception;
+      char *error;
+      _jdwp_jvmtiEnv->GetErrorName (jerr, &error);
+      String *msg = reinterpret_cast<String *> (JvNewStringUTF (error));
+      _jdwp_jvmtiEnv->Deallocate (reinterpret_cast<unsigned char *> (error));
+      
+      if (jerr == JVMTI_ERROR_ILLEGAL_ARGUMENT) 
+        throw new JdwpIllegalArgumentException (msg);
+      else
+        throw new JdwpInternalErrorException (msg);
+    }
+  
+  return frame_count; 
 }
 
 jint

Re: [RFA] JDWP implment getFrames, getFrame, getFrameCount

[Keith Seitz]

Kyle Galloway wrote:

> Comments, questions?

I'm not a maintainer, but since I am quite involved in this code, I 
would really, really appreciate it if you could break this into smaller 
patches. It just makes it easier (and therefore faster) to parse. [Hey, 
I'm getting old! :-)]

[And you need to submit the classpath changes upstream to 
classpath-patches@gnu.org first.]

Keith

[RFA] JDWP implment getFrameCount

[Kyle Galloway]

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

Keith Seitz wrote:
> Kyle Galloway wrote:
>
>> Comments, questions?
>
> I'm not a maintainer, but since I am quite involved in this code, I 
> would really, really appreciate it if you could break this into 
> smaller patches. It just makes it easier (and therefore faster) to 
> parse. [Hey, I'm getting old! :-)]
>
> [And you need to submit the classpath changes upstream to 
> classpath-patches@gnu.org first.]
As requested, I have submitted the classpath changes, and once they are 
approved will submit the getFrame and getFrames patches in separate 
chunks.  For now, getFrameCount involves no classpath changes so I have 
started with this.

This patch implements the JDWP method getFrameCount from 
natVMVirtualMachine.cc.  It calls jvmti->GetFrameCount then returns the 
result.

Changelog

2007-02-01  Kyle Galloway  <kgallowa@redhat.com>

    * gnu/classpath/jdwp/natVMVirtualMachine.cc (getFrameCount): Implment.

Comments?

- Kyle


[-- Attachment #2: getframecount-jdwp.patch --]
[-- Type: text/x-patch, Size: 709 bytes --]

Index: /notnfs/kgallowa/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- /notnfs/kgallowa/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(revision 121464)
+++ /notnfs/kgallowa/gcc/libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(working copy)
@@ -369,7 +369,15 @@
 gnu::classpath::jdwp::VMVirtualMachine::
 getFrameCount (MAYBE_UNUSED Thread *thread)
 {
-  return 0;
+  jint frame_count;
+  jthread thr = reinterpret_cast<jthread> (thread);
+  
+  jvmtiError jerr = _jdwp_jvmtiEnv->GetFrameCount (thr, &frame_count);
+  
+  if (jerr != JVMTI_ERROR_NONE)
+    throw_jvmti_error (jerr);
+  
+  return frame_count;
 }
 
 jint

Re: [RFA] JDWP implment getFrameCount

[Tom Tromey]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> This patch implements the JDWP method getFrameCount from
Kyle> natVMVirtualMachine.cc.  It calls jvmti->GetFrameCount then returns
Kyle> the result.

Kyle>  gnu::classpath::jdwp::VMVirtualMachine::
Kyle>  getFrameCount (MAYBE_UNUSED Thread *thread)

Remove the MAYBE_UNUSED.

Kyle> +  jthread thr = reinterpret_cast<jthread> (thread);

I think you shouldn't need this cast any more.

Ok with those changes.  Assuming I'm right about the cast :)

Tom

Re: [RFA] JDWP implment getFrameCount

[Kyle Galloway]

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
> Kyle> This patch implements the JDWP method getFrameCount from
> Kyle> natVMVirtualMachine.cc.  It calls jvmti->GetFrameCount then returns
> Kyle> the result.
>
> Kyle>  gnu::classpath::jdwp::VMVirtualMachine::
> Kyle>  getFrameCount (MAYBE_UNUSED Thread *thread)
>
> Remove the MAYBE_UNUSED.
>
> Kyle> +  jthread thr = reinterpret_cast<jthread> (thread);
>
> I think you shouldn't need this cast any more.
>
> Ok with those changes.  Assuming I'm right about the cast :)
>   
I've made those changes and committed.  You are correct, a jthread is 
now defined as a java::lang::Thread* so the cast was just creating 
another identical pointer to the same object.

Thanks,
Kyle

[RFA] JDWP implement VMVM::getFrame

[Kyle Galloway]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

Hi,

The GNU Classpath changes for this patch have now gone in (see 
cp-commits as of this morning), this is part 2 of 3 of the large patch 
from before.

This patch implements the JDWP method getFrame from
natVMVirtualMachine.cc.  This method calls jvmti-GetStackTrace to get 
info about 1 particular frame, then creates a new VMFrame and returns 
it.  This was changed to now take a jlong, which saves having to pack a 
ByteBuffer to use this method from getFrames.  VMFrame has also been 
augmented since it was intentionally left unfinshed until the specifics 
of stack tracing were determined.

Changelog

2007-02-02  Kyle Galloway  <kgallowa@redhat.com>

     * classpath/gnu/classpath/jdwp/processor/
     StackFrameCommandSet.java (executeGetValues): Pass jlong instead
     of ByteBuffer.
     (executeSetValues): Ditto.
     (executeThisObject): Ditto.
     * classpath/gnu/classpath/jdwp/processor/
     StackFrameCommandSet.class: Rebuilt.
     * classpath/lib/gnu/classpath/jdwp/VMVirtualMachine.class:
     Rebuilt.
     * classpath/lib/gnu/classpath/jdwp/VMFrame.class: Rebuilt.
     * classpath/lib/gnu/classpath/jdwp/exception/
     InvalidFrameException.java: New file.
     * gnu/classpath/jdwp/VMFrame.java: Added field for thread of
     frame.
     (Constructor): New method.
     * gnu/classpath/jdwp/VMFrame.h: Regenerated.
     * gnu/classpath/jdwp/VMVirtualMachine.java
     (getFrame): Changed ByteBuffer to jlong.
     * gnu/classpath/jdwp/natVMVirtualMachine.cc
     (getFrame): Implment.
     * gnu/classpath/jdwp/VMVirtualMachine.h: Regenerated.


Comments?

- Kyle



[-- Attachment #2: getframe-jdwp.patch --]
[-- Type: text/x-patch, Size: 9726 bytes --]

Index: libjava/classpath/gnu/classpath/jdwp/processor/StackFrameCommandSet.java
===================================================================
--- libjava/classpath/gnu/classpath/jdwp/processor/StackFrameCommandSet.java	(revision 121296)
+++ libjava/classpath/gnu/classpath/jdwp/processor/StackFrameCommandSet.java	(working copy)
@@ -1,5 +1,5 @@
 /* StackFrameCommandSet.java -- class to implement the StackFrame Command Set
-   Copyright (C) 2005 Free Software Foundation
+   Copyright (C) 2005, 2007 Free Software Foundation
  
 This file is part of GNU Classpath.
 
@@ -107,7 +107,8 @@
     // has a reference to them. Furthermore they are not ReferenceTypeIds since
     // these are held permanently and we want these to be held only as long as
     // the Thread is suspended.
-    VMFrame frame = VMVirtualMachine.getFrame(thread, bb);
+    long frameID = bb.getLong();
+    VMFrame frame = VMVirtualMachine.getFrame(thread, frameID);
     int slots = bb.getInt();
     os.writeInt(slots); // Looks pointless but this is the protocol
     for (int i = 0; i < slots; i++)
@@ -125,7 +126,8 @@
     ObjectId tId = idMan.readObjectId(bb);
     Thread thread = (Thread) tId.getObject();
 
-    VMFrame frame = VMVirtualMachine.getFrame(thread, bb);
+    long frameID = bb.getLong();
+    VMFrame frame = VMVirtualMachine.getFrame(thread, frameID);
 
     int slots = bb.getInt();
     for (int i = 0; i < slots; i++)
@@ -142,7 +144,8 @@
     ObjectId tId = idMan.readObjectId(bb);
     Thread thread = (Thread) tId.getObject();
 
-    VMFrame frame = VMVirtualMachine.getFrame(thread, bb);
+    long frameID = bb.getLong();
+    VMFrame frame = VMVirtualMachine.getFrame(thread, frameID);
 
     Object thisObject = frame.getObject();
     Value.writeTaggedValue(os, thisObject);
Index: libjava/classpath/gnu/classpath/jdwp/exception/InvalidFrameException.java
===================================================================
--- libjava/classpath/gnu/classpath/jdwp/exception/InvalidFrameException.java	(revision 0)
+++ libjava/classpath/gnu/classpath/jdwp/exception/InvalidFrameException.java	(revision 0)
@@ -0,0 +1,63 @@
+/* InvalidFrameException.java -- invalid jframeid
+   Copyright (C) 2007 Free Software Foundation
+
+This file is part of GNU Classpath.
+
+GNU Classpath is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2, or (at your option)
+any later version.
+
+GNU Classpath is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Classpath; see the file COPYING.  If not, write to the
+Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version. */
+
+
+package gnu.classpath.jdwp.exception;
+
+import gnu.classpath.jdwp.JdwpConstants;
+
+/**
+ * An exception thrown when the debugger requests an invalid frame in the call
+ * stack.
+ *
+ * @author Kyle Galloway  (kgallowa@redhat.com)
+ */
+public class InvalidFrameException
+  extends JdwpException
+{
+  public InvalidFrameException(long id)
+  {
+    super(JdwpConstants.Error.INVALID_FRAMEID, 
+		  "invalid frame id (" + id + ")");
+  }
+
+  public InvalidFrameException(String msg)
+  {
+    super(JdwpConstants.Error.INVALID_FRAMEID, msg);
+  }
+}
Index: libjava/gnu/classpath/jdwp/VMFrame.java
===================================================================
--- libjava/gnu/classpath/jdwp/VMFrame.java	(revision 121464)
+++ libjava/gnu/classpath/jdwp/VMFrame.java	(working copy)
@@ -1,5 +1,5 @@
 /* VMFrame.java -- Reference implementation of VM hooks for JDWP Frame access.
-   Copyright (C) 2005, 2006 Free Software Foundation
+   Copyright (C) 2005, 2006, 2007 Free Software Foundation
 
 This file is part of GNU Classpath.
 
@@ -54,7 +54,10 @@
    */
   public static final int SIZE = 8;
 
-  // The object this frame resides in
+  // The thread this frame resides in
+  private Thread thread;
+   
+  //The object of this frame
   private Object obj;
   
   // The current location of this frame
@@ -64,6 +67,20 @@
   private long id;
   
   /**
+   * Create a new VMFrame object.
+   * 
+   * @param thr a Thread, the thread this frame is in
+   * @param frame_id a long, the jframeID of this frame
+   * @param frame_loc a Location, the location of this frame
+   */
+  public VMFrame(Thread thr, long frame_id, Location frame_loc)
+  {
+    thread = thr;
+    id = frame_id;
+    loc = frame_loc;
+  }
+  
+  /**
    * Gets the current location of the frame.
    */
   public Location getLocation()
@@ -84,6 +101,14 @@
    * @param value The value to assign the variable to
    */
   public native void setValue(int slot, Object value);
+  
+  /**
+   * Get the thread this frame is in.
+   */
+  public Thread getThread()
+  {
+    return thread;
+  }
 
   /**
    * Get the object which is represented by 'this' in the context of the frame,
Index: libjava/gnu/classpath/jdwp/VMVirtualMachine.java
===================================================================
--- libjava/gnu/classpath/jdwp/VMVirtualMachine.java	(revision 121464)
+++ libjava/gnu/classpath/jdwp/VMVirtualMachine.java	(working copy)
@@ -1,7 +1,7 @@
 /* VMVirtualMachine.java -- A reference implementation of a JDWP virtual
    machine
 
-   Copyright (C) 2005, 2006 Free Software Foundation
+   Copyright (C) 2005, 2006, 2007 Free Software Foundation
 
 This file is part of GNU Classpath.
 
@@ -243,7 +243,7 @@
    * @param  bb      buffer containing the frame's ID
    * @return the desired frame
    */
-  public static native VMFrame getFrame (Thread thread, ByteBuffer bb)
+  public static native VMFrame getFrame (Thread thread, long frameID)
     throws JdwpException;
 
   /**
Index: libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(revision 121504)
+++ libjava/gnu/classpath/jdwp/natVMVirtualMachine.cc	(working copy)
@@ -13,6 +13,8 @@
 #include <jvm.h>
 #include <jvmti.h>
 
+#include <java-interp.h>
+
 #include <java/lang/Class.h>
 #include <java/lang/ClassLoader.h>
 #include <java/lang/Integer.h>
@@ -20,6 +22,7 @@
 #include <java/lang/StringBuilder.h>
 #include <java/lang/Thread.h>
 #include <java/nio/ByteBuffer.h>
+#include <java/nio/ByteBufferImpl.h>
 #include <java/util/ArrayList.h>
 #include <java/util/Hashtable.h>
 #include <java/util/Iterator.h>
@@ -34,8 +37,10 @@
 #include <gnu/classpath/jdwp/event/ThreadStartEvent.h>
 #include <gnu/classpath/jdwp/event/VmDeathEvent.h>
 #include <gnu/classpath/jdwp/event/VmInitEvent.h>
+#include <gnu/classpath/jdwp/exception/InvalidFrameException.h>
 #include <gnu/classpath/jdwp/exception/InvalidMethodException.h>
 #include <gnu/classpath/jdwp/exception/JdwpInternalErrorException.h>
+#include <gnu/classpath/jdwp/util/Location.h>
 #include <gnu/classpath/jdwp/util/MethodResult.h>
 
 using namespace java::lang;
@@ -360,9 +365,50 @@
 
 gnu::classpath::jdwp::VMFrame *
 gnu::classpath::jdwp::VMVirtualMachine::
-getFrame (MAYBE_UNUSED Thread *thread, MAYBE_UNUSED::java::nio::ByteBuffer *bb)
+getFrame (Thread *thread, jlong frameID)
 {
-  return NULL;
+  using namespace gnu::classpath::jdwp::exception;
+  
+  _Jv_Frame *vm_frame = (_Jv_Frame *) thread->frame;
+  jint depth = 0;
+  _Jv_Frame *frame = reinterpret_cast<_Jv_Frame *> (frameID); 
+  
+  // We need to find the stack depth of the frame, so search through the call
+  // stack to find it.  This also checks for a valid frameID.
+  while (vm_frame != frame)
+    {
+      vm_frame = vm_frame->next;
+      depth++;
+      if (vm_frame == NULL)
+        throw new InvalidFrameException (frameID);
+    }
+  
+  Location *loc = NULL;
+  jvmtiFrameInfo info;
+  jvmtiError jerr;
+  jint num_frames;
+  jclass klass;
+  
+  // Get the info for the frame of interest
+  jerr = _jdwp_jvmtiEnv->GetStackTrace (thread, depth, 1, &info, &num_frames);
+   
+  if (jerr != JVMTI_ERROR_NONE)
+    throw_jvmti_error (jerr);
+  
+  jerr = _jdwp_jvmtiEnv->GetMethodDeclaringClass (info.method, &klass);
+      
+  if (jerr != JVMTI_ERROR_NONE)
+    throw_jvmti_error (jerr);
+
+  VMMethod *meth 
+    = getClassMethod (klass, reinterpret_cast<jlong> (info.method));
+  
+  if (info.location == -1)
+    loc = new Location (meth, 0);
+  else
+    loc = new Location (meth, static_cast<jlong> (info.location));
+  
+  return new VMFrame (thread, reinterpret_cast<jlong> (vm_frame), loc); 
 }
 
 jint

Re: [RFA] JDWP implement VMVM::getFrame

[Tom Tromey]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> This patch implements the JDWP method getFrame from
Kyle> natVMVirtualMachine.cc.

Looks good.  Just one nit...

Kyle> +    loc = new Location (meth, static_cast<jlong> (info.location));

By my reading, info.location is a jlong, so we shouldn't need this
cast.

Assuming I'm right, this is ok with that change.
And if I'm wrong, it is ok without it :)

Tom

Re: [RFA] JDWP implement VMVM::getFrame

[Kyle Galloway]

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
> Kyle> This patch implements the JDWP method getFrame from
> Kyle> natVMVirtualMachine.cc.
>
> Looks good.  Just one nit...
>
> Kyle> +    loc = new Location (meth, static_cast<jlong> (info.location));
>
> By my reading, info.location is a jlong, so we shouldn't need this
> cast.
>
> Assuming I'm right, this is ok with that change.
> And if I'm wrong, it is ok without it :)
>   
You are correct, I have removed the cast and committed.

- Kyle