[RFA] Fix printing frame arguments for COFF debug info

[RFA] Fix printing frame arguments for COFF debug info

[Eli Zaretskii]

Currently, debugging DJGPP programs with COFF debug info fails to
print frame arguments:

  GNU gdb (GDB) 6.8.50.20090410
  Copyright (C) 2009 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
  and "show warranty" for details.
  This GDB was configured as "--host=i586-pc-msdosdjgpp --target=djgpp".
  For bug reporting instructions, please see:
  <http://www.gnu.org/software/gdb/bugs/>...
  (gdb) start
  Temporary breakpoint 1 at 0x1748: file tchset.c, line 6.
  Starting program: d:/usr/djgpp/gnu/gdb-68.410/gdb/./tchset45c.exe

  Temporary breakpoint 1, main (argc=<error reading variable>,
      argv=<error reading variable>) at tchset.c:6
  6         const char *cset = locale_charset ();
  (gdb) frame
  #0  main (argc=<error reading variable>, argv=<error reading variable>)
      at tchset.c:6
  6         const char *cset = locale_charset ();

GDB 6.1 does not have this problem, and neither does the current CVS
when DWARF-2 or stabs are used.

This started to happen because of this change:

  2008-05-06  Joel Brobecker  <brobecker@adacore.com>

	  * valprint.c (val_print): Add new language parameter and use it
	  instead of using the current_language. Update calls to val_print
	  throughout.
	  (common_val_print): Add new langauge parameter and pass it to
	  val_print.
	  * stack.c (print_frame_args): Update call to common_val_print
	  using the appropriate language.

After this change, print_frame_args uses the following logic to
compute the language with which to print frame arguments:

                  /* Use the appropriate language to display our symbol,
                     unless the user forced the language to a specific
                     language.  */
                  if (language_mode == language_mode_auto)
                    language = language_def (SYMBOL_LANGUAGE (sym));
                  else
                    language = current_language;

However, it does not check the value returned by language_def.  What
if that value is itself language_auto?  If that happens, val_print
will throw an error, because language_auto does not know how to print
values -- it's just a placeholder for some other, "real" language.

And it just so happens that coffread.c:process_coff_symbol does this:

  SYMBOL_LANGUAGE (sym) = language_auto;

The first step towards solving this is to fix coffread.c, as in the
patch I suggest below.  OK to commit it?

Even if this patch is accepted, I'm not sure that's all we need to do.
At least minsyms.c:prim_record_minimal_symbol_and_info also sets the
symbol's language to language_auto.  Should we make sure neither
common_val_print nor val_print ever get language_auto as the language?
That is, should we fall back to current_language in that case?

2009-05-16  Eli Zaretskii  <eliz@gnu.org>

	* coffread.c (process_coff_symbol): Set the symbol's language to
	the language of current_subfile.

