[RFA] JMTI Exception Events

[RFA] JMTI Exception Events

[Kyle Galloway]

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

I've had one or another variation of this lying around for some time, 
but I have finalized it.  This patch adds notification of JVMTI 
Exception Events.  In the catch section of the debug interpreter, it 
grabs information about the exception, then creates a new ExceptionEvent 
object.  This object handles checking if the exception has already 
generated an event, as well as whether or not the exception will be 
caught.  It then calls it's native side to dispatch the method.

In order to clean up the code in the catch block of the interpreter, I 
added check_handler, which does exactly what was done before, but makes 
things alot cleaner and allows me to call check_handler to see if an 
exception will be caught further up the stack.

Before I commit this, I wanted to clear up which files I should commit. 
  Do I need to include more than sources.am and Makefile.am?  For 
example, should the changed Makefile.ins go in as well?

Questions/comments/concerns?

Thanks,
Kyle

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

	* interpret.cc (_Jv_InterpMethod::check_handler): New method.
	* interpret-run.cc: Change the catch section to report exception
	events and to use the new check_handler method.
	* include/java-interp.h (_Jv_InterpMethod): Add check_handler.
	* gnu/gcj/jvmti/ExceptionEvent.java: New file.
	* gnu/gcj/jvmti/ExceptionEvent.h: New file.
	* gnu/gcj/jvmti/natExceptionEvent.cc: New file.
	* libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class: New
	file.
	* sources.am: Added ExceptionEvent.java.
	* Makefile.am: Added natExceptionEvent.cc



[-- Attachment #2: exceptions-jvmti.patch --]
[-- Type: text/x-patch, Size: 12603 bytes --]

Index: libjava/interpret.cc
===================================================================
--- libjava/interpret.cc	(revision 121856)
+++ libjava/interpret.cc	(working copy)
@@ -43,6 +43,7 @@
 #include <gnu/classpath/jdwp/Jdwp.h>
 #include <gnu/gcj/jvmti/Breakpoint.h>
 #include <gnu/gcj/jvmti/BreakpointManager.h>
+#include <gnu/gcj/jvmti/ExceptionEvent.h>
 
 #ifdef INTERPRETER
 
@@ -1366,6 +1367,49 @@
   return -1;
 }
 
