[PATCH] Allow printing of Guile values

[PATCH] Allow printing of Guile values

[Ludovic Courtès]

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

Hi,

The attached patch fixes printing of Guile objects.  It does so by
querying the inferior Guile process for a textual representation of the
values.

Actually, support routines (namely `gdb_print ()') have been in Guile
for a long time, but have never been used by GDB.  I tested it with
Guile 1.8, but it should also work with 1.6, and perhaps even with older
versions.

Example:

  Breakpoint 4, scm_sum (x=0xf2, y=0x6) at numbers.c:3953
  3953      if (SCM_UNBNDP (y))
  (gdb) set language scheme
  Warning: the current language does not match this frame.
  (gdb) frame
  #0  scm_sum (x=60, y=1) at numbers.c:3953
  3953      if (SCM_UNBNDP (y))

Same with non-immediate values:

  Breakpoint 3, deval (x=0xb7c38978, env=0xb7bc7370) at eval.i.c:215
  215   debug.prev = scm_i_last_debug_frame ();
  (gdb) set language scheme
  (gdb) frame
  #0  deval (
      x=(#<variable b7c3dbc0 value: #<primitive-procedure copy-tree>> (#<variable b7c3e5b0 value: #<procedure apply (fun . args)>> #@1+0 (#<variable b7c094c0 value: #<primitive-procedure cdr>> #@0+0))), env=(((exp env) (false-if-exception (stat str)) (((str) "/home/ludo/.guile"))) ((f) #<procedure #f (expr)>))) at eval.i.c:215
  215   debug.prev = scm_i_last_debug_frame ();


It's my first time hacking GDB, so please don't hesitate to correct me
if the patch looks bad.

I have not yet signed copyright assignment papers for GDB.  I will do it
if deemed necessary.

Thanks,
Ludovic.


gdb/ChangeLog

2007-07-20  Ludovic Courtès  <ludo@gnu.org>

	* scm-lang.c (is_scmvalue_type): Don't check for `TYPE_CODE' of
	TYPE since it may lead to wrong results.
	* scm-valprint.c (scm_inferior_print): Use facilities provided
	by the inferior Guile process, namely `gdb_print ()'.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: The patch --]
[-- Type: text/x-patch, Size: 3051 bytes --]

--- scm-lang.c.orig	2007-07-20 10:04:20.000000000 +0200
+++ scm-lang.c	2007-07-20 16:12:47.000000000 +0200
@@ -1,6 +1,6 @@
 /* Scheme/Guile language support routines for GDB, the GNU debugger.
 
-   Copyright (C) 1995, 1996, 1998, 2000, 2001, 2002, 2003, 2004, 2005 Free
+   Copyright (C) 1995, 1996, 1998, 2000, 2001, 2002, 2003, 2004, 2005, 2007 Free
    Software Foundation, Inc.
 
    This file is part of GDB.
@@ -59,8 +59,7 @@
 int
 is_scmvalue_type (struct type *type)
 {
-  if (TYPE_CODE (type) == TYPE_CODE_INT
-      && TYPE_NAME (type) && strcmp (TYPE_NAME (type), "SCM") == 0)
+  if (TYPE_NAME (type) && strcmp (TYPE_NAME (type), "SCM") == 0)
     {
       return 1;
     }

--- scm-valprint.c.orig	2007-07-20 16:07:11.000000000 +0200
+++ scm-valprint.c	2007-07-20 16:08:13.000000000 +0200
@@ -1,6 +1,6 @@
 /* Scheme/Guile language support routines for GDB, the GNU debugger.
 
-   Copyright (C) 1995, 1996, 1998, 1999, 2000, 2001, 2005 Free Software
+   Copyright (C) 1995, 1996, 1998, 1999, 2000, 2001, 2005, 2007 Free Software
    Foundation, Inc.
 
    This file is part of GDB.
@@ -31,6 +31,7 @@
 #include "valprint.h"
 #include "gdbcore.h"
 #include "c-lang.h"
+#include "infcall.h"
 
 static void scm_ipruk (char *, LONGEST, struct ui_file *);
 static void scm_scmlist_print (LONGEST, struct ui_file *, int, int,
@@ -39,14 +40,53 @@
 			       int, enum val_prettyprint);
 
 /* Prints the SCM value VALUE by invoking the inferior, if appropraite.
-   Returns >= 0 on succes;  retunr -1 if the inferior cannot/should not
+   Returns >= 0 on success;  return -1 if the inferior cannot/should not
    print VALUE. */
 
 static int
 scm_inferior_print (LONGEST value, struct ui_file *stream, int format,
 		    int deref_ref, int recurse, enum val_prettyprint pretty)
 {
-  return -1;
+  struct value *func, *arg, *result;
+  struct symbol *gdb_output_sym, *gdb_output_len_sym;
+  char *output;
+  int ret, output_len;
+
+  func = find_function_in_inferior ("gdb_print");
+  arg = value_from_longest (builtin_type_CORE_ADDR, value);
+
+  result = call_function_by_hand (func, 1, &arg);
+  ret = (int) value_as_long (result);
+  if (ret == 0)
+    {
+      /* XXX: Should we cache these symbols?  */
+      gdb_output_sym =
+	lookup_symbol_global ("gdb_output", NULL, VAR_DOMAIN,
+			      (struct symtab **) NULL);
+      gdb_output_len_sym =
+	lookup_symbol_global ("gdb_output_length", NULL, VAR_DOMAIN,
+			      (struct symtab **) NULL);
+
+      if ((gdb_output_sym == NULL) || (gdb_output_len_sym == NULL))
+	ret = -1;
+      else
+	{
+	  struct value *remote_buffer;
+
+	  read_memory (SYMBOL_VALUE_ADDRESS (gdb_output_len_sym),
+		       (char *) &output_len, sizeof (output_len));
+
+	  output = (char *) alloca (output_len);
+	  remote_buffer = value_at (builtin_type_CORE_ADDR,
+				    SYMBOL_VALUE_ADDRESS (gdb_output_sym));
+	  read_memory (value_as_address (remote_buffer),
+		       output, output_len);
+
+	  ui_file_write (stream, output, output_len);
+	}
+    }
+
+  return ret;
 }
 
 /* {Names of immediate symbols}

Re: [PATCH] Allow printing of Guile values

[Daniel Jacobowitz]

On Fri, Jul 20, 2007 at 04:27:30PM +0200, Ludovic Courtès wrote:
> Hi,
> 
> The attached patch fixes printing of Guile objects.  It does so by
> querying the inferior Guile process for a textual representation of the
> values.
> 
> Actually, support routines (namely `gdb_print ()') have been in Guile
> for a long time, but have never been used by GDB.  I tested it with
> Guile 1.8, but it should also work with 1.6, and perhaps even with older
> versions.

Unfortunately, Guile support has been removed from GDB, so this patch
no longer applies.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Hi,

Daniel Jacobowitz <drow@false.org> writes:

> Unfortunately, Guile support has been removed from GDB, so this patch
> no longer applies.

Hmm, that was against GDB 6.6.

It's unfortunate that support has been removed without leaving Guile
folks an opportunity to do something about it.

Is there a chance that minimal support could be reinstated?  It's true
that most of the `scm-' files in 6.6 are unfinished, outdated, or
complex (they actually re-implement parts of Guile, which is admittedly
overkill and certainly hard to maintain).  By "minimal support", I mean
that we'd arrange to move most support routines in Guile, and leave
(almost) maintenance-free code in GDB.

What do you think?

Thanks,
Ludovic.

Re: [PATCH] Allow printing of Guile values

[Jim Blandy]

ludo@gnu.org (Ludovic Courtès) writes:
> Daniel Jacobowitz <drow@false.org> writes:
>
>> Unfortunately, Guile support has been removed from GDB, so this patch
>> no longer applies.
>
> Hmm, that was against GDB 6.6.
>
> It's unfortunate that support has been removed without leaving Guile
> folks an opportunity to do something about it.
>
> Is there a chance that minimal support could be reinstated?

Definitely.  Part of the rationale for deleting it was, "Oh, if
someone shows up to keep it alive, we've still got it in CVS."  I'll
be happy to review changes.  The key is that someone make themselves
available to keep it working.

> It's true that most of the `scm-' files in 6.6 are unfinished,
> outdated, or complex (they actually re-implement parts of Guile,
> which is admittedly overkill and certainly hard to maintain).  By
> "minimal support", I mean that we'd arrange to move most support
> routines in Guile, and leave (almost) maintenance-free code in GDB.

A debugger should be able to inspect the state of programs in serious
disarray, so there's an argument against relying too much on invoking
functions in Guile.  Certainly, though, the best code would draw a
clear distinction between scm-lang.c and scm-lang-guile.c.

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Hi Jim,

Jim Blandy <jimb@codesourcery.com> writes:

> Definitely.  Part of the rationale for deleting it was, "Oh, if
> someone shows up to keep it alive, we've still got it in CVS."  I'll
> be happy to review changes.  The key is that someone make themselves
> available to keep it working.

Great.

> A debugger should be able to inspect the state of programs in serious
> disarray, so there's an argument against relying too much on invoking
> functions in Guile.  Certainly, though, the best code would draw a
> clear distinction between scm-lang.c and scm-lang-guile.c.

Agreed.  However, to be very pragmatic, we may be better off with
minimal support that works and doesn't add a significant maintenance
burden.  For instance, while valuable, interpretation of Guile values on
the GDB side requires a significant amount of code that must be kept in
sync with Guile.

How would you like to proceed: would you like to reinstall the deleted
files and patch against them, or would you prefer that we somehow start
anew?

Thanks,
Ludovic.

Re: [PATCH] Allow printing of Guile values

[Jim Blandy]

ludo@gnu.org (Ludovic Courtès) writes:
> Jim Blandy <jimb@codesourcery.com> writes:
>
>> Definitely.  Part of the rationale for deleting it was, "Oh, if
>> someone shows up to keep it alive, we've still got it in CVS."  I'll
>> be happy to review changes.  The key is that someone make themselves
>> available to keep it working.
>
> Great.
>
>> A debugger should be able to inspect the state of programs in serious
>> disarray, so there's an argument against relying too much on invoking
>> functions in Guile.  Certainly, though, the best code would draw a
>> clear distinction between scm-lang.c and scm-lang-guile.c.
>
> Agreed.  However, to be very pragmatic, we may be better off with
> minimal support that works and doesn't add a significant maintenance
> burden.  For instance, while valuable, interpretation of Guile values on
> the GDB side requires a significant amount of code that must be kept in
> sync with Guile.
>
> How would you like to proceed: would you like to reinstall the deleted
> files and patch against them, or would you prefer that we somehow start
> anew?

Let's take the files you've got now and just put them in.

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Jim Blandy <jimb@codesourcery.com> writes:

> ludo@gnu.org (Ludovic Courtès) writes:

>> How would you like to proceed: would you like to reinstall the deleted
>> files and patch against them, or would you prefer that we somehow start
>> anew?
>
> Let's take the files you've got now and just put them in.

The patch requires `scm-*.[ch]' because those files are more or less
interdependent.

I'll email `assign@gnu.org' so that we don't have to worry about future
changes.

Thanks!

Ludovic.

Re: [PATCH] Allow printing of Guile values

[Jim Blandy]

ludo@gnu.org (Ludovic Courtès) writes:
> The attached patch fixes printing of Guile objects.  It does so by
> querying the inferior Guile process for a textual representation of the
> values.

Hi, Ludovic.  How are things going with this?

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Hi Jim,

Jim Blandy <jimb@codesourcery.com> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>> The attached patch fixes printing of Guile objects.  It does so by
>> querying the inferior Guile process for a textual representation of the
>> values.
>
> Hi, Ludovic.  How are things going with this?

I just received by snail mail FSF's copyright assignment form so things
will soon be in order.

Besides, I haven't yet looked into other aspects of Guile support in
GDB.

Thanks,
Ludovic.

Re: [PATCH] Allow printing of Guile values

[Jim Blandy]

ludo@chbouib.org (Ludovic Courtès) writes:
>> ludo@gnu.org (Ludovic Courtès) writes:
>>> The attached patch fixes printing of Guile objects.  It does so by
>>> querying the inferior Guile process for a textual representation of the
>>> values.
>>
>> Hi, Ludovic.  How are things going with this?
>
> I just received by snail mail FSF's copyright assignment form so things
> will soon be in order.
>
> Besides, I haven't yet looked into other aspects of Guile support in
> GDB.

Hi, Ludovic.  I see that your name is now listed in the FSF's
copyright assignments list, so feel free to check in your change.
Please also add yourself to the Write After Approval list in
gdb/MAINTAINERS, as a separate commit.

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Hi,

Jim Blandy <jimb@codesourcery.com> writes:

> Please also add yourself to the Write After Approval list in
> gdb/MAINTAINERS, as a separate commit.

I used UTF-8, I hope this is not a problem for anyone.  How about adding
Emacs local variables so that it gets the right encoding upon file
opening?

Thanks,
Ludovic.

Re: [PATCH] Allow printing of Guile values

[Eli Zaretskii]

> From:  ludo@gnu.org (Ludovic =?iso-8859-1?Q?Court=E8s?=)
> Date:  Thu, 09 Aug 2007 23:53:38 +0200
> 
> I used UTF-8, I hope this is not a problem for anyone.  How about adding
> Emacs local variables so that it gets the right encoding upon file
> opening?

Fine with me.

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Hi,

Jim Blandy <jimb@codesourcery.com> writes:

> Hi, Ludovic.  I see that your name is now listed in the FSF's
> copyright assignments list, so feel free to check in your change.

Committed.

2007-08-10  Ludovic Courtès  <ludo@gnu.org>

	* Makefile.in (SFILES): Add scm-{exp,lang,valprint}.c.
	(scm_lang_h, scm_tags_h): New.
	(COMMON_OBS): Add scm-{exp,lang,valprint}.o.
	(scm-exp.o, scm-lang.o, scm-valprint.o): New targets.
	* defs.h (enum language): Add `language_scm'.

Thanks,
Ludovic.

Re: [PATCH] Allow printing of Guile values

[Ludovic Courtès]

Hi,

ludo@gnu.org (Ludovic Courtès) writes:

> gdb/ChangeLog
>
> 2007-07-20  Ludovic Courtès  <ludo@gnu.org>
>
> 	* scm-lang.c (is_scmvalue_type): Don't check for `TYPE_CODE' of
> 	TYPE since it may lead to wrong results.
> 	* scm-valprint.c (scm_inferior_print): Use facilities provided
> 	by the inferior Guile process, namely `gdb_print ()'.

I just received a copy of the FSF-signed copyright assignment form, so
this patch can potentially be applied now (the `copyright.list' file at
fencepost hasn't yet been updated, though).

Thanks,
Ludovic.