--- coffread.c~0	2009-01-03 09:57:51.000000000 +0200
+++ coffread.c	2009-05-16 19:24:27.843781500 +0300
@@ -1482,7 +1482,7 @@ process_coff_symbol (struct coff_symbol 
   memset (sym, 0, sizeof (struct symbol));
   name = cs->c_name;
   name = EXTERNAL_NAME (name, objfile->obfd);
-  SYMBOL_LANGUAGE (sym) = language_auto;
+  SYMBOL_LANGUAGE (sym) = current_subfile->language;
   SYMBOL_SET_NAMES (sym, name, strlen (name), objfile);
 
   /* default assumptions */

More problems with language in printing symbols

[Eli Zaretskii]

There's a similar, but different problem with printing symbols in
"maint print symbols".  When the program is compiled with COFF debug
info, "maint print symbols" throws an error for some symbols:

    Error printing symbol:
    internal error - unimplemented function unk_lang_print_type called.

This happens because of this change:

    2005-06-03  Joel Brobecker  <brobecker@adacore.com>

	    * symmisc.c (dump_symtab_1): Renamed from dump_symtab.
	    (dump_symtab): New function.

The new dump_symtab does this:

    /* Set the current language to the language of the symtab we're dumping
       because certain routines used during dump_symtab() use the current
       language to print an image of the symbol.  We'll restore it later.  */
    saved_lang = set_language (symtab->language);

    dump_symtab_1 (objfile, symtab, outfile);

    set_language (saved_lang);

However, it does not check what is the value of symtab->language.  It
so happens that coffread.c creates a symtab for a "_globals_"
pseudo-file, whose symtab gets its language set to language_unknown,
because deduce_language_from_filename does not recognize such a file
name.  And, of course, trying to print symbols with language_unknown
as the current language does not give good results...

I'm unsure how best to fix that.  We could try recognizing "_globals_"
in deduce_language_from_filename, but what language to use for it?  Or
we could add some defensive fallback in dump_symtab which would not
switch the language if it is language_unknown.

Comments? ideas?

Re: [RFA] More problems with language in printing symbols

[Eli Zaretskii]

> Date: Sat, 16 May 2009 20:59:48 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org
> 
> The new dump_symtab does this:
> 
>     /* Set the current language to the language of the symtab we're dumping
>        because certain routines used during dump_symtab() use the current
>        language to print an image of the symbol.  We'll restore it later.  */
>     saved_lang = set_language (symtab->language);
> 
>     dump_symtab_1 (objfile, symtab, outfile);
> 
>     set_language (saved_lang);
> 
> However, it does not check what is the value of symtab->language.  It
> so happens that coffread.c creates a symtab for a "_globals_"
> pseudo-file, whose symtab gets its language set to language_unknown,
> because deduce_language_from_filename does not recognize such a file
> name.  And, of course, trying to print symbols with language_unknown
> as the current language does not give good results...
> 
> I'm unsure how best to fix that.  We could try recognizing "_globals_"
> in deduce_language_from_filename, but what language to use for it?  Or
> we could add some defensive fallback in dump_symtab which would not
> switch the language if it is language_unknown.
> 
> Comments? ideas?

Here's a patch that implements 2 ideas: it uses language_minimal for
the "_globals_" pseudo-file, and it makes dump_symtab be defensive
about the language of the symtab we are dumping.

OK to commit?

2009-05-17  Eli Zaretskii  <eliz@gnu.org>

	* symmisc.c (dump_symtab): Switch the current language to
	the language of the symtab we are dumping only if the symtab's
	language is neither language_auto nor language_unknown.

	* coffread.c (coff_symtab_read): Set language_minimal as the
	language for the "_globals_" pseudo-file.

--- coffread.c~1	2009-05-16 19:24:27.843781500 +0300
+++ coffread.c	2009-05-17 19:07:34.874125000 +0300
@@ -758,6 +758,11 @@ coff_symtab_read (long symtab_offset, un
 	    coff_end_symtab (objfile);
 
 	  coff_start_symtab ("_globals_");
+	  /* coff_start_symtab will set the language of this symtab to
+	     language_unknown, since such a ``file name'' is not
+	     recognized.  Override that with the minimal language to
+	     allow printing values in this symtab.  */
+	  current_subfile->language = language_minimal;
 	  complete_symtab ("_globals_", 0, 0);
 	  /* done with all files, everything from here on out is globals */
 	}

--- symmisc.c~0	2009-01-03 09:57:53.000000000 +0200
+++ symmisc.c	2009-05-17 19:02:46.139750000 +0300
@@ -496,16 +496,23 @@ static void
 dump_symtab (struct objfile *objfile, struct symtab *symtab,
 	     struct ui_file *outfile)
 {
-  enum language saved_lang;
-
   /* Set the current language to the language of the symtab we're dumping
      because certain routines used during dump_symtab() use the current
-     language to print an image of the symbol.  We'll restore it later.  */
-  saved_lang = set_language (symtab->language);
+     language to print an image of the symbol.  We'll restore it later.
+     But use only real languages, not placeholders.  */
+  if (symtab->language != language_unknown
+      && symtab->language != language_auto)
+    {
+      enum language saved_lang;
+
+      saved_lang = set_language (symtab->language);
 
-  dump_symtab_1 (objfile, symtab, outfile);
+      dump_symtab_1 (objfile, symtab, outfile);
 
-  set_language (saved_lang);
+      set_language (saved_lang);
+    }
+  else
+    dump_symtab_1 (objfile, symtab, outfile);
 }
 
 void

Re: [RFA] Fix printing frame arguments for COFF debug info

[Joel Brobecker]

> The first step towards solving this is to fix coffread.c, as in the
> patch I suggest below.  OK to commit it?

> 2009-05-16  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* coffread.c (process_coff_symbol): Set the symbol's language to
> 	the language of current_subfile.

That seems like a sensible change, and it is something we also do
in the stabs reader, so I'd say this is OK.

> Even if this patch is accepted, I'm not sure that's all we need to do.
> At least minsyms.c:prim_record_minimal_symbol_and_info also sets the
> symbol's language to language_auto.  Should we make sure neither
> common_val_print nor val_print ever get language_auto as the language?
> That is, should we fall back to current_language in that case?

I see that this is addressed in one of your followup messages, so
I'll send my take on it there.

-- 
Joel

Re: [RFA] More problems with language in printing symbols

[Joel Brobecker]

> Here's a patch that implements 2 ideas: it uses language_minimal for
> the "_globals_" pseudo-file,

Great minds think alike - I was going to suggest the same thing :-).

> 2009-05-17  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* symmisc.c (dump_symtab): Switch the current language to
> 	the language of the symtab we are dumping only if the symtab's
> 	language is neither language_auto nor language_unknown.
> 
> 	* coffread.c (coff_symtab_read): Set language_minimal as the
> 	language for the "_globals_" pseudo-file.

Both changes look OK to me.

-- 
Joel

Re: [RFA] Fix printing frame arguments for COFF debug info

[Eli Zaretskii]

> Date: Wed, 20 May 2009 14:32:00 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > The first step towards solving this is to fix coffread.c, as in the
> > patch I suggest below.  OK to commit it?
> 
> > 2009-05-16  Eli Zaretskii  <eliz@gnu.org>
> > 
> > 	* coffread.c (process_coff_symbol): Set the symbol's language to
> > 	the language of current_subfile.
> 
> That seems like a sensible change, and it is something we also do
> in the stabs reader, so I'd say this is OK.

Thanks.

> > Even if this patch is accepted, I'm not sure that's all we need to do.
> > At least minsyms.c:prim_record_minimal_symbol_and_info also sets the
> > symbol's language to language_auto.  Should we make sure neither
> > common_val_print nor val_print ever get language_auto as the language?
> > That is, should we fall back to current_language in that case?
> 
> I see that this is addressed in one of your followup messages

Not really, no.  The followup messages deal with a different problem.
This issue, i.e. whether common_val_print and/or val_print should be
defensive about getting language_auto, still remains.  I'd like to
hear your opinion about that.

Re: [RFA] Fix printing frame arguments for COFF debug info

[Joel Brobecker]

> > I see that this is addressed in one of your followup messages
> 
> Not really, no.  The followup messages deal with a different problem.
> This issue, i.e. whether common_val_print and/or val_print should be
> defensive about getting language_auto, still remains.  I'd like to
> hear your opinion about that.

Ah yes, of course! We could add guards in the valprint routines,
I supposed, but I'm not sure it's really worth the effort, now
that we have fixed the cause.  The error is not crippling, so
I personally wouldn't bother. But I wouldn't object either, especially
if it's a small localized change (I think I'd make the change in
val_print).

-- 
Joel

Re: [RFA] More problems with language in printing symbols

[Eli Zaretskii]

> Date: Wed, 20 May 2009 14:39:53 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > Here's a patch that implements 2 ideas: it uses language_minimal for
> > the "_globals_" pseudo-file,
> 
> Great minds think alike - I was going to suggest the same thing :-).
> 
> > 2009-05-17  Eli Zaretskii  <eliz@gnu.org>
> > 
> > 	* symmisc.c (dump_symtab): Switch the current language to
> > 	the language of the symtab we are dumping only if the symtab's
> > 	language is neither language_auto nor language_unknown.
> > 
> > 	* coffread.c (coff_symtab_read): Set language_minimal as the
> > 	language for the "_globals_" pseudo-file.
> 
> Both changes look OK to me.

Thanks, committed.

Re: [RFA] Fix printing frame arguments for COFF debug info

[Eli Zaretskii]

> Date: Wed, 20 May 2009 14:32:00 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > The first step towards solving this is to fix coffread.c, as in the
> > patch I suggest below.  OK to commit it?
> 
> > 2009-05-16  Eli Zaretskii  <eliz@gnu.org>
> > 
> > 	* coffread.c (process_coff_symbol): Set the symbol's language to
> > 	the language of current_subfile.
> 
> That seems like a sensible change, and it is something we also do
> in the stabs reader, so I'd say this is OK.

Thanks, committed.

Re: [RFA] Fix printing frame arguments for COFF debug info

[Eli Zaretskii]

> Date: Thu, 21 May 2009 10:05:02 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > > I see that this is addressed in one of your followup messages
> > 
> > Not really, no.  The followup messages deal with a different problem.
> > This issue, i.e. whether common_val_print and/or val_print should be
> > defensive about getting language_auto, still remains.  I'd like to
> > hear your opinion about that.
> 
> Ah yes, of course! We could add guards in the valprint routines,
> I supposed, but I'm not sure it's really worth the effort, now
> that we have fixed the cause.  The error is not crippling, so
> I personally wouldn't bother. But I wouldn't object either, especially
> if it's a small localized change (I think I'd make the change in
> val_print).

How about this:

2009-05-23  Eli Zaretskii  <eliz@gnu.org>

	* valprint.c (val_print): If LANGUAGE is "unknown" or "auto" or
	"local", fall back on current_language.


--- valprint.c~0	2009-03-21 07:03:53.000000000 +0200
+++ valprint.c	2009-05-23 13:33:14.062500000 +0300
@@ -257,6 +257,14 @@ val_print (struct type *type, const gdb_
       return (0);
     }
 
+  /* If the LANGUAGE argument we received is one of the placeholder
+     ``languages'', its la_val_print method will throw an error.  To
+     avoiding that, try falling back on current_language.  */
+  if ((language->la_language == language_unknown
+       || language->la_language == language_auto)
+      && language->la_language != current_language->la_language)
+    language = current_language;
+
   TRY_CATCH (except, RETURN_MASK_ERROR)
     {
       ret = language->la_val_print (type, valaddr, embedded_offset, address,

Re: [RFA] Fix printing frame arguments for COFF debug info

[Joel Brobecker]

> 2009-05-23  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* valprint.c (val_print): If LANGUAGE is "unknown" or "auto" or
> 	"local", fall back on current_language.

The patch looks good, although I'm still unconvinced that it is actually
necessary.  I'm slightly worried about the potential for feeding an
expression to the wrong language (as we've seen a crash a long time
ago when feeding some C++ stuff thinking it was Ada), but we can
actually look into that if it ever happens.

One of the things that you patch does, is also hidding an error that
was actually an internal error.  We're trying to recover nicely from
the situation in order to be more useful for the user, so getting rid
of the error is OK, but I'd probably still emit a complaint.  WDYT?

> --- valprint.c~0	2009-03-21 07:03:53.000000000 +0200
> +++ valprint.c	2009-05-23 13:33:14.062500000 +0300
> @@ -257,6 +257,14 @@ val_print (struct type *type, const gdb_
>        return (0);
>      }
>  
> +  /* If the LANGUAGE argument we received is one of the placeholder
> +     ``languages'', its la_val_print method will throw an error.  To
> +     avoiding that, try falling back on current_language.  */
        ^^^ avoid

-- 
Joel

Re: [RFA] Fix printing frame arguments for COFF debug info

[Eli Zaretskii]

> Date: Sun, 24 May 2009 23:29:22 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> One of the things that you patch does, is also hidding an error that
> was actually an internal error.

Well, using current_language was the old behavior before your change,
so my patch is just falling back on that old behavior.

> We're trying to recover nicely from the situation in order to be
> more useful for the user, so getting rid of the error is OK, but I'd
> probably still emit a complaint.  WDYT?

How about a warning under verbose operation?  Would that be enough?

Re: [RFA] Fix printing frame arguments for COFF debug info

[Joel Brobecker]

> Well, using current_language was the old behavior before your change,
> so my patch is just falling back on that old behavior.

True :).

> > We're trying to recover nicely from the situation in order to be
> > more useful for the user, so getting rid of the error is OK, but I'd
> > probably still emit a complaint.  WDYT?
> 
> How about a warning under verbose operation?  Would that be enough?

Sure. I don't know what the verbose switch would be, but I believe
that complaints provide that kind of functionality - by default,
complaints are turned off, but you can configure GDB to print
the first N complaints emitted.

-- 
Joel

Re: [RFA] Fix printing frame arguments for COFF debug info

[Eli Zaretskii]

> Date: Mon, 25 May 2009 15:55:37 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > > We're trying to recover nicely from the situation in order to be
> > > more useful for the user, so getting rid of the error is OK, but I'd
> > > probably still emit a complaint.  WDYT?
> > 
> > How about a warning under verbose operation?  Would that be enough?
> 
> Sure. I don't know what the verbose switch would be

I thought about info_verbose.

> I believe that complaints provide that kind of functionality - by
> default, complaints are turned off, but you can configure GDB to
> print the first N complaints emitted.

I looked into this, and I don't see a way to display a complaint in
this context.  The problem is that val_print is called a lot in the
middle of displaying arguments to functions, where a non-fatal warning
message cannot be reasonably displayed, especially if the message is
long.  Perhaps I'm missing something, but I don't see any machinery in
complaints.c (or elsewhere, for that matter) to handle such
situations.