Line number support for interpreter

[Mark Wielaard <mark at klomp dot org>]

Hi,

Here is a first attempt at a patch to add support for line number and
source file name support to interpreted classes/methods.

The patch does the following:
- Implement reading of class and code attributes.
  - _Jv_InterpClass gets an extra source_name field.
  - _Jv_InterpMethod gets an extra line_table and length field.
  - There is a new struct _Jv_LineTableEntry that holds the information.
- For the direct-threaded interpreter the line_table pc field is
  remapped during compile().
- For the interpreter the _Jv_MethodChain now has a new pc field that
  holds the current 'pc' (this replaces the, also stack allocated, pc
  field in the run() method).
- This new field is used by StackTrace to fill in the _Jv_frame_info.
  - _Jv_frame_info.addr now either holds the actual address or the
  _Jv_InterpMethod pointer depending on whether _Jv_frame_info.interp is
  NULL or not. If it is not NULL then it contains the actual pc_t for
  the frame.
- _Jv_InterpMethod has a new get_source_line(pc_t) method which is
  called from NameFinder to fill in the appropriate fields of the
  StackTraceElement.

It contains several hacks that might not be a good idea, but not being a
real C or C++ hacker I didn't know how to solve this better for now. So
the DIRECT_THREADED define and pc_t typedef moved to the java-interp.h
include file. In interpret.cc there is terrible hack since I was lazy
and didn't want to rename all occurrences of 'pc' to 'frame_desc.pc'.
I'll try to clean those up after I get some comments on this.

I haven't tested the patch very much yet. But a simple test class that
throws some exceptions works and shows the correct line numbers. And I
am able to startup eclipse (2.1.2) with it under plain or
direct-threaded interpreter. (This was also the motivation for the patch
since I use to run eclipse under gij and got annoyed by the fact that
exceptions didn't have source and line numbers.)

So, Comments?

Cheers,

Mark

Index: defineclass.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/defineclass.cc,v
retrieving revision 1.35
diff -u -r1.35 defineclass.cc
--- defineclass.cc	24 Oct 2003 09:29:41 -0000	1.35
+++ defineclass.cc	9 May 2004 23:34:28 -0000
@@ -1,6 +1,6 @@
 // defineclass.cc - defining a class from .class format.
 
-/* Copyright (C) 1999, 2000, 2001, 2002, 2003  Free Software Foundation
+/* Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -222,6 +222,9 @@
     len    = length;
     pos    = 0;
     def    = (_Jv_InterpClass*) klass;
+
+    // Source file name is set when there is a "SourceFile" class attribute.
+    def->source_name = NULL;
   }
 
   /** and here goes the parser members defined out-of-line */
@@ -605,26 +608,56 @@
     }
 }
 
-void _Jv_ClassReader::read_one_code_attribute (int /*method*/) 
+void _Jv_ClassReader::read_one_code_attribute (int method_index) 
 {
-  /* ignore for now, ... later we may want to pick up
-     line number information, for debugging purposes;
-     in fact, the whole debugger issue is open!  */
-
-  /* int name = */ read2u ();
+  int name = read2u ();
   int length = read4 ();
-  skip (length);
-
+  if (is_attribute_name (name, "LineNumberTable"))
+    {
+      _Jv_InterpMethod *method =
+	(_Jv_InterpMethod*) def->interpreted_methods[method_index];
+      if (method->line_table != NULL)
+	throw_class_format_error ("only one LineNumberTable per method");
+
+      int table_len = read2u ();
+      _Jv_LineTableEntry* table
+	= (_Jv_LineTableEntry *) _Jv_Malloc (table_len
+					     * sizeof (_Jv_LineTableEntry));
+      for (int i = 0; i < table_len; i++)
+	{
+	  table[i].pc.i = read2u ();
+	  table[i].line = read2u ();
+	}
+      method->line_table_len = table_len;
+      method->line_table = table;
+    }
+  else
+    {
+      /* ignore unknown code attributes */
+      skip (length);
+    }
 }
 
 void _Jv_ClassReader::read_one_class_attribute () 
 {
-  /* we also ignore the class attributes, ...
+  /* we also ignore most class attributes, ...
      some day we'll add inner-classes support. */
 
-  /* int name = */ read2u ();
+  int name = read2u ();
   int length = read4 ();
-  skip (length);
+  if (is_attribute_name (name, "SourceFile"))
+    {
+      int source_index = read2u ();
+      check_tag (source_index, JV_CONSTANT_Utf8);
+      prepare_pool_entry (source_index, JV_CONSTANT_Utf8);
+      def->source_name
+	= _Jv_NewStringUtf8Const (def->constants.data[source_index].utf8);
+    }
+  else
+    {
+      /* ignore unknown class attributes */
+      skip (length);
+    }
 }
 
 
@@ -1278,6 +1311,10 @@
   method->defining_class = def;
   method->self           = &def->methods[method_index];
   method->prepared       = NULL;
+
+  // This is only set if there is a "LineNumberTable" code attribute.
+  method->line_table_len = 0;
+  method->line_table     = NULL;
 
   // grab the byte code!
   memcpy ((void*) method->bytecode (),
Index: interpret.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/interpret.cc,v
retrieving revision 1.40
diff -u -r1.40 interpret.cc
--- interpret.cc	14 Oct 2003 17:53:41 -0000	1.40
+++ interpret.cc	9 May 2004 23:34:28 -0000
@@ -1,6 +1,6 @@
 // interpret.cc - Code for the interpreter
 
-/* Copyright (C) 1999, 2000, 2001, 2002, 2003 Free Software Foundation
+/* Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation
 
    This file is part of libgcj.
 
@@ -12,11 +12,6 @@
 
 #include <config.h>
 
-// Define this to get the direct-threaded interpreter.  If undefined,
-// we revert to a basic bytecode interpreter.  The former is faster
-// but uses more memory.
-#define DIRECT_THREADED
-
 #pragma implementation "java-interp.h"
 
 #include <jvm.h>
@@ -56,26 +51,6 @@
 
 extern "C" double __ieee754_fmod (double,double);
 
-// This represents a single slot in the "compiled" form of the
-// bytecode.
-union insn_slot
-{
-  // Address of code.
-  void *insn;
-  // An integer value used by an instruction.
-  jint int_val;
-  // A pointer value used by an instruction.
-  void *datum;
-};
-
-// The type of the PC depends on whether we're doing direct threading
-// or a more ordinary bytecode interpreter.
-#ifdef DIRECT_THREADED
-typedef insn_slot *pc_t;
-#else
-typedef unsigned char *pc_t;
-#endif
-
 static inline void dupx (_Jv_word *sp, int n, int x)
 {
   // first "slide" n+x elements n to the right
@@ -755,6 +730,10 @@
       exc[i].handler_type.p = handler;
     }
 
+  // Update line number table
+  for (int i = 0; i < line_table_len; i++)
+    line_table[i].pc.p = &insns[pc_mapping[line_table[i].pc.i]];
+
   prepared = insns;
 }
 #endif /* DIRECT_THREADED */
@@ -1012,10 +991,11 @@
     0
   };
 
-  pc_t pc;
+// XXX Ugly quick and dirty HACK so we don't need to rename the variable
+// in the complete source.
+#define pc frame_desc.pc
 
 #ifdef DIRECT_THREADED
-
 #define NEXT_INSN goto *((pc++)->insn)
 #define INTVAL() ((pc++)->int_val)
 #define AVAL() ((pc++)->datum)
@@ -3212,6 +3192,28 @@
 void
 _Jv_EndOfInterpreter (void)
 {
+}
+
+// XXX HACK HACK HACK
+#undef pc
+
+int
+_Jv_InterpMethod::get_source_line(pc_t mpc)
+{
+#ifdef DIRECT_THREADED
+  void *lpc = (void *) ((insn_slot *) mpc - 1);
+#else
+  int lpc = mpc - 1 - bytecode ();
+#endif
+
+  int line = line_table_len > 0 ? line_table[0].line : -1;
+  for (int i = 1; i < line_table_len; i++)
+    if (PCVAL(line_table[i].pc) > lpc)
+      break;
+    else
+      line = line_table[i].line;
+
+  return line;
 }
 
 static void
Index: gnu/gcj/runtime/natNameFinder.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/gcj/runtime/natNameFinder.cc,v
retrieving revision 1.5
diff -u -r1.5 natNameFinder.cc
--- gnu/gcj/runtime/natNameFinder.cc	14 Nov 2003 01:48:29 -0000	1.5
+++ gnu/gcj/runtime/natNameFinder.cc	9 May 2004 23:34:28 -0000
@@ -1,6 +1,6 @@
 // natNameFinder.cc - native helper methods for NameFinder.java
 
-/* Copyright (C) 2002, 2003  Free Software Foundation, Inc
+/* Copyright (C) 2002, 2003, 2004  Free Software Foundation, Inc
 
    This file is part of libgcj.
 
@@ -123,22 +123,24 @@
 {
 #ifdef INTERPRETER
   _Jv_frame_info *stack = (_Jv_frame_info *) addrs;
-  if (stack[n].interp == NULL)
+  pc_t pc = (pc_t) stack[n].interp;
+  if (pc == NULL)
     return NULL;
 
   _Jv_InterpMethod *meth
-    = reinterpret_cast<_Jv_InterpMethod *> (stack[n].interp);
+    = reinterpret_cast<_Jv_InterpMethod *> (stack[n].addr);
   java::lang::StringBuffer *sb = new java::lang::StringBuffer();
   sb->append(_Jv_NewStringUtf8Const(meth->self->name));
   sb->append(_Jv_NewStringUtf8Const(meth->self->signature));
-  // FIXME: source file name and line number can be found from
-  // bytecode debug information.  But currently we don't keep that
-  // around.
   // FIXME: is using the defining class correct here?
-  java::lang::String *className = meth->defining_class->getName();
+  _Jv_InterpClass *clazz
+    = reinterpret_cast<_Jv_InterpClass *> (meth->defining_class);
+  java::lang::String *source = clazz->source_name;
+  jint line = meth->get_source_line(pc);
+  java::lang::String *className = clazz->getName();
   java::lang::String *methodName
 	  = demangleInterpreterMethod(sb->toString(), className);
-  return new java::lang::StackTraceElement(NULL, -1,
+  return new java::lang::StackTraceElement(source, line,
 					   className, methodName, false);
 #else // INTERPRETER
   return NULL;
Index: gnu/gcj/runtime/natStackTrace.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/gcj/runtime/natStackTrace.cc,v
retrieving revision 1.6
diff -u -r1.6 natStackTrace.cc
--- gnu/gcj/runtime/natStackTrace.cc	2 Oct 2003 07:10:34 -0000	1.6
+++ gnu/gcj/runtime/natStackTrace.cc	9 May 2004 23:34:28 -0000
@@ -57,6 +57,15 @@
   _Jv_frame_info *frame;
   if (len > 0)
     {
+      // When we don't have an interpreter we simply allocate len
+      // frames which will hold the addresses we just collected.  If
+      // we do have an interpreter the frames will have an extra 'interp'
+      // field that we have to fill in with NULL if the address doesn't
+      // come from the interpreter and otherwise we fill it in with
+      // the 'pc' of the next interpreter frame for this thread and let
+      // the 'addr' field point to an interpreted method struct (which can
+      // be queried later to collect the name of the class and method
+      // that was executing).
 #ifdef INTERPRETER
       extern void _Jv_StartOfInterpreter (void);
       extern void _Jv_EndOfInterpreter (void);
@@ -70,15 +79,21 @@
       frame = (_Jv_frame_info *) _Jv_Malloc (len * sizeof (_Jv_frame_info));
       for (int n = 0; n < len; n++)
 	{
-	  frame[n].addr = p[n];
 #ifdef INTERPRETER
 	  if (p[n] >= &_Jv_StartOfInterpreter && p[n] <= &_Jv_EndOfInterpreter)
 	    {
-	      frame[n].interp = (void *) interp_frame->self;
+	      frame[n].interp = (void *) interp_frame->pc;
+	      frame[n].addr = (void *) interp_frame->self;
 	      interp_frame = interp_frame->next;
 	    }
 	  else
-	    frame[n].interp = 0;
+	    {
+	      frame[n].interp = NULL;
+#endif
+	      // This is the only line in the for loop without INTERPRETER.
+	      frame[n].addr = p[n];
+#ifdef INTERPRETER
+	    }
 #endif // INTERPRETER
 	}
     }
@@ -142,7 +157,7 @@
   if (frame->interp)
     {
       _Jv_InterpMethod *meth
-	= reinterpret_cast<_Jv_InterpMethod *> (frame->interp);
+	= reinterpret_cast<_Jv_InterpMethod *> (frame->addr);
       return meth->defining_class;
     }
 #endif // INTERPRETER
@@ -160,7 +175,7 @@
   if (frame->interp)
     {
       meth
-	= reinterpret_cast<_Jv_InterpMethod *> (frame->interp)
+	= reinterpret_cast<_Jv_InterpMethod *> (frame->addr)
 	->get_method();
     }
 #endif // INTERPRETER
Index: include/java-interp.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/java-interp.h,v
retrieving revision 1.23
diff -u -r1.23 java-interp.h
--- include/java-interp.h	24 Oct 2003 09:29:42 -0000	1.23
+++ include/java-interp.h	9 May 2004 23:34:28 -0000
@@ -28,6 +28,31 @@
 #include <ffi.h>
 }
 
+// Define this to get the direct-threaded interpreter.  If undefined,
+// we revert to a basic bytecode interpreter.  The former is faster
+// but uses more memory.
+#define DIRECT_THREADED 
+
+#ifdef DIRECT_THREADED
+// This represents a single slot in the "compiled" form of the
+// bytecode.
+union insn_slot
+{
+  // Address of code.
+  void *insn;
+  // An integer value used by an instruction.
+  jint int_val;
+  // A pointer value used by an instruction.
+  void *datum;
+};
+
+// The type of the PC depends on whether we're doing direct threading
+// or a more ordinary bytecode interpreter.
+typedef insn_slot *pc_t;
+#else
+typedef unsigned char *pc_t;
+#endif // DIRECT_THREADED
+
 extern inline jboolean
 _Jv_IsInterpretedClass (jclass c)
 {
@@ -101,6 +126,17 @@
   }
 };
 
+// This structure holds the bytecode pc and corresponding source code
+// line number.  An array (plus length field) of this structure is put
+// in each _Jv_InterpMethod and used to resolve the (internal) program
+// counter of the interpreted method to an actual java source file
+// line.
+struct  _Jv_LineTableEntry
+{
+  _Jv_InterpPC pc;
+  int line;
+};
+
 class _Jv_InterpMethod : public _Jv_MethodBase
 {
   _Jv_ushort       max_stack;
@@ -109,6 +145,11 @@
 
   _Jv_ushort       exc_count;
 
+  // Length of the line_table.
+  // When this is zero then line_table is NULL.
+  int line_table_len;
+  _Jv_LineTableEntry *line_table;
+
   void *prepared;
 
   unsigned char* bytecode () 
@@ -143,6 +184,11 @@
 
   void run (void*, ffi_raw *);
 
+  // If there is a line number table for this method then this
+  // function returns the java source file line number given a process
+  // counter (from a stack frame). Otherwise it returns -1.
+  int get_source_line(pc_t mpc);
+
  public:
   static void dump_object(jobject o);
 
@@ -162,6 +208,7 @@
 {
   _Jv_MethodBase **interpreted_methods;
   _Jv_ushort        *field_initializers;
+  jstring	 source_name;
 
   friend class _Jv_ClassReader;
   friend class _Jv_InterpMethod;
@@ -174,6 +221,8 @@
 
   friend _Jv_MethodBase ** _Jv_GetFirstMethod (_Jv_InterpClass *klass);
   friend void _Jv_Defer_Resolution (void *cl, _Jv_Method *meth, void **);
+
+  friend class gnu::gcj::runtime::NameFinder;
 };
 
 // We have an interpreted class CL and we're trying to find the
@@ -252,6 +301,13 @@
 struct _Jv_MethodChain
 {
   const _Jv_InterpMethod *self;
+
+  // The current virtual process counter.
+  // Used by the interpreter run method to keep track of where it is.
+  // Can be turned into an actual source line by feeding it to
+  // self->get_source_line().
+  pc_t pc;
+
   _Jv_MethodChain **ptr;
   _Jv_MethodChain *next;
 
Index: include/jvm.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/jvm.h,v
retrieving revision 1.62
diff -u -r1.62 jvm.h
--- include/jvm.h	26 Oct 2003 02:25:41 -0000	1.62
+++ include/jvm.h	9 May 2004 23:34:29 -0000
@@ -119,8 +119,8 @@
   // PC value.
   void *addr;
 #ifdef INTERPRETER
-  // Actually a _Jv_InterpMethod, but we don't want to include
-  // java-interp.h everywhere.
+  // If not NULL addr is a _Jv_InterpMethod, otherwise addr is a real address.
+  // Actually a pc_t, but we don't want to include java-interp.h everywhere.
   void *interp;
 #endif // INTERPRETER
 };

Attachment: signature.asc
Description: This is a digitally signed message part