Side-effects of double-parsing patch

Side-effects of double-parsing patch

[Paul N. Hilfinger]

Fernando,

The patch of 2001-01-26 to fix double parsing of filenames causes some
other changes, and I'm not certain which are intentional.  [A quick
review for the general audience: GDB used to call symbol_file_command
in a number of places for, well, reading the symbol file.  However,
when called "internally" (that is, for purposes other than
implementing the symbol-file command), this caused file names to get
parsed twice, with odd effects in some cases.  These calls were therefore
changed to calls to symbol_file_add_main, which immediately calls
symbol_file_add with some filled-in arguments.]

Now here's my question: symbol_file_command eventually calls
symbol_file_add, too, but it then goes on to call RESET_HP_UX_GLOBALS, 
reinit_cache_frame, and set_initial_language.  This no longer happens
for the "internal" calls.  One side effect is that set_initial_language is 
not called when GDB starts up, so that one now always starts in C (I think).
Unfortunately, this turns out to be a slight problem in the changes I am 
making for Ada.  This could be just a peculiarity of Ada, of course, so before
I do anything kludgey, may I ask what the reasoning was behind dispensing with 
these three calls?

Thanks.

Paul Hilfinger

Re: Side-effects of double-parsing patch

[Fernando Nasser]

"Paul N. Hilfinger" wrote:
> 
> Now here's my question: symbol_file_command eventually calls
> symbol_file_add, too, but it then goes on to call RESET_HP_UX_GLOBALS,
> reinit_cache_frame, and set_initial_language.  This no longer happens
> for the "internal" calls.  One side effect is that set_initial_language is
> not called when GDB starts up, so that one now always starts in C (I think).
> Unfortunately, this turns out to be a slight problem in the changes I am
> making for Ada.  This could be just a peculiarity of Ada, of course, so before
> I do anything kludgey, may I ask what the reasoning was behind dispensing with
> these three calls?

I looked again at the logic in there and I am inclined to believe it was
a mistake.  

I tend to dislike side-effects and probably wanted symbol_file_add() to
do only what the name implies and let the upper layers do whatever
actions they think are appropriate to be done concomitantly. But in this
case the reset of the frame cache and the setting of the language are
almost "part" of the action of loading a symbol file.

The attached patch moves the operations you've mentioned to the
"internal" libgdb call.  I have just run the testsuite (I have no time
to test it any further today).  You can help by trying it and let us
know if it solves your problem.

Thanks for reporting this.

Regards,
Fernando




-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.31
diff -c -p -r1.31 symfile.c
*** symfile.c	2001/04/05 02:02:13	1.31
--- symfile.c	2001/04/29 16:13:12
*************** symbol_file_add (char *name, int from_tt
*** 893,898 ****
--- 893,907 ----
    if (target_new_objfile_hook)
      target_new_objfile_hook (objfile);
  
+ #ifdef HPUXHPPA
+   RESET_HP_UX_GLOBALS ();
+ #endif
+   /* Getting new symbols may change our opinion about
+      what is frameless.  */
+   reinit_frame_cache ();
+ 
+   set_initial_language ();
+ 
    return (objfile);
  }
  
*************** symbol_file_command (char *args, int fro
*** 980,993 ****
  		{
                    name = *argv;
  		  symbol_file_add (name, from_tty, NULL, 1, flags);
- #ifdef HPUXHPPA
- 		  RESET_HP_UX_GLOBALS ();
- #endif
- 		  /* Getting new symbols may change our opinion about
- 		     what is frameless.  */
- 		  reinit_frame_cache ();
- 
- 		  set_initial_language ();
  		}
  	  argv++;
  	}
--- 989,994 ----
From ezannoni@cygnus.com Sun Apr 29 15:56:00 2001
From: Elena Zannoni <ezannoni@cygnus.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] Fix issues with DOSish file names in symbol tables
Date: Sun, 29 Apr 2001 15:56:00 -0000
Message-id: <15084.39992.691349.50765@kwikemart.cygnus.com>
References: <1438-Sat28Apr2001115732+0300-eliz@is.elta.co.il>
X-SW-Source: 2001-04/msg00270.html
Content-length: 2517

Eli Zaretskii writes:
 > The following minor changes fix GDB handling of DOS-style file and
 > directory names recorded in the debug info.  The specific issues are:
 > the form of an absolute file name and the case-insensitive nature of
 > the underlying file system, which can leak into the debug info.
 > 
 > It seems like Jim Blandy and/or Elena need to approve this.
 > 
 > Okay to commit?

Eli,

yes, ok with me. 2 less occurrences of STREQ, excellent!

Elena


 > 
 > 2001-04-28  Eli Zaretskii  <eliz@is.elta.co.il>
 > 
 > 	* buildsym.c (start_subfile): Use FILENAME_CMP instead of STREQ.
 > 	(top-level): #include filenames.h.
 > 
 > 	* dwarf2read.c (dwarf2_start_subfile): Use IS_ABSOLUTE_PATH and
 > 	FILENAME_CMP, to DTRT on non-Posix platforms.
 > 	(top-level): #include filenames.h.
 > 
 > --- gdb/buildsym.c~0	Fri Dec 15 03:01:44 2000
 > +++ gdb/buildsym.c	Sat Apr 28 11:19:40 2001
 > @@ -37,6 +37,7 @@
 >  #include "expression.h"		/* For "enum exp_opcode" used by... */
 >  #include "language.h"		/* For "longest_local_hex_string_custom" */
 >  #include "bcache.h"
 > +#include "filenames.h"		/* For DOSish file names */
 >  /* Ask buildsym.h to define the vars it normally declares `extern'.  */
 >  #define	EXTERN
 >  /**/
 > @@ -531,7 +532,7 @@ start_subfile (char *name, char *dirname
 >  
 >    for (subfile = subfiles; subfile; subfile = subfile->next)
 >      {
 > -      if (STREQ (subfile->name, name))
 > +      if (FILENAME_CMP (subfile->name, name) == 0)
 >  	{
 >  	  current_subfile = subfile;
 >  	  return;
 > --- gdb/dwarf2read.c~0	Wed Jan 24 02:22:46 2001
 > +++ gdb/dwarf2read.c	Sat Apr 28 11:14:34 2001
 > @@ -35,6 +35,7 @@
 >  #include "buildsym.h"
 >  #include "demangle.h"
 >  #include "expression.h"
 > +#include "filenames.h"	/* for DOSish file names */
 >  
 >  #include "language.h"
 >  #include "complaints.h"
 > @@ -4058,14 +4059,14 @@ dwarf2_start_subfile (char *filename, ch
 >    /* If the filename isn't absolute, try to match an existing subfile
 >       with the full pathname.  */
 >  
 > -  if (*filename != '/' && dirname != NULL)
 > +  if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
 >      {
 >        struct subfile *subfile;
 >        char *fullname = concat (dirname, "/", filename, NULL);
 >  
 >        for (subfile = subfiles; subfile; subfile = subfile->next)
 >  	{
 > -	  if (STREQ (subfile->name, fullname))
 > +	  if (FILENAME_CMP (subfile->name, fullname) == 0)
 >  	    {
 >  	      current_subfile = subfile;
 >  	      xfree (fullname);
 > 

Re: Side-effects of double-parsing patch

[Paul N. Hilfinger]

THanks.  This patch appears to fix the problem for me, at least.

Paul Hilfinger