+// Method to check if an exception is hadled at some location (pc) in a method
+// (meth).  It then sets the pc to the start of the handler
+int
+_Jv_InterpMethod::check_handler (pc_t *pc, _Jv_InterpMethod *meth,
+                                java::lang::Throwable *ex)
+{
+#ifdef DIRECT_THREADED
+  void *logical_pc = (void *) ((insn_slot *) (*pc) - 1);
+#else
+  int logical_pc = (*pc) - 1 - meth->bytecode ();
+#endif
+  _Jv_InterpException *exc = meth->exceptions ();
+  jclass exc_class = ex->getClass ();
+
+  for (int i = 0; i < meth->exc_count; i++)
+    {
+      if (PCVAL (exc[i].start_pc) <= logical_pc
+          && logical_pc < PCVAL (exc[i].end_pc))
+        {
+#ifdef DIRECT_THREADED
+              jclass handler = (jclass) exc[i].handler_type.p;
+#else
+              jclass handler = NULL;
+              if (exc[i].handler_type.i != 0)
+                    handler
+                      = (_Jv_Linker::resolve_pool_entry (meth->defining_class,
+                                                                             ex$
+#endif /* DIRECT_THREADED */
+              if (handler == NULL || handler->isAssignableFrom (exc_class))
+                {
+#ifdef DIRECT_THREADED
+                  (*pc) = (insn_slot *) exc[i].handler_pc.p;
+#else
+                  (*pc) = meth->bytecode () + exc[i].handler_pc.i;
+#endif /* DIRECT_THREADED */
+                  return true;
+                }
+          }
+      }
+return false;
+}
+
+
 void
 _Jv_InterpMethod::get_line_table (jlong& start, jlong& end,
 				  jintArray& line_numbers,
Index: libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream

Property changes on: libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class
___________________________________________________________________
Name: svn:mime-type
   + application/octet-stream

Index: libjava/include/java-interp.h
===================================================================
--- libjava/include/java-interp.h	(revision 121856)
+++ libjava/include/java-interp.h	(working copy)
@@ -210,6 +210,11 @@
   // Convenience function for indexing bytecode PC/insn slots in
   // line tables for JDWP
   jlong insn_index (pc_t pc);
+  
+  // Helper function used to check if there is a handler for an exception
+  // present at this code index
+  int check_handler (pc_t *pc, _Jv_InterpMethod *meth,
+                     java::lang::Throwable *ex);
    
   /* Get the line table for this method.
    * start  is the lowest index in the method
Index: libjava/gnu/gcj/jvmti/ExceptionEvent.java
===================================================================
--- libjava/gnu/gcj/jvmti/ExceptionEvent.java	(revision 0)
+++ libjava/gnu/gcj/jvmti/ExceptionEvent.java	(revision 0)
@@ -0,0 +1,85 @@
+// ExceptionEvent - an exception event for JVMTI
+
+/* Copyright (C) 2006  Free Software Foundation
+
+   This file is part of libgcj.
+
+This software is copyrighted work licensed under the terms of the
+Libgcj License.  Please consult the file "LIBGCJ_LICENSE" for
+details.  */
+
+package gnu.gcj.jvmti;
+
+import java.util.WeakHashMap;
+
+/**
+ * Class to create and send JVMTI Exception events
+ *
+ * @author Kyle Galloway (kgallowa@redhat.com)
+ */
+public class ExceptionEvent
+{
+  // Information about where the exception was thrown
+  private long _throw_meth, _throw_loc;
+  
+  // Information about where the exception was or can be caught
+  private long _catch_meth, _catch_loc;
+  
+  // Thread where the exception occurred
+  private Thread _thread;
+  
+  // The exception
+  private Throwable _ex;
+  
+  // A hash map of the exceptions we've already seen in a thread's call stack
+  private static WeakHashMap<Thread, Throwable> _ex_map;
+  
+  /**
+   * Constructs a new ExceptionEvent and sends it.  If it is not caught
+   * within the frame where it was thrown (catch_meth and catch_loc are null),
+   * check_catch will check for a possible catch further up the call stack 
+   * before marking it uncaught.
+   * 
+   * @param thr the thread where the exception occurred
+   * @param throw_meth the method of the throw (a jmethodID)
+   * @param throw_loc the location of the throw (a jlocation)
+   * @param ex the exception
+   * @param catch_meth the method of the catch (a jmethodID), null indicates
+   * that the exception was not caught in the frame where it was thrown
+   * @param catch_loc the location of the catch (a jlocation), null indicates
+   * that the exception was not caught in the frame where it was thrown
+   */
+  public ExceptionEvent (Thread thr, long throw_meth, long throw_loc,
+		                       Throwable ex, long catch_meth, long catch_loc)
+  {
+    _thread = thr;
+    _ex = ex;
+    _throw_meth = throw_meth;
+    _throw_loc = throw_loc;
+    _catch_meth = catch_meth;
+    _catch_loc = catch_loc;
+    
+    if (_ex_map == null)
+      _ex_map = new WeakHashMap ();
+    
+    // Check if we have already seen this exception in this thread, if not send
+    // a new event and update the HashMap.
+    if (_ex_map.containsKey (thr))
+      {
+        if (!(_ex_map.get (thr).equals (ex)))
+          {
+        	_ex_map.put (thr, ex);
+            send_event ();
+          }
+      }
+    else
+      {
+        _ex_map.put (thr, ex);
+        send_event ();
+      }
+  }
+  
+  public native void send_event ();
+  
+  public native void check_catch ();
+}
Index: libjava/gnu/gcj/jvmti/ExceptionEvent.h
===================================================================
--- libjava/gnu/gcj/jvmti/ExceptionEvent.h	(revision 0)
+++ libjava/gnu/gcj/jvmti/ExceptionEvent.h	(revision 0)
@@ -0,0 +1,43 @@
+// DO NOT EDIT THIS FILE - it is machine generated -*- c++ -*-
+
+#ifndef __gnu_gcj_jvmti_ExceptionEvent__
+#define __gnu_gcj_jvmti_ExceptionEvent__
+
+#pragma interface
+
+#include <java/lang/Object.h>
+
+extern "Java"
+{
+  namespace gnu
+  {
+    namespace gcj
+    {
+      namespace jvmti
+      {
+        class ExceptionEvent;
+      }
+    }
+  }
+}
+
+class gnu::gcj::jvmti::ExceptionEvent : public ::java::lang::Object
+{
+public:
+  ExceptionEvent (::java::lang::Thread *, jlong, jlong, ::java::lang::Throwable *, jlong, jlong);
+  virtual void send_event ();
+  virtual void check_catch ();
+private:
+  jlong __attribute__((aligned(__alignof__( ::java::lang::Object ))))  _throw_meth;
+  jlong _throw_loc;
+  jlong _catch_meth;
+  jlong _catch_loc;
+  ::java::lang::Thread *_thread;
+  ::java::lang::Throwable *_ex;
+  static ::java::util::WeakHashMap *_ex_map;
+public:
+
+  static ::java::lang::Class class$;
+};
+
+#endif /* __gnu_gcj_jvmti_ExceptionEvent__ */
Index: libjava/gnu/gcj/jvmti/natExceptionEvent.cc
===================================================================
--- libjava/gnu/gcj/jvmti/natExceptionEvent.cc	(revision 0)
+++ libjava/gnu/gcj/jvmti/natExceptionEvent.cc	(revision 0)
@@ -0,0 +1,59 @@
+// natExceptionEvent.cc - C++ code for JVMTI Exception events
+
+/* Copyright (C) 2006  Free Software Foundation
+
+   This file is part of libgcj.
+
+This software is copyrighted work licensed under the terms of the
+Libgcj License.  Please consult the file "LIBGCJ_LICENSE" for
+details.  */
+
+#include <config.h>
+#include <gcj/cni.h>
+#include <gcj/method.h>
+#include <java-interp.h>
+#include <java-insns.h>
+#include <java-assert.h>
+#include <jvmti.h>
+#include <jvmti-int.h>
+
+#include <gnu/gcj/jvmti/ExceptionEvent.h>
+
+void
+gnu::gcj::jvmti::ExceptionEvent::send_event ()
+{
+  // Check if the exception is caught somewhere in the interpreted call stack
+  if (_catch_meth == 0 || _catch_loc == 0)
+    check_catch ();
+    
+  JNIEnv *jni = _Jv_GetCurrentJNIEnv ();
+
+  _Jv_JVMTI_PostEvent (JVMTI_EVENT_EXCEPTION, _thread, jni,
+                       reinterpret_cast<jmethodID> (_throw_meth),
+                       static_cast<jlocation> (_throw_loc), _ex,
+                       reinterpret_cast<jmethodID> (_catch_meth),
+                       static_cast<jlocation> (_catch_loc)); 
+}
+
+// This method looks up the interpreted call stack to see if the excetion will
+// eventually be caught by some java method
+void
+gnu::gcj::jvmti::ExceptionEvent::check_catch ()
+{
+  _Jv_InterpFrame *frame 
+    = reinterpret_cast<_Jv_InterpFrame *> (_thread->interp_frame);
+  
+  while ((frame=frame->next_interp))
+    {
+	  _Jv_InterpMethod *meth 
+	    = reinterpret_cast<_Jv_InterpMethod *> (frame->self);
+	  pc_t pc = frame->pc;
+		
+	  if (meth->check_handler (&pc, meth, _ex))
+	    {
+	      _catch_meth = reinterpret_cast<jlong> (meth->get_method ());
+	      _catch_loc = meth->insn_index (pc);
+          break;
+	    }
+    }
+}
Index: libjava/interpret-run.cc
===================================================================
--- libjava/interpret-run.cc	(revision 121856)
+++ libjava/interpret-run.cc	(working copy)
@@ -2540,43 +2540,37 @@
     }
   catch (java::lang::Throwable *ex)
     {
-#ifdef DIRECT_THREADED
-      void *logical_pc = (void *) ((insn_slot *) pc - 1);
-#else
-      int logical_pc = pc - 1 - meth->bytecode ();
+#ifdef DEBUG
+       // This needs to be done before the pc is changed.
+       jlong throw_loc = meth->insn_index (pc);
 #endif
-      _Jv_InterpException *exc = meth->exceptions ();
-      jclass exc_class = ex->getClass ();
-
-      for (int i = 0; i < meth->exc_count; i++)
-	{
-	  if (PCVAL (exc[i].start_pc) <= logical_pc
-	      && logical_pc < PCVAL (exc[i].end_pc))
-	    {
-#ifdef DIRECT_THREADED
-	      jclass handler = (jclass) exc[i].handler_type.p;
-#else
-	      jclass handler = NULL;
-	      if (exc[i].handler_type.i != 0)
-		handler = (_Jv_Linker::resolve_pool_entry (meth->defining_class,
-							   exc[i].handler_type.i)).clazz;
-#endif /* DIRECT_THREADED */
-
-	      if (handler == NULL || handler->isAssignableFrom (exc_class))
-		{
-
-#ifdef DIRECT_THREADED
-		  pc = (insn_slot *) exc[i].handler_pc.p;
-#else
-		  pc = meth->bytecode () + exc[i].handler_pc.i;
-#endif /* DIRECT_THREADED */
-		  sp = stack;
-		  sp++->o = ex; // Push exception.
-		  NEXT_INSN;
-		}
-	    }
-	}
-
+      // Check if the exception is handled and, if so, set the pc to the start
+      // of the appropriate catch block.
+      if (meth->check_handler (&pc, meth, ex))
+        {
+          sp = stack;
+          sp++->o = ex; // Push exception.
+#ifdef DEBUG
+          if (JVMTI_REQUESTED_EVENT (Exception))
+            {
+              using namespace gnu::gcj::jvmti;
+              jlong throw_meth = reinterpret_cast<jlong> (meth->get_method ());
+              jlong catch_loc = meth->insn_index (pc);
+              new ExceptionEvent (thread, throw_meth, throw_loc,
+                                           ex, throw_meth, catch_loc);
+            }
+#endif
+          NEXT_INSN;
+        }
+#ifdef DEBUG
+      if (JVMTI_REQUESTED_EVENT (Exception))
+        {
+          using namespace gnu::gcj::jvmti;
+          jlong throw_meth = reinterpret_cast<jlong> (meth->get_method ());
+          new ExceptionEvent (thread, throw_meth, throw_loc,
+                                           ex, NULL, NULL);
+        }
+#endif
       // No handler, so re-throw.
       throw ex;
     }
Index: libjava/sources.am
===================================================================
--- libjava/sources.am	(revision 121856)
+++ libjava/sources.am	(working copy)
@@ -509,6 +509,7 @@
 gnu_gcj_jvmti_source_files = \
 gnu/gcj/jvmti/Breakpoint.java \
 gnu/gcj/jvmti/BreakpointManager.java \
+gnu/gcj/jvmti/ExceptionEvent.java \
 gnu/gcj/jvmti/Location.java
 
 gnu_gcj_jvmti_header_files = $(patsubst %.java,%.h,$(gnu_gcj_jvmti_source_files))
Index: libjava/Makefile.am
===================================================================
--- libjava/Makefile.am	(revision 121856)
+++ libjava/Makefile.am	(working copy)
@@ -826,6 +826,7 @@
 gnu/gcj/io/natSimpleSHSStream.cc \
 gnu/gcj/io/shs.cc \
 gnu/gcj/jvmti/natBreakpoint.cc \
+gnu/gcj/jvmti/natExceptionEvent.cc \
 gnu/gcj/runtime/natFinalizerThread.cc \
 gnu/gcj/runtime/natSharedLibLoader.cc \
 gnu/gcj/runtime/natSystemClassLoader.cc \

Re: [RFA] JMTI Exception Events

[Tom Tromey]

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

Kyle> Before I commit this, I wanted to clear up which files I should
Kyle> commit. Do I need to include more than sources.am and Makefile.am?
Kyle> For example, should the changed Makefile.ins go in as well?

Yes, commit everything that is changed.

Kyle> Questions/comments/concerns?

A few minor things, but one or two substantive ones as well.  Read on..

Kyle> +// Method to check if an exception is hadled at some location (pc) in a method

Typo: "handled".

Kyle> +// (meth).  It then sets the pc to the start of the handler
Kyle> +int
Kyle> +_Jv_InterpMethod::check_handler (pc_t *pc, _Jv_InterpMethod *meth,
Kyle> +                                java::lang::Throwable *ex)

This should be declared as returning bool or jboolean, not int.
The meaning of the return value should be documented in the comment.

Kyle> +return false;

Indentation is incorrect.

Kyle> +  private static WeakHashMap<Thread, Throwable> _ex_map;

I think you probably want to initialize this here, rather than in the
ExceptionEvent constructor.  Doing it in the constructor means that
there is a race.

Static fields are initialized when the class is initialized -- so it
is already done lazily.

Kyle> +      _ex_map = new WeakHashMap ();

... when you do this I think it is preferable to write:

    .. = new WeakHashMap<Thread, Throwable> ();

Kyle> +        if (!(_ex_map.get (thr).equals (ex)))

Without an equals() method in this class, I doubt this does what you
intend.  As it is this equals() will always return false, because the
default equals() tests object identity, and a new object is never
identical to any existing one.

Kyle> +            send_event ();

It is a bit odd to have a side-effect-ing constructor.  I'd say it is
a bit more idiomatic to have a static method to dispatch the event,
which creates the event object as needed.  Seeing a raw 'new ...' as a
statement is kind of strange.

Kyle> +// This method looks up the interpreted call stack to see if the excetion will

Typo: "exception".

Tom

Re: [RFA] JMTI Exception Events

[Kyle Galloway]

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

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
> Kyle> Before I commit this, I wanted to clear up which files I should
> Kyle> commit. Do I need to include more than sources.am and Makefile.am?
> Kyle> For example, should the changed Makefile.ins go in as well?
>
> Yes, commit everything that is changed.
>
> Kyle> Questions/comments/concerns?
>
> A few minor things, but one or two substantive ones as well.  Read on..
>
> Kyle> +// Method to check if an exception is hadled at some location (pc) in a method
>
> Typo: "handled".
>   
Fixed.
> Kyle> +// (meth).  It then sets the pc to the start of the handler
> Kyle> +int
> Kyle> +_Jv_InterpMethod::check_handler (pc_t *pc, _Jv_InterpMethod *meth,
> Kyle> +                                java::lang::Throwable *ex)
>
> This should be declared as returning bool or jboolean, not int.
> The meaning of the return value should be documented in the comment.
>
> Kyle> +return false;
>
> Indentation is incorrect.
>   
Fixed.
> Kyle> +  private static WeakHashMap<Thread, Throwable> _ex_map;
>
> I think you probably want to initialize this here, rather than in the
> ExceptionEvent constructor.  Doing it in the constructor means that
> there is a race.
>
> Static fields are initialized when the class is initialized -- so it
> is already done lazily.
>
> Kyle> +      _ex_map = new WeakHashMap ();
>
> ... when you do this I think it is preferable to write:
>
>     .. = new WeakHashMap<Thread, Throwable> ();
>   
I've changed that.
> Kyle> +        if (!(_ex_map.get (thr).equals (ex)))
>
> Without an equals() method in this class, I doubt this does what you
> intend.  As it is this equals() will always return false, because the
> default equals() tests object identity, and a new object is never
> identical to any existing one.
>   
In this case, however, I believe I want to only equal the exact same 
exception. If an exception is not caught in a method, that method is 
popped off the stack and the exception is pushed. Then the next method 
will hit the catch block of the interpreter with the same Exception 
object. What I'm trying to check is if it is the same exception that 
triggered a previous event. When this check fails, it means that the 
exception was caught and another exception was thrown in this thread, 
which should trigger a new event. Now, am I wrong in thinking that an 
exception thrown to the caller of a method is the same exception, does a 
new Exception object get created?
> Kyle> +            send_event ();
>
> It is a bit odd to have a side-effect-ing constructor.  I'd say it is
> a bit more idiomatic to have a static method to dispatch the event,
> which creates the event object as needed.  Seeing a raw 'new ...' as a
> statement is kind of strange.
>   
What I have done now is add a method called post_exception_event, which 
checks to see if this is a repeated throw, and if not constucts an 
ExceptionEvent and alls send event to send the JVMTI event notification.
> Kyle> +// This method looks up the interpreted call stack to see if the excetion will
>
> Typo: "exception".
>   
Fixed.

I think the attached patch addresses all these issues.

Further comments?

- Kyle


[-- Attachment #2: exceptions-jvmti.patch --]
[-- Type: text/x-patch, Size: 13760 bytes --]

Index: libjava/interpret.cc
===================================================================
--- libjava/interpret.cc	(revision 121856)
+++ libjava/interpret.cc	(working copy)
@@ -43,6 +43,7 @@
 #include <gnu/classpath/jdwp/Jdwp.h>
 #include <gnu/gcj/jvmti/Breakpoint.h>
 #include <gnu/gcj/jvmti/BreakpointManager.h>
+#include <gnu/gcj/jvmti/ExceptionEvent.h>
 
 #ifdef INTERPRETER
 
@@ -1366,6 +1367,51 @@
   return -1;
 }
 
+// Method to check if an exception is caught at some location in a method
+// (meth).  Returns true if this method (meth) contains a catch block for the
+// exception (ex). False otherwise.  If there is a catch block, it sets the pc
+// to the location of the beginning of the catch block.
+jboolean
+_Jv_InterpMethod::check_handler (pc_t *pc, _Jv_InterpMethod *meth,
+                                java::lang::Throwable *ex)
+{
+#ifdef DIRECT_THREADED
+  void *logical_pc = (void *) ((insn_slot *) (*pc) - 1);
+#else
+  int logical_pc = (*pc) - 1 - meth->bytecode ();
+#endif
+  _Jv_InterpException *exc = meth->exceptions ();
+  jclass exc_class = ex->getClass ();
+
+  for (int i = 0; i < meth->exc_count; i++)
+    {
+      if (PCVAL (exc[i].start_pc) <= logical_pc
+          && logical_pc < PCVAL (exc[i].end_pc))
+        {
+#ifdef DIRECT_THREADED
+              jclass handler = (jclass) exc[i].handler_type.p;
+#else
+              jclass handler = NULL;
+              if (exc[i].handler_type.i != 0)
+                    handler
+                      = (_Jv_Linker::resolve_pool_entry (meth->defining_class,
+                                                                             ex$
+#endif /* DIRECT_THREADED */
+              if (handler == NULL || handler->isAssignableFrom (exc_class))
+                {
+#ifdef DIRECT_THREADED
+                  (*pc) = (insn_slot *) exc[i].handler_pc.p;
+#else
+                  (*pc) = meth->bytecode () + exc[i].handler_pc.i;
+#endif /* DIRECT_THREADED */
+                  return true;
+                }
+          }
+      }
+  return false;
+}
+
+
 void
 _Jv_InterpMethod::get_line_table (jlong& start, jlong& end,
 				  jintArray& line_numbers,
Index: libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream

Property changes on: libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class
___________________________________________________________________
Name: svn:mime-type
   + application/octet-stream

Index: libjava/include/java-interp.h
===================================================================
--- libjava/include/java-interp.h	(revision 121856)
+++ libjava/include/java-interp.h	(working copy)
@@ -210,6 +210,11 @@
   // Convenience function for indexing bytecode PC/insn slots in
   // line tables for JDWP
   jlong insn_index (pc_t pc);
+  
+  // Helper function used to check if there is a handler for an exception
+  // present at this code index
+  jboolean check_handler (pc_t *pc, _Jv_InterpMethod *meth,
+                     java::lang::Throwable *ex);
    
   /* Get the line table for this method.
    * start  is the lowest index in the method
Index: libjava/gnu/gcj/jvmti/ExceptionEvent.java
===================================================================
--- libjava/gnu/gcj/jvmti/ExceptionEvent.java	(revision 0)
+++ libjava/gnu/gcj/jvmti/ExceptionEvent.java	(revision 0)
@@ -0,0 +1,96 @@
+// ExceptionEvent - an exception event for JVMTI
+
+/* Copyright (C) 2006  Free Software Foundation
+
+   This file is part of libgcj.
+
+This software is copyrighted work licensed under the terms of the
+Libgcj License.  Please consult the file "LIBGCJ_LICENSE" for
+details.  */
+
+package gnu.gcj.jvmti;
+
+import java.util.WeakHashMap;
+
+/**
+ * Class to create and send JVMTI Exception events
+ *
+ * @author Kyle Galloway (kgallowa@redhat.com)
+ */
+public class ExceptionEvent
+{
+  // Information about where the exception was thrown
+  private long _throw_meth, _throw_loc;
+  
+  // Information about where the exception was or can be caught
+  private long _catch_meth, _catch_loc;
+  
+  // Thread where the exception occurred
+  private Thread _thread;
+  
+  // The exception
+  private Throwable _ex;
+  
+  // A hash map of the exceptions we've already seen in a thread's call stack
+  private static WeakHashMap<Thread, Throwable> _ex_map = new WeakHashMap<Thread, Throwable> ();
+  
+  /**
+   * Constructs a new ExceptionEvent and sends it.  If it is not caught
+   * within the frame where it was thrown (catch_meth and catch_loc are null),
+   * check_catch will check for a possible catch further up the call stack 
+   * before marking it uncaught.
+   * 
+   * @param thr the thread where the exception occurred
+   * @param throw_meth the method of the throw (a jmethodID)
+   * @param throw_loc the location of the throw (a jlocation)
+   * @param ex the exception
+   * @param catch_meth the method of the catch (a jmethodID), null indicates
+   * that the exception was not caught in the frame where it was thrown
+   * @param catch_loc the location of the catch (a jlocation), null indicates
+   * that the exception was not caught in the frame where it was thrown
+   */
+  private ExceptionEvent (Thread thr, long throw_meth, long throw_loc,
+		                  Throwable ex, long catch_meth, long catch_loc)
+  {
+    _thread = thr;
+    _ex = ex;
+    _throw_meth = throw_meth;
+    _throw_loc = throw_loc;
+    _catch_meth = catch_meth;
+    _catch_loc = catch_loc;
+  }
+  
+  public static void post_exception_event (Thread thr, long throw_meth,
+		                                  long throw_loc, Throwable ex,
+		                                  long catch_meth, long catch_loc)
+  {
+    // Check to see if there is an entry for this Thread thr in the has map.
+	// If not, add the thread to the hash map and send an ExceptionEvent.
+	if (_ex_map.containsKey (thr))
+	  {
+		// Check to see if we are receiving events for the same exception, or a
+		// new one.  If it is not the same exception beign rethrown, send a new
+		// event.
+	    if (!(_ex_map.get (thr).equals (ex)))
+	      {
+            _ex_map.put (thr, ex);
+            ExceptionEvent event = new ExceptionEvent (thr, throw_meth,
+            		                                   throw_loc, ex,
+            		                                   catch_meth, catch_loc);  
+            event.send_event ();
+          }
+	  }
+	else
+	  {
+	    _ex_map.put (thr, ex);
+	    ExceptionEvent event = new ExceptionEvent (thr, throw_meth,
+                                                 throw_loc, ex,
+                                                 catch_meth, catch_loc);
+	    event.send_event ();
+	  }
+  }
+  
+  public native void send_event ();
+  
+  public native void check_catch ();
+}
Index: libjava/gnu/gcj/jvmti/ExceptionEvent.h
===================================================================
--- libjava/gnu/gcj/jvmti/ExceptionEvent.h	(revision 0)
+++ libjava/gnu/gcj/jvmti/ExceptionEvent.h	(revision 0)
@@ -0,0 +1,44 @@
+
+// DO NOT EDIT THIS FILE - it is machine generated -*- c++ -*-
+
+#ifndef __gnu_gcj_jvmti_ExceptionEvent__
+#define __gnu_gcj_jvmti_ExceptionEvent__
+
+#pragma interface
+
+#include <java/lang/Object.h>
+extern "Java"
+{
+  namespace gnu
+  {
+    namespace gcj
+    {
+      namespace jvmti
+      {
+          class ExceptionEvent;
+      }
+    }
+  }
+}
+
+class gnu::gcj::jvmti::ExceptionEvent : public ::java::lang::Object
+{
+
+  ExceptionEvent(::java::lang::Thread *, jlong, jlong, ::java::lang::Throwable *, jlong, jlong);
+public:
+  static void post_exception_event(::java::lang::Thread *, jlong, jlong, ::java::lang::Throwable *, jlong, jlong);
+  virtual void send_event();
+  virtual void check_catch();
+private:
+  jlong __attribute__((aligned(__alignof__( ::java::lang::Object)))) _throw_meth;
+  jlong _throw_loc;
+  jlong _catch_meth;
+  jlong _catch_loc;
+  ::java::lang::Thread * _thread;
+  ::java::lang::Throwable * _ex;
+  static ::java::util::WeakHashMap * _ex_map;
+public:
+  static ::java::lang::Class class$;
+};
+
+#endif // __gnu_gcj_jvmti_ExceptionEvent__
Index: libjava/gnu/gcj/jvmti/natExceptionEvent.cc
===================================================================
--- libjava/gnu/gcj/jvmti/natExceptionEvent.cc	(revision 0)
+++ libjava/gnu/gcj/jvmti/natExceptionEvent.cc	(revision 0)
@@ -0,0 +1,59 @@
+// natExceptionEvent.cc - C++ code for JVMTI Exception events
+
+/* Copyright (C) 2006  Free Software Foundation
+
+   This file is part of libgcj.
+
+This software is copyrighted work licensed under the terms of the
+Libgcj License.  Please consult the file "LIBGCJ_LICENSE" for
+details.  */
+
+#include <config.h>
+#include <gcj/cni.h>
+#include <gcj/method.h>
+#include <java-interp.h>
+#include <java-insns.h>
+#include <java-assert.h>
+#include <jvmti.h>
+#include <jvmti-int.h>
+
+#include <gnu/gcj/jvmti/ExceptionEvent.h>
+
+void
+gnu::gcj::jvmti::ExceptionEvent::send_event ()
+{
+  // Check if the exception is caught somewhere in the interpreted call stack
+  if (_catch_meth == 0 || _catch_loc == 0)
+    check_catch ();
+    
+  JNIEnv *jni = _Jv_GetCurrentJNIEnv ();
+
+  _Jv_JVMTI_PostEvent (JVMTI_EVENT_EXCEPTION, _thread, jni,
+                       reinterpret_cast<jmethodID> (_throw_meth),
+                       static_cast<jlocation> (_throw_loc), _ex,
+                       reinterpret_cast<jmethodID> (_catch_meth),
+                       static_cast<jlocation> (_catch_loc)); 
+}
+
+// This method looks up the interpreted call stack to see if the exception will
+// eventually be caught by some java method.
+void
+gnu::gcj::jvmti::ExceptionEvent::check_catch ()
+{
+  _Jv_InterpFrame *frame 
+    = reinterpret_cast<_Jv_InterpFrame *> (_thread->interp_frame);
+  
+  while ((frame=frame->next_interp))
+    {
+	  _Jv_InterpMethod *meth 
+	    = reinterpret_cast<_Jv_InterpMethod *> (frame->self);
+	  pc_t pc = frame->pc;
+		
+	  if (meth->check_handler (&pc, meth, _ex))
+	    {
+	      _catch_meth = reinterpret_cast<jlong> (meth->get_method ());
+	      _catch_loc = meth->insn_index (pc);
+          break;
+	    }
+    }
+}
Index: libjava/interpret-run.cc
===================================================================
--- libjava/interpret-run.cc	(revision 121856)
+++ libjava/interpret-run.cc	(working copy)
@@ -2540,43 +2540,38 @@
     }
   catch (java::lang::Throwable *ex)
     {
-#ifdef DIRECT_THREADED
-      void *logical_pc = (void *) ((insn_slot *) pc - 1);
-#else
-      int logical_pc = pc - 1 - meth->bytecode ();
+#ifdef DEBUG
+       // This needs to be done before the pc is changed.
+       jlong throw_loc = meth->insn_index (pc);
 #endif
-      _Jv_InterpException *exc = meth->exceptions ();
-      jclass exc_class = ex->getClass ();
-
-      for (int i = 0; i < meth->exc_count; i++)
-	{
-	  if (PCVAL (exc[i].start_pc) <= logical_pc
-	      && logical_pc < PCVAL (exc[i].end_pc))
-	    {
-#ifdef DIRECT_THREADED
-	      jclass handler = (jclass) exc[i].handler_type.p;
-#else
-	      jclass handler = NULL;
-	      if (exc[i].handler_type.i != 0)
-		handler = (_Jv_Linker::resolve_pool_entry (meth->defining_class,
-							   exc[i].handler_type.i)).clazz;
-#endif /* DIRECT_THREADED */
-
-	      if (handler == NULL || handler->isAssignableFrom (exc_class))
-		{
-
-#ifdef DIRECT_THREADED
-		  pc = (insn_slot *) exc[i].handler_pc.p;
-#else
-		  pc = meth->bytecode () + exc[i].handler_pc.i;
-#endif /* DIRECT_THREADED */
-		  sp = stack;
-		  sp++->o = ex; // Push exception.
-		  NEXT_INSN;
-		}
-	    }
-	}
-
+      // Check if the exception is handled and, if so, set the pc to the start
+      // of the appropriate catch block.
+      if (meth->check_handler (&pc, meth, ex))
+        {
+          sp = stack;
+          sp++->o = ex; // Push exception.
+#ifdef DEBUG
+          if (JVMTI_REQUESTED_EVENT (Exception))
+            {
+              using namespace gnu::gcj::jvmti;
+              jlong throw_meth = reinterpret_cast<jlong> (meth->get_method ());
+              jlong catch_loc = meth->insn_index (pc);
+              ExceptionEvent::post_exception_event (thread, throw_meth,
+                                                    throw_loc, ex, throw_meth,
+                                                    catch_loc);
+            }
+#endif
+          NEXT_INSN;
+        }
+#ifdef DEBUG
+      if (JVMTI_REQUESTED_EVENT (Exception))
+        {
+          using namespace gnu::gcj::jvmti;
+          jlong throw_meth = reinterpret_cast<jlong> (meth->get_method ());
+          ExceptionEvent::post_exception_event (thread, throw_meth, throw_loc,
+                                                ex, NULL, NULL);
+        }
+#endif
       // No handler, so re-throw.
       throw ex;
     }
Index: libjava/sources.am
===================================================================
--- libjava/sources.am	(revision 121856)
+++ libjava/sources.am	(working copy)
@@ -509,6 +509,7 @@
 gnu_gcj_jvmti_source_files = \
 gnu/gcj/jvmti/Breakpoint.java \
 gnu/gcj/jvmti/BreakpointManager.java \
+gnu/gcj/jvmti/ExceptionEvent.java \
 gnu/gcj/jvmti/Location.java
 
 gnu_gcj_jvmti_header_files = $(patsubst %.java,%.h,$(gnu_gcj_jvmti_source_files))
Index: libjava/Makefile.am
===================================================================
--- libjava/Makefile.am	(revision 121856)
+++ libjava/Makefile.am	(working copy)
@@ -826,6 +826,7 @@
 gnu/gcj/io/natSimpleSHSStream.cc \
 gnu/gcj/io/shs.cc \
 gnu/gcj/jvmti/natBreakpoint.cc \
+gnu/gcj/jvmti/natExceptionEvent.cc \
 gnu/gcj/runtime/natFinalizerThread.cc \
 gnu/gcj/runtime/natSharedLibLoader.cc \
 gnu/gcj/runtime/natSystemClassLoader.cc \

Re: [RFA] JMTI Exception Events

[Tom Tromey]

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

Kyle> +        if (!(_ex_map.get (thr).equals (ex)))

>> Without an equals() method in this class, I doubt this does what you
>> intend.  As it is this equals() will always return false, because the
>> default equals() tests object identity, and a new object is never
>> identical to any existing one.

Kyle> In this case, however, I believe I want to only equal the exact same
Kyle> exception.

Ah, thanks.  I misread this, sorry about that.

If you really intend to check identity, use '==' and not 'equals()'.
Otherwise things can be weird if a subclass happens to override equals.

Kyle> What I have done now is add a method called post_exception_event,
Kyle> which checks to see if this is a repeated throw, and if not constucts
Kyle> an ExceptionEvent and alls send event to send the JVMTI event
Kyle> notification.

Please use javaStyleNaming.  I'm more a gnu-style person myself, but
we're stuck with that.  (One of my little pet peeves is that the stack
trace stuff uses uppercase for the start of method names... weird and
somehow irritating.)

Kyle> +  public static void post_exception_event (Thread thr, long throw_meth,
Kyle> +		                                  long throw_loc, Throwable ex,
Kyle> +		                                  long catch_meth, long catch_loc)

I can't tell due to the patch stuff but the formatting of this looks
weird in my mail buffer; so please make sure it looks right before
committing.

Tom

Re: [RFA] JMTI Exception Events

[Kyle Galloway]

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
> Kyle> +        if (!(_ex_map.get (thr).equals (ex)))
>
>   
>>> Without an equals() method in this class, I doubt this does what you
>>> intend.  As it is this equals() will always return false, because the
>>> default equals() tests object identity, and a new object is never
>>> identical to any existing one.
>>>       
>
> Kyle> In this case, however, I believe I want to only equal the exact same
> Kyle> exception.
>
> Ah, thanks.  I misread this, sorry about that.
>
> If you really intend to check identity, use '==' and not 'equals()'.
> Otherwise things can be weird if a subclass happens to override equals.
>   
Ok, no problem.
> Kyle> What I have done now is add a method called post_exception_event,
> Kyle> which checks to see if this is a repeated throw, and if not constucts
> Kyle> an ExceptionEvent and alls send event to send the JVMTI event
> Kyle> notification.
>
> Please use javaStyleNaming.  I'm more a gnu-style person myself, but
> we're stuck with that.  (One of my little pet peeves is that the stack
> trace stuff uses uppercase for the start of method names... weird and
> somehow irritating.)
>   
So you want, postExceptionEvent instead of post_exception_event.  What 
about the two native calls should they be sendEvent and checkCatch.  
Also do you want the variable names changed to throwMeth, catchLoc etc?  
And while I'm at it, should it be funtion() or function () in java files 
outside of classpath, I think I've seen both?.  I'm sorry to keep asking 
about/screwing this up but I find it can be a little confusing to know 
which conventions to use sometimes.
> Kyle> +  public static void post_exception_event (Thread thr, long throw_meth,
> Kyle> +		                                  long throw_loc, Throwable ex,
> Kyle> +		                                  long catch_meth, long catch_loc)
>
> I can't tell due to the patch stuff but the formatting of this looks
> weird in my mail buffer; so please make sure it looks right before
> committing.
>   
Other than these is this ok to go in?

- Kyle

Re: [RFA] JMTI Exception Events

[Tom Tromey]

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

>> Please use javaStyleNaming.  I'm more a gnu-style person myself, but
>> we're stuck with that.  (One of my little pet peeves is that the stack
>> trace stuff uses uppercase for the start of method names... weird and
>> somehow irritating.)

Kyle> So you want, postExceptionEvent instead of post_exception_event.

Yes please.

Kyle> What about the two native calls should they be sendEvent and
Kyle> checkCatch.

Ah, yes.

Kyle> Also do you want the variable names changed to throwMeth, catchLoc
Kyle> etc?

Short names are fine.  Variables I'm less picky about, it is mostly
method names that look weird when not java-style.

Kyle> And while I'm at it, should it be funtion() or function () in
Kyle> java files outside of classpath, I think I've seen both?.

Officially we use function() for calls.  I don't recall whether we
have an official stance on definitions.  There is a lot of
inconsistency here.

Kyle> I'm sorry to keep asking about/screwing this up but I find it
Kyle> can be a little confusing to know which conventions to use
Kyle> sometimes.

Yeah.  I thought there was a classpath style guideline somewhere,
maybe in classpath/doc.  ChangeLogs are described in the GNU coding
standards; if you're an info users they are already on your system...

Kyle> Other than these is this ok to go in?

Yes, thank you.

Tom

Re: [RFA] JMTI Exception Events

[Kyle Galloway]

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
>   
>>> Please use javaStyleNaming.  I'm more a gnu-style person myself, but
>>> we're stuck with that.  (One of my little pet peeves is that the stack
>>> trace stuff uses uppercase for the start of method names... weird and
>>> somehow irritating.)
>>>       
>
> Kyle> So you want, postExceptionEvent instead of post_exception_event.
>
> Yes please.
>
> Kyle> What about the two native calls should they be sendEvent and
> Kyle> checkCatch.
>
> Ah, yes.
>
> Kyle> Also do you want the variable names changed to throwMeth, catchLoc
> Kyle> etc?
>
> Short names are fine.  Variables I'm less picky about, it is mostly
> method names that look weird when not java-style.
>
> Kyle> And while I'm at it, should it be funtion() or function () in
> Kyle> java files outside of classpath, I think I've seen both?.
>
> Officially we use function() for calls.  I don't recall whether we
> have an official stance on definitions.  There is a lot of
> inconsistency here.
>
> Kyle> I'm sorry to keep asking about/screwing this up but I find it
> Kyle> can be a little confusing to know which conventions to use
> Kyle> sometimes.
>
> Yeah.  I thought there was a classpath style guideline somewhere,
> maybe in classpath/doc.  ChangeLogs are described in the GNU coding
> standards; if you're an info users they are already on your system...
>
> Kyle> Other than these is this ok to go in?
>
> Yes, thank you.
>   
Changed and committed.

Thanks,
Kyle

Re: [RFA] JMTI Exception Events

[Andrew Haley]

Kyle Galloway writes:
 > I've had one or another variation of this lying around for some time, 
 > 
 > Thanks,
 > Kyle
 > 
 > ChangeLog
 > 2007-02-14  Kyle Galloway  <kgallowa@redhat.com>
 > 
 > 	* interpret.cc (_Jv_InterpMethod::check_handler): New method.
 > 	* interpret-run.cc: Change the catch section to report exception
 > 	events and to use the new check_handler method.
 > 	* include/java-interp.h (_Jv_InterpMethod): Add check_handler.
 > 	* gnu/gcj/jvmti/ExceptionEvent.java: New file.
 > 	* gnu/gcj/jvmti/ExceptionEvent.h: New file.
 > 	* gnu/gcj/jvmti/natExceptionEvent.cc: New file.
 > 	* libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class: New
 > 	file.
 > 	* sources.am: Added ExceptionEvent.java.
 > 	* Makefile.am: Added natExceptionEvent.cc

Your indentation has gone FUBAR.

The ChangeLog entry looks like this:

2007-02-15  Kyle Galloway  <kgallowa@redhat.com>
        
        * interpret.cc (_Jv_InterpMethod::check_handler): New method.
    * interpret-run.cc: Change the catch section to report exception
    events and to use the new check_handler method.
    * include/java-interp.h (_Jv_InterpMethod): Add check_handler.
    * gnu/gcj/jvmti/ExceptionEvent.java: New file.
    * gnu/gcj/jvmti/ExceptionEvent.h: New file.
    * gnu/gcj/jvmti/natExceptionEvent.cc: New file.
    * libjava/classpath/lib/gnu/gcj/jvmti/ExceptionEvent.class: New
    file.
    * sources.am: Added ExceptionEvent.java.
    * Makefile.am: Added natExceptionEvent.cc
    * Makefile.in: Regenerated.
    * include/Makefile.in: Regenerated.
    * gcj/Makefile.in: Regenerated.

Andrew.