RFA: [symfile.c} Fix to symbol_file_add()

RFA: [symfile.c} Fix to symbol_file_add()

[Fernando Nasser]

Paul N. Hilfinger has pointed out to me that a few operations should be
done every time a new symbol file is read.  This was an oversight in a
patch I wrote in January.  The small patch attached fixes that.

ChangeLog:

	* symfile.c (symbol_file_command): Move cleanup operations
	from here...
	(symbol_file_add): ...to here, so they are run every time a new
	symbol file is read.

-- 
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 nsd@redhat.com Mon Apr 30 09:32:00 2001
From: Nicholas Duffek <nsd@redhat.com>
To: gdb-patches@sourceware.cygnus.com
Cc: ezannoni@cygnus.com, phdm@macqel.be, kevinb@cygnus.com, Peter.Schauer@regent.e-technik.tu-muenchen.de, jimb@cygnus.com
Subject: [RFA] patch to skip bigtoc fixup code
Date: Mon, 30 Apr 2001 09:32:00 -0000
Message-id: <200104301626.MAA00208@nog.bosbc.com>
X-SW-Source: 2001-04/msg00274.html
Content-length: 8515

This patch is an improvement over the one I committed a couple of weeks
ago.  With the new patch, GDB works similarly to dbx:

  1. "step" silently steps through bigtoc fixup code.

  2. Numeric fixup code addresses resolve to symbols @FIX*.  For example,
     when debugging a cc1plus binary containing bigtoc fixup section @FIX1
     in address range 0x1001b614 through 0x1001bfc8, "x/i 0x1001b6fc"
     displays "<@FIX1+232>: addis r3,r2,1".

Without the patch, "step" stops at bigtoc fixup code because GDB detects
that it has stepped outside the current function into the middle of
another function.  A subsequent "step" would return to the original
function.

The patch uses some of the trampoline hooks in handle_inferior_event() to
tell GDB to skip bigtoc fixup code.  The hooks detect bigtoc fixup code by
looking for "@FIX" in the first 4 bytes of the code's function name.  "@"
symbols are no longer ignored by xcoffread.c; instead, they introduduce
new symbol tables and generate function minimal_symbols via the
misc_func_recorded mechanism.

ChangeLog:

	* config/rs6000/tm-rs6000.h (IN_SOLIB_RETURN_TRAMPOLINE): Define.
	(rs6000_in_solib_return_trampoline): Declare.
	* rs6000-tdep.c (rs6000_in_solib_return_trampoline): New
	function.
	(rs6000_skip_trampoline_code): Skip bigtoc fixup code.
	* xcoffread.c (read_xcoff_symtab): Perform the ISFCN function
	check after the CSECT check rather than before it.  Allocate
	separate symtabs for CSECTs whose names begin with '@'.
	(scan_xcoff_symtab): Don't ignore symbols beginning with '@'.
	Activate the misc_func_recorded mechanism for whose names begin
	with '@'.

Tested on powerpc-ibm-aix4.3.3.0.  Okay to apply?

Nicholas Duffek
<nsd@redhat.com>

[patch follows]

Index: gdb/config/rs6000/tm-rs6000.h
===================================================================
diff -up gdb/config/rs6000/tm-rs6000.h gdb/config/rs6000/tm-rs6000.h
--- gdb/config/rs6000/tm-rs6000.h	Fri Apr 27 13:41:03 2001
+++ gdb/config/rs6000/tm-rs6000.h	Fri Apr 27 12:09:00 2001
@@ -36,6 +36,13 @@ extern char *pc_load_segment_name (CORE_
 #undef CPLUS_MARKER
 #define CPLUS_MARKER '.'
 
+/* Return whether PC in function NAME is in code that should be skipped when
+   single-stepping.  */
+
+#define IN_SOLIB_RETURN_TRAMPOLINE(pc, name) \
+  rs6000_in_solib_return_trampoline (pc, name)
+extern int rs6000_in_solib_return_trampoline (CORE_ADDR, char *);
+
 /* If PC is in some function-call trampoline code, return the PC
    where the function itself actually starts.  If not, return NULL.  */
 
Index: gdb/rs6000-tdep.c
===================================================================
diff -up gdb/rs6000-tdep.c gdb/rs6000-tdep.c
--- gdb/rs6000-tdep.c	Fri Apr 27 13:41:08 2001
+++ gdb/rs6000-tdep.c	Fri Apr 27 13:40:52 2001
@@ -1045,19 +1045,55 @@ rs6000_extract_return_value (struct type
 
 static CORE_ADDR rs6000_struct_return_address;
 
-/* Indirect function calls use a piece of trampoline code to do context
-   switching, i.e. to set the new TOC table. Skip such code if we are on
-   its first instruction (as when we have single-stepped to here). 
-   Also skip shared library trampoline code (which is different from
+/* Return whether handle_inferior_event() should proceed through code
+   starting at PC in function NAME when stepping.
+
+   The AIX -bbigtoc linker option generates functions @FIX0, @FIX1, etc. to
+   handle memory references that are too distant to fit in instructions
+   generated by the compiler.  For example, if 'foo' in the following
+   instruction:
+
+     lwz r9,foo(r2)
+
+   is greater than 32767, the linker might replace the lwz with a branch to
+   somewhere in @FIX1 that does the load in 2 instructions and then branches
+   back to where execution should continue.
+
+   GDB should silently step over @FIX code, just like AIX dbx does.
+   Unfortunately, the linker uses the "b" instruction for the branches,
+   meaning that the link register doesn't get set.  Therefore, GDB's usual
+   step_over_function() mechanism won't work.
+
+   Instead, use the IN_SOLIB_RETURN_TRAMPOLINE and SKIP_TRAMPOLINE_CODE hooks
+   in handle_inferior_event() to skip past @FIX code.  */
+
+int
+rs6000_in_solib_return_trampoline (CORE_ADDR pc, char *name)
+{
+  return name && !strncmp (name, "@FIX", 4);
+}
+
+/* Skip code that the user doesn't want to see when stepping:
+
+   1. Indirect function calls use a piece of trampoline code to do context
+   switching, i.e. to set the new TOC table.  Skip such code if we are on
+   its first instruction (as when we have single-stepped to here).
+
+   2. Skip shared library trampoline code (which is different from
    indirect function call trampolines).
+
+   3. Skip bigtoc fixup code.
+
    Result is desired PC to step until, or NULL if we are not in
-   trampoline code.  */
+   code that should be skipped.  */
 
 CORE_ADDR
 rs6000_skip_trampoline_code (CORE_ADDR pc)
 {
   register unsigned int ii, op;
+  int rel;
   CORE_ADDR solib_target_pc;
+  struct minimal_symbol *msymbol;
 
   static unsigned trampoline_code[] =
   {
@@ -1070,6 +1106,21 @@ rs6000_skip_trampoline_code (CORE_ADDR p
     0x4e800020,			/*    br                */
     0
   };
+
+  /* Check for bigtoc fixup code.  */
+  msymbol = lookup_minimal_symbol_by_pc (pc);
+  if (msymbol && rs6000_in_solib_return_trampoline (pc, SYMBOL_NAME (msymbol)))
+    {
+      /* Double-check that the third instruction from PC is relative "b".  */
+      op = read_memory_integer (pc + 8, 4);
+      if ((op & 0xfc000003) == 0x48000000)
+	{
+	  /* Extract bits 6-29 as a signed 24-bit relative word address and
+	     add it to the containing PC.  */
+	  rel = ((int)(op << 6) >> 6);
+	  return pc + 8 + rel;
+	}
+    }
 
   /* If pc is in a shared library trampoline, return its target.  */
   solib_target_pc = find_solib_trampoline_target (pc);
Index: gdb/xcoffread.c
===================================================================
diff -up gdb/xcoffread.c gdb/xcoffread.c
--- gdb/xcoffread.c	Fri Apr 27 13:41:13 2001
+++ gdb/xcoffread.c	Fri Apr 27 12:52:10 2001
@@ -1100,14 +1100,6 @@ read_xcoff_symtab (pst)
 	  /* done with all files, everything from here on is globals */
 	}
 
-      /* if explicitly specified as a function, treat is as one. */
-      if (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF)
-	{
-	  bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
-				0, cs->c_naux, &main_aux);
-	  goto function_entry_point;
-	}
-
       if ((cs->c_sclass == C_EXT || cs->c_sclass == C_HIDEXT)
 	  && cs->c_naux == 1)
 	{
@@ -1177,7 +1169,8 @@ read_xcoff_symtab (pst)
 						SECT_OFF_TEXT (objfile));
 		      file_end_addr = file_start_addr + CSECT_LEN (&main_aux);
 
-		      if (cs->c_name && cs->c_name[0] == '.')
+		      if (cs->c_name && (cs->c_name[0] == '.'
+					 || cs->c_name[0] == '@'))
 			{
 			  last_csect_name = cs->c_name;
 			  last_csect_val = cs->c_value;
@@ -1251,6 +1244,16 @@ read_xcoff_symtab (pst)
 	    }
 	}
 
+      /* If explicitly specified as a function, treat is as one.  This check
+	 evaluates to true for @FIX* bigtoc CSECT symbols, so it must occur
+	 after the above CSECT check.  */
+      if (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF)
+	{
+	  bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
+				0, cs->c_naux, &main_aux);
+	  goto function_entry_point;
+	}
+
       switch (cs->c_sclass)
 	{
 
@@ -2295,14 +2298,8 @@ scan_xcoff_symtab (objfile)
 	    else
 	      csect_aux = main_aux[0];
 
-	    /* If symbol name starts with ".$" or "$", ignore it. 
-
-	       A symbol like "@FIX1" introduces a section for -bbigtoc jump
-	       tables, which contain anonymous linker-generated code. 
-	       Ignore those sections to avoid "pc 0x... in read in psymtab,
-	       but not in symtab" warnings from find_pc_sect_symtab.  */
-
-	    if (namestring[0] == '$' || namestring[0] == '@'
+	    /* If symbol name starts with ".$" or "$", ignore it.  */
+	    if (namestring[0] == '$'
 		|| (namestring[0] == '.' && namestring[1] == '$'))
 	      break;
 
@@ -2348,7 +2345,11 @@ scan_xcoff_symtab (objfile)
 			       objfile->static_psymbols.next);
 			  }
 		      }
-		    if (namestring && namestring[0] == '.')
+		    /* Activate the misc_func_recorded mechanism for
+		       compiler- and linker-generated CSECTs like ".strcmp"
+		       and "@FIX1".  */ 
+		    if (namestring && (namestring[0] == '.'
+				       || namestring[0] == '@'))
 		      {
 			last_csect_name = namestring;
 			last_csect_val = symbol.n_value;

Re: RFA: [symfile.c} Fix to symbol_file_add()

[Elena Zannoni]

Fernando Nasser writes:
 > Paul N. Hilfinger has pointed out to me that a few operations should be
 > done every time a new symbol file is read.  This was an oversight in a
 > patch I wrote in January.  The small patch attached fixes that.
 > 
 > ChangeLog:
 > 
 > 	* symfile.c (symbol_file_command): Move cleanup operations
 > 	from here...
 > 	(symbol_file_add): ...to here, so they are run every time a new
 > 	symbol file is read.
 > 

This would change the behavior of all the calls to symbol_file_add().
Which may not be what you want. Later on in symfile.c there is another
call to symbol_file_add which is followed by reinit_frame_cache(). In
this case the same thing would be done twice, and in other calls it
would be done, when it was not done before.  I am not sure why the
other calls to symbol_file_add don't reinititalize things, but it
seems a little too risky to do this, w/o making sure those platforms
are not affected.

coff-solib.c:91:	  objfile = symbol_file_add (filename, from_tty,
cxux-nat.c:370:  objfile = symbol_file_add (LIBC_FILE, 0, NULL, 0, OBJF_READNOW);
cxux-nat.c:390:	      symbol_file_add (path_name, 1, &section_addrs, 0, 0);
irix5-nat.c:850:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
osfsolib.c:585:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
pa64solib.c:273:  so->objfile = symbol_file_add (name, from_tty, &section_addrs, 0, OBJF_SHARED);
remote-udi.c:1291:  symbol_file_add (strtok (args, " \t"), from_tty, NULL, 1, 0);
remote-vx.c:670:  objfile = symbol_file_add (name, from_tty, NULL, 0, 0);
solib.c:313:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
somsolib.c:290:  so->objfile = symbol_file_add (name, from_tty, NULL, 0, OBJF_SHARED);
symfile.c:905:  symbol_file_add (args, from_tty, NULL, 1, 0);
symfile.c:982:		  symbol_file_add (name, from_tty, NULL, 1, flags);
symfile.c:1549:  symbol_file_add (filename, from_tty, &section_addrs, 0, flags);
win32-nat.c:444:  p->ret = symbol_file_add (p->name, p->from_tty, p->addrs, p->mainline, p->flags);


Since the original code substitution was to replace calls to
symbol_file_command() with symbol_file_add_main(), I would think that
the lines below should be left in the symbol_file_command function and
added to symbol_file_add_main() instead, therefore preserving the
original semantics.

Elena


 > --
 > Fernando Nasser
 > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > 2323 Yonge Street, Suite #300
 > Toronto, Ontario   M4P 2C9Index: 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 ----

Re: RFA: [symfile.c} Fix to symbol_file_add()

[Fernando Nasser]

Thanks for reviewing my patch so promptly Elena.  Please see my comments
below.

Elena Zannoni wrote:
> 
> Fernando Nasser writes:
>  > Paul N. Hilfinger has pointed out to me that a few operations should be
>  > done every time a new symbol file is read.  This was an oversight in a
>  > patch I wrote in January.  The small patch attached fixes that.
>  >
>  > ChangeLog:
>  >
>  >      * symfile.c (symbol_file_command): Move cleanup operations
>  >      from here...
>  >      (symbol_file_add): ...to here, so they are run every time a new
>  >      symbol file is read.
>  >
> 
> This would change the behavior of all the calls to symbol_file_add().
> Which may not be what you want.


You are right, this bits:
>  > + #ifdef HPUXHPPA
>  > +   RESET_HP_UX_GLOBALS ();
>  > + #endif
>  > +
>  > +   set_initial_language ();

should only be done when a new "main" is loaded.  I suggested moving it
to symbol_file_add_main(), which is called exactly in those cases.

However, the bit:
>  > +   /* Getting new symbols may change our opinion about
>  > +      what is frameless.  */
>  > +   reinit_frame_cache ();

should _always_ be done, whenever we load new symbols.

What happens today, and happened _before_ my patch (it hasn't changed
any call to symbol_file_add() nor symbol_file_add() itself except for
the clear part), is that _some_ of the callers do it and some do not.

I guess this will be very difficult to control unless we move the
reinit_frame_cache () call to inside symbol_file_add().

So, I would, if you agree, post a patch (and ask Paul to help me test
it) that does:

1) Move the HP and set_initial_language() bits to symbol_file_add_main()

2) Move the reinit_frame_cache () bit to symbol_file_add(), as the
current patch does.

3) Remove the duplicate call to reinit_frame_cache () from the targets.


How does it sound?


Regards,
Fernando



> Later on in symfile.c there is another
> call to symbol_file_add which is followed by reinit_frame_cache(). In
> this case the same thing would be done twice, and in other calls it
> would be done, when it was not done before.  I am not sure why the
> other calls to symbol_file_add don't reinititalize things, but it
> seems a little too risky to do this, w/o making sure those platforms
> are not affected.
> 
> coff-solib.c:91:          objfile = symbol_file_add (filename, from_tty,
> cxux-nat.c:370:  objfile = symbol_file_add (LIBC_FILE, 0, NULL, 0, OBJF_READNOW);
> cxux-nat.c:390:       symbol_file_add (path_name, 1, &section_addrs, 0, 0);
> irix5-nat.c:850:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
> osfsolib.c:585:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
> pa64solib.c:273:  so->objfile = symbol_file_add (name, from_tty, &section_addrs, 0, OBJF_SHARED);
> remote-udi.c:1291:  symbol_file_add (strtok (args, " \t"), from_tty, NULL, 1, 0);
> remote-vx.c:670:  objfile = symbol_file_add (name, from_tty, NULL, 0, 0);
> solib.c:313:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
> somsolib.c:290:  so->objfile = symbol_file_add (name, from_tty, NULL, 0, OBJF_SHARED);
> symfile.c:905:  symbol_file_add (args, from_tty, NULL, 1, 0);
> symfile.c:982:            symbol_file_add (name, from_tty, NULL, 1, flags);
> symfile.c:1549:  symbol_file_add (filename, from_tty, &section_addrs, 0, flags);
> win32-nat.c:444:  p->ret = symbol_file_add (p->name, p->from_tty, p->addrs, p->mainline, p->flags);
> 
> Since the original code substitution was to replace calls to
> symbol_file_command() with symbol_file_add_main(), I would think that
> the lines below should be left in the symbol_file_command function and
> added to symbol_file_add_main() instead, therefore preserving the
> original semantics.
> 
> Elena
> 
>  > --
>  > Fernando Nasser
>  > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
>  > 2323 Yonge Street, Suite #300
>  > Toronto, Ontario   M4P 2C9Index: 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 ----

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: [symfile.c} Fix to symbol_file_add()

[Elena Zannoni]

Fernando Nasser writes:
 > Thanks for reviewing my patch so promptly Elena.  Please see my comments
 > below.
 > 
 > Elena Zannoni wrote:
 > > 
 > > Fernando Nasser writes:
 > >  > Paul N. Hilfinger has pointed out to me that a few operations should be
 > >  > done every time a new symbol file is read.  This was an oversight in a
 > >  > patch I wrote in January.  The small patch attached fixes that.
 > >  >
 > >  > ChangeLog:
 > >  >
 > >  >      * symfile.c (symbol_file_command): Move cleanup operations
 > >  >      from here...
 > >  >      (symbol_file_add): ...to here, so they are run every time a new
 > >  >      symbol file is read.
 > >  >
 > > 
 > > This would change the behavior of all the calls to symbol_file_add().
 > > Which may not be what you want.
 > 
 > 
 > You are right, this bits:
 > >  > + #ifdef HPUXHPPA
 > >  > +   RESET_HP_UX_GLOBALS ();
 > >  > + #endif
 > >  > +
 > >  > +   set_initial_language ();
 > 
 > should only be done when a new "main" is loaded.  I suggested moving it
 > to symbol_file_add_main(), which is called exactly in those cases.
 > 

Yes, agreed.

 > However, the bit:
 > >  > +   /* Getting new symbols may change our opinion about
 > >  > +      what is frameless.  */
 > >  > +   reinit_frame_cache ();
 > 
 > should _always_ be done, whenever we load new symbols.
 > 
 > What happens today, and happened _before_ my patch (it hasn't changed
 > any call to symbol_file_add() nor symbol_file_add() itself except for
 > the clear part), is that _some_ of the callers do it and some do not.
 > 

Yes, true. But I wonder if that was intended. What happens if you
manually load a shared library? Do you need to clear the frame cache
at that point?

 > I guess this will be very difficult to control unless we move the
 > reinit_frame_cache () call to inside symbol_file_add().
 > 
 > So, I would, if you agree, post a patch (and ask Paul to help me test
 > it) that does:
 > 
 > 1) Move the HP and set_initial_language() bits to symbol_file_add_main()
 > 

OK.

 > 2) Move the reinit_frame_cache () bit to symbol_file_add(), as the
 > current patch does.
 > 

I need to understand better what happens with shared libraries.
SOLIB_ADD ends up calling this. I see it used in the attach command
and in this case a reinit_frame_cache is OK, so is for the case in
sol-thread.c but what about the other calls?

 > 3) Remove the duplicate call to reinit_frame_cache () from the targets.
 > 

After 2 is resolved.

 > 
 > How does it sound?

Mostly ok. 


Elena

 > 
 > 
 > Regards,
 > Fernando
 > 
 > 
 > 
 > > Later on in symfile.c there is another
 > > call to symbol_file_add which is followed by reinit_frame_cache(). In
 > > this case the same thing would be done twice, and in other calls it
 > > would be done, when it was not done before.  I am not sure why the
 > > other calls to symbol_file_add don't reinititalize things, but it
 > > seems a little too risky to do this, w/o making sure those platforms
 > > are not affected.
 > > 
 > > coff-solib.c:91:          objfile = symbol_file_add (filename, from_tty,
 > > cxux-nat.c:370:  objfile = symbol_file_add (LIBC_FILE, 0, NULL, 0, OBJF_READNOW);
 > > cxux-nat.c:390:       symbol_file_add (path_name, 1, &section_addrs, 0, 0);
 > > irix5-nat.c:850:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
 > > osfsolib.c:585:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
 > > pa64solib.c:273:  so->objfile = symbol_file_add (name, from_tty, &section_addrs, 0, OBJF_SHARED);
 > > remote-udi.c:1291:  symbol_file_add (strtok (args, " \t"), from_tty, NULL, 1, 0);
 > > remote-vx.c:670:  objfile = symbol_file_add (name, from_tty, NULL, 0, 0);
 > > solib.c:313:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
 > > somsolib.c:290:  so->objfile = symbol_file_add (name, from_tty, NULL, 0, OBJF_SHARED);
 > > symfile.c:905:  symbol_file_add (args, from_tty, NULL, 1, 0);
 > > symfile.c:982:            symbol_file_add (name, from_tty, NULL, 1, flags);
 > > symfile.c:1549:  symbol_file_add (filename, from_tty, &section_addrs, 0, flags);
 > > win32-nat.c:444:  p->ret = symbol_file_add (p->name, p->from_tty, p->addrs, p->mainline, p->flags);
 > > 
 > > Since the original code substitution was to replace calls to
 > > symbol_file_command() with symbol_file_add_main(), I would think that
 > > the lines below should be left in the symbol_file_command function and
 > > added to symbol_file_add_main() instead, therefore preserving the
 > > original semantics.
 > > 
 > > Elena
 > > 
 > >  > --
 > >  > Fernando Nasser
 > >  > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > >  > 2323 Yonge Street, Suite #300
 > >  > Toronto, Ontario   M4P 2C9Index: 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 ----
 > 
 > -- 
 > Fernando Nasser
 > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > 2323 Yonge Street, Suite #300
 > Toronto, Ontario   M4P 2C9
 > 

Re: RFA: [symfile.c} Fix to symbol_file_add()

[Fernando Nasser]

Elena Zannoni wrote:
> 
>  > However, the bit:
>  > >  > +   /* Getting new symbols may change our opinion about
>  > >  > +      what is frameless.  */
>  > >  > +   reinit_frame_cache ();
>  >
>  > should _always_ be done, whenever we load new symbols.
>  >
> 
> I need to understand better what happens with shared libraries.
> SOLIB_ADD ends up calling this. I see it used in the attach command
> and in this case a reinit_frame_cache is OK, so is for the case in
> sol-thread.c but what about the other calls?
> 

The comment "Getting new symbols may change our opinion about what is
frameless." basically answers your question.  GDB is much more reliable
at figuring out where the prologue ends when symbols are available.  If
you are stopped inside a function (in a shared library) and you did not
had the symbols when you created the frame cache, you did a less job
than you could have done if you had symbols.

That is why the frame cache is always reset, so the frames can be
reconstructed with mode information and, thus, more precisely. 

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

RFA: [symfile.c} Fix to symbol_file_add() [REPOST]

[Fernando Nasser]

After the fruitful discussions with Elena, I believe I have a better
patch, so please consider this one instead.

As Elena explained, we must let the callers of symbol_file_add() do the
cache reset as we may be loading several shared libraries.  Doing the
cache reset for each one is too much overhead as we will have to grab
the data for filling the current frame every single time.  Well,
symbol_file_add_main () is a caller, so I guess it can go there.  And
none of its callers has a a call to reinit_frame_cache() as they used to
call symbol_file_command().

I guess the previous behavior is being safely restored now.

However, several of the callers of symbol_file_add() do not call
reinit_frame_cache() as they should.  But as far as I can tell, this has
always been like that.  The only thing that is lost is the possibility
to get a better frame description of the current stack by using the
newly read symbols.  I don't know enough about all this targets to know
if this is important (I don't know why they are reading symbol files). 
The shared library case is already taken care of as Elena has pointed
out.



ChangeLog:

        * symfile.c (symbol_file_command): Move cleanup operations
        from here...
        (symbol_file_add_main): ...to here, so they are run every time
        a new main symbol file is read.


-- 
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/05/02 15:35:32
*************** symbol_file_add (char *name, int from_tt
*** 896,908 ****
    return (objfile);
  }
  
! /* Just call the above with default values.
!    Used when the file is supplied in the gdb command line. */
     
  void
  symbol_file_add_main (char *args, int from_tty)
  {
    symbol_file_add (args, from_tty, NULL, 1, 0);
  }
  
  void
--- 896,920 ----
    return (objfile);
  }
  
! /* Call the above with default values and update whatever is
!    affected by a new main().
!    Used when the file is supplied in the gdb command line
!    and by some targets. */
     
  void
  symbol_file_add_main (char *args, int from_tty)
  {
    symbol_file_add (args, from_tty, NULL, 1, 0);
+ 
+ #ifdef HPUXHPPA
+   RESET_HP_UX_GLOBALS ();
+ #endif
+ 
+   /* Getting new symbols may change our opinion about
+      what is frameless.  */
+   reinit_frame_cache ();
+ 
+   set_initial_language ();
  }
  
  void
*************** 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++;
  	}
--- 992,997 ----
From ezannoni@cygnus.com Wed May 02 09:06:00 2001
From: Elena Zannoni <ezannoni@cygnus.com>
To: Fernando Nasser <fnasser@redhat.com>
Cc: Elena Zannoni <ezannoni@cygnus.com>, gdb-patches@sources.redhat.com, hilfingr@otisco.mckusick.com
Subject: Re: RFA: [symfile.c} Fix to symbol_file_add() [REPOST]
Date: Wed, 02 May 2001 09:06:00 -0000
Message-id: <15088.12423.711167.908434@kwikemart.cygnus.com>
References: <3AED8391.C6B9A456@redhat.com> <15085.43538.55216.581538@kwikemart.cygnus.com> <3AEDF6A3.440D0C62@redhat.com> <15086.11696.278572.217415@kwikemart.cygnus.com> <3AEEB0E2.3232D2DE@redhat.com> <15087.31793.429533.289522@kwikemart.cygnus.com> <3AF02D6E.333B2AAB@redhat.com>
X-SW-Source: 2001-05/msg00010.html
Content-length: 3520

Thanks Fernando!
But shouldn't we be leaving the stuff in symbol_file_command?
I.e. only adding it to the new function?
 

Elena


Fernando Nasser writes:
 > After the fruitful discussions with Elena, I believe I have a better
 > patch, so please consider this one instead.
 > 
 > As Elena explained, we must let the callers of symbol_file_add() do the
 > cache reset as we may be loading several shared libraries.  Doing the
 > cache reset for each one is too much overhead as we will have to grab
 > the data for filling the current frame every single time.  Well,
 > symbol_file_add_main () is a caller, so I guess it can go there.  And
 > none of its callers has a a call to reinit_frame_cache() as they used to
 > call symbol_file_command().
 > 
 > I guess the previous behavior is being safely restored now.
 > 
 > However, several of the callers of symbol_file_add() do not call
 > reinit_frame_cache() as they should.  But as far as I can tell, this has
 > always been like that.  The only thing that is lost is the possibility
 > to get a better frame description of the current stack by using the
 > newly read symbols.  I don't know enough about all this targets to know
 > if this is important (I don't know why they are reading symbol files). 
 > The shared library case is already taken care of as Elena has pointed
 > out.
 > 
 > 
 > 
 > ChangeLog:
 > 
 >         * symfile.c (symbol_file_command): Move cleanup operations
 >         from here...
 >         (symbol_file_add_main): ...to here, so they are run every time
 >         a new main symbol file is read.
 > 
 > 
 > -- 
 > Fernando Nasser
 > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > 2323 Yonge Street, Suite #300
 > Toronto, Ontario   M4P 2C9Index: 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/05/02 15:35:32
 > *************** symbol_file_add (char *name, int from_tt
 > *** 896,908 ****
 >     return (objfile);
 >   }
 >   
 > ! /* Just call the above with default values.
 > !    Used when the file is supplied in the gdb command line. */
 >      
 >   void
 >   symbol_file_add_main (char *args, int from_tty)
 >   {
 >     symbol_file_add (args, from_tty, NULL, 1, 0);
 >   }
 >   
 >   void
 > --- 896,920 ----
 >     return (objfile);
 >   }
 >   
 > ! /* Call the above with default values and update whatever is
 > !    affected by a new main().
 > !    Used when the file is supplied in the gdb command line
 > !    and by some targets. */
 >      
 >   void
 >   symbol_file_add_main (char *args, int from_tty)
 >   {
 >     symbol_file_add (args, from_tty, NULL, 1, 0);
 > + 
 > + #ifdef HPUXHPPA
 > +   RESET_HP_UX_GLOBALS ();
 > + #endif
 > + 
 > +   /* Getting new symbols may change our opinion about
 > +      what is frameless.  */
 > +   reinit_frame_cache ();
 > + 
 > +   set_initial_language ();
 >   }
 >   
 >   void
 > *************** 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++;
 >   	}
 > --- 992,997 ----

Re: RFA: [symfile.c} Fix to symbol_file_add() [REPOST]

[Fernando Nasser]

Elena Zannoni wrote:
> 
> Thanks Fernando!
> But shouldn't we be leaving the stuff in symbol_file_command?
> I.e. only adding it to the new function?
> 

Yes, of course.  It calls symbol_file_add(), not symbol_file_add_main()
because of the flags.  Silly me.

Yes, please consider the patch without the removal part.

P.S.: I guess  symbol_file_add_main() should have the flags argument,
but I guess this thing has been going on for too long now for me to go
after all callers and add the argument.


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: [symfile.c} Fix to symbol_file_add() [REPOST]

[Elena Zannoni]

Fernando Nasser writes:
 > Elena Zannoni wrote:
 > > 
 > > Thanks Fernando!
 > > But shouldn't we be leaving the stuff in symbol_file_command?
 > > I.e. only adding it to the new function?
 > > 
 > 
 > Yes, of course.  It calls symbol_file_add(), not symbol_file_add_main()
 > because of the flags.  Silly me.
 > 
 > Yes, please consider the patch without the removal part.

Ok, sure.  Check it in. Just for completion can you post the final
diff once you are done?

Thanks
Elena


 > 
 > P.S.: I guess  symbol_file_add_main() should have the flags argument,
 > but I guess this thing has been going on for too long now for me to go
 > after all callers and add the argument.

Add a FIXME/comment.

Elena


 > 
 > 
 > -- 
 > Fernando Nasser
 > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
 > 2323 Yonge Street, Suite #300
 > Toronto, Ontario   M4P 2C9
 > 

Re: RFA: [symfile.c} Fix to symbol_file_add() [REPOST]

[Fernando Nasser]

Elena Zannoni wrote:
> 
> Fernando Nasser writes:
>  > Elena Zannoni wrote:
>  > >
>  > > Thanks Fernando!
>  > > But shouldn't we be leaving the stuff in symbol_file_command?
>  > > I.e. only adding it to the new function?
>  > >
>  >
>  > Yes, of course.  It calls symbol_file_add(), not symbol_file_add_main()
>  > because of the flags.  Silly me.
>  >
>  > Yes, please consider the patch without the removal part.
> 
> Ok, sure.  Check it in. Just for completion can you post the final
> diff once you are done?
> 

Absolutely.


> Thanks
> Elena
> 
>  >
>  > P.S.: I guess  symbol_file_add_main() should have the flags argument,
>  > but I guess this thing has been going on for too long now for me to go
>  > after all callers and add the argument.
> 
> Add a FIXME/comment.
> 

Good idea.


Thank you for all your help.


P.S.: I will have to do all this tomorrow afternoon as I have to prepare
a presentation.


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: [symfile.c} Fix to symbol_file_add() [REPOST]

[Fernando Nasser]

Checked in.

Thanks again Elena.

Fernando


> ChangeLog:
> 
>         * symfile.c (symbol_file_add_main_1): New static function.
> Passes
>         the flags arguments to  symbol_file_add() and takes care of any
>         necessary reinitializations.
>         (symbol_file_command): Call symbol_file_add_main_1() instead of
>         symbol_file_add().
>         (symbol_file_add_main): Ditto.
> 
> 
>   ------------------------------------------------------------------------
> 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/05/07 15:44:48
> *************** static void set_initial_language (void);
> *** 109,114 ****
> --- 109,116 ----
> 
>   static void load_command (char *, int);
> 
> + static void symbol_file_add_main_1 (char *args, int from_tty, int flags);
> +
>   static void add_symbol_file_command (char *, int);
> 
>   static void add_shared_symbol_files_command (char *, int);
> *************** symbol_file_add (char *name, int from_tt
> *** 896,908 ****
>     return (objfile);
>   }
> 
> ! /* Just call the above with default values.
> !    Used when the file is supplied in the gdb command line. */
> 
>   void
>   symbol_file_add_main (char *args, int from_tty)
>   {
> !   symbol_file_add (args, from_tty, NULL, 1, 0);
>   }
> 
>   void
> --- 898,931 ----
>     return (objfile);
>   }
> 
> ! /* Call symbol_file_add() with default values and update whatever is
> !    affected by the loading of a new main().
> !    Used when the file is supplied in the gdb command line
> !    and by some targets with special loading requirements.
> !    The auxiliary function, symbol_file_add_main_1(), has the flags
> !    argument for the switches that can only be specified in the symbol_file
> !    command itself.  */
> 
>   void
>   symbol_file_add_main (char *args, int from_tty)
> + {
> +   symbol_file_add_main_1 (args, from_tty, 0);
> + }
> +
> + static void
> + symbol_file_add_main_1 (char *args, int from_tty, int flags)
>   {
> !   symbol_file_add (args, 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 ();
>   }
> 
>   void
> *************** symbol_file_command (char *args, int fro
> *** 979,993 ****
>               else
>                 {
>                     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++;
>         }
> --- 1002,1009 ----
>               else
>                 {
>                     name = *argv;
> 
> !                 symbol_file_add_main_1 (name, from_tty, flags);
>                 }
>           argv++;
>         }

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: RFA: [symfile.c} Fix to symbol_file_add() [REPOST]

[Fernando Nasser]

Final version of the patch to be checked in.

OK to check in now?

Thanks once again for your help Elena.


ChangeLog:

        * symfile.c (symbol_file_add_main_1): New static function. 
Passes
	the flags arguments to  symbol_file_add() and takes care of any
	necessary reinitializations.
	(symbol_file_command): Call symbol_file_add_main_1() instead of
	symbol_file_add().  
        (symbol_file_add_main): Ditto. 


-- 
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/05/07 15:44:48
*************** static void set_initial_language (void);
*** 109,114 ****
--- 109,116 ----
  
  static void load_command (char *, int);
  
+ static void symbol_file_add_main_1 (char *args, int from_tty, int flags);
+ 
  static void add_symbol_file_command (char *, int);
  
  static void add_shared_symbol_files_command (char *, int);
*************** symbol_file_add (char *name, int from_tt
*** 896,908 ****
    return (objfile);
  }
  
! /* Just call the above with default values.
!    Used when the file is supplied in the gdb command line. */
     
  void
  symbol_file_add_main (char *args, int from_tty)
  {
!   symbol_file_add (args, from_tty, NULL, 1, 0);
  }
  
  void
--- 898,931 ----
    return (objfile);
  }
  
! /* Call symbol_file_add() with default values and update whatever is
!    affected by the loading of a new main().
!    Used when the file is supplied in the gdb command line
!    and by some targets with special loading requirements.
!    The auxiliary function, symbol_file_add_main_1(), has the flags
!    argument for the switches that can only be specified in the symbol_file
!    command itself.  */
     
  void
  symbol_file_add_main (char *args, int from_tty)
+ {
+   symbol_file_add_main_1 (args, from_tty, 0);
+ }
+ 
+ static void
+ symbol_file_add_main_1 (char *args, int from_tty, int flags)
  {
!   symbol_file_add (args, 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 ();
  }
  
  void
*************** symbol_file_command (char *args, int fro
*** 979,993 ****
  	      else
  		{
                    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++;
  	}
--- 1002,1009 ----
  	      else
  		{
                    name = *argv;
  
! 		  symbol_file_add_main_1 (name, from_tty, flags);
  		}
  	  argv++;
  	}
From fnasser@redhat.com Mon May 07 09:10:00 2001
From: Fernando Nasser <fnasser@redhat.com>
To: Keith Seitz <keiths@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] gdbtk testsuite enhancements - infrastructure
Date: Mon, 07 May 2001 09:10:00 -0000
Message-id: <3AF6C889.CEF2ED52@redhat.com>
References: <Pine.GSO.4.33.0105070854530.15434-100000@ryobi.cygnus.com>
X-SW-Source: 2001-05/msg00085.html
Content-length: 9951

We have previously discussed this with Keith in the Insight list.

If nobody objects, Keith will make this change.

Fernando


Keith Seitz wrote:
> 
> Hi,
> 
> I have completed a bunch of work to enhance the gdbtk (aka Insight)
> testsuite. This patch adds functionality to gdb's dejagnu testsuite
> infrastructure to better support gui testing. Specifically, it
> incorporates the ability to run the tests off-screen if Xvfb is installed.
> 
> Executive summary of how it will all affect the average gdb contributor:
> If env variable GDB_DISPLAY is set, it will run gdbtk tests using that
> display. If GDB_DISPLAY is not set, testsuite will attempt to run Xvfb for
> testsing. If Xvfb is not found, tests are skipped.
> 
> So, to test on unix, either set GDB_DISPLAY=$DISPLAY (and don't touch
> anything) or put Xvfb in your path. On Cygwin, you must set GDB_DISPLAY
> (to anything) to run the tests.
> 
> There are accompanying gdb.gdbtk/*.exp changes to accompany this. See
> follow-up.
> 
> Keith
> 
> testsuite/ChangeLog:
> 2001-05-07  Keith Seitz  <keiths@cygnus.com>
> 
>         * lib/gdb.exp (gdbtk_initialize_display): New proc which will
>         set up the display for testing.
>         (gdbtk_start): Convert all paths to paths that tcl will like.
>         Export target information to environment.
>         (_gdbtk_xvfb_init): New proc to start Xvfb if available and
>         necessary.
>         (_gdbtk_xvfb_exit): New proc to kill Xvfb if necessary.
>         (to_tcl_path): New proc to convert a given pathname into
>         a path acceptible as an argument to a tcl command.
>         (_gdbtk_export_target_info): New proc to export target info
>         into the environment for gdbtk testing.
>         (gdbtk_done): New proc to signal end-of-test.
> 
> Patch:
> Index: gdb/testsuite/lib/gdb.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
> retrieving revision 1.7
> diff -u -r1.7 gdb.exp
> --- gdb/testsuite/lib/gdb.exp   2001/03/06 08:22:01     1.7
> +++ gdb/testsuite/lib/gdb.exp   2001/05/07 15:53:17
> @@ -1599,6 +1599,30 @@
>    }
>  }
> 
> +# Initializes the display for gdbtk testing.
> +# Returns 1 if tests should run, 0 otherwise.
> +proc gdbtk_initialize_display {} {
> +  global _using_windows
> +
> +  # This is hacky, but, we don't have much choice. When running
> +  # expect under Windows, tcl_platform(platform) is "unix".
> +  if {![info exists _using_windows]} {
> +    set _using_windows [expr {![catch {exec cygpath --help}]}]
> +  }
> +
> +  if {![_gdbtk_xvfb_init]} {
> +    if {$_using_windows} {
> +      untested "No GDB_DISPLAY -- skipping tests"
> +    } else {
> +      untested "No GDB_DISPLAY or Xvfb -- skipping tests"
> +    }
> +
> +    return 0
> +  }
> +
> +  return 1
> +}
> +
>  # From dejagnu:
>  # srcdir = testsuite src dir (e.g., devo/gdb/testsuite)
>  # objdir = testsuite obj dir (e.g., gdb/testsuite)
> @@ -1632,34 +1656,34 @@
>      }
>    }
> 
> -
>    set wd [pwd]
> +
> +  # Find absolute path to test
> +  set test [to_tcl_path -abs $test]
> +
> +  # Set environment variables for tcl libraries and such
>    cd $srcdir
>    set abs_srcdir [pwd]
> -  cd [file join $abs_srcdir .. gdbtk library]
> -  set env(GDBTK_LIBRARY) [pwd]
> -  cd [file join $abs_srcdir .. .. tcl library]
> -  set env(TCL_LIBRARY) [pwd]
> -  cd [file join $abs_srcdir .. .. tk library]
> -  set env(TK_LIBRARY) [pwd]
> -  cd [file join $abs_srcdir .. .. tix library]
> -  set env(TIX_LIBRARY) [pwd]
> -  cd [file join $abs_srcdir .. .. itcl itcl library]
> -  set env(ITCL_LIBRARY) [pwd]
> -  cd [file join .. $abs_srcdir .. .. libgui library]
> -  set env(CYGNUS_GUI_LIBRARY) [pwd]
> -  cd $wd
> -  cd [file join $abs_srcdir $subdir]
> -  set env(DEFS) [file join [pwd] defs]
> +  set env(GDBTK_LIBRARY) [to_tcl_path -abs [file join $abs_srcdir .. gdbtk library]]
> +  set env(TCL_LIBRARY) [to_tcl_path -abs [file join $abs_srcdir .. .. tcl library]]
> +  set env(TK_LIBRARY) [to_tcl_path -abs [file join $abs_srcdir .. .. tk library]]
> +  set env(TIX_LIBRARY) [to_tcl_path -abs [file join $abs_srcdir .. .. tix library]]
> +  set env(ITCL_LIBRARY) [to_tcl_path -abs [file join $abs_srcdir .. .. itcl itcl library]]
> +  set env(CYGNUS_GUI_LIBRARY) [to_tcl_path -abs [file join .. $abs_srcdir .. .. libgui library]]
> +  set env(DEFS) [to_tcl_path -abs [file join $abs_srcdir $subdir defs]]
> +
>    cd $wd
>    cd [file join $objdir $subdir]
>    set env(OBJDIR) [pwd]
>    cd $wd
> 
> +  # Set info about target into env
> +  _gdbtk_export_target_info
> +
>    set env(SRCDIR) $abs_srcdir
>    set env(GDBTK_VERBOSE) 1
> -  set env(GDBTK_LOGFILE) [file join $objdir gdb.log]
> -  set env(GDBTK_TEST_RUNNING) 1
> +  set env(GDBTK_LOGFILE) [to_tcl_path [file join $objdir gdb.log]]
> +
>    set err [catch {exec $GDB -nx -q --tclcommand=$test} res]
>    if { $err } {
>      perror "Execing $GDB failed: $res"
> @@ -1668,6 +1692,149 @@
>    return $res
>  }
> 
> +# Start xvfb when using it.
> +# The precedence is:
> +#   1. If GDB_DISPLAY is set, use it
> +#   2. If Xvfb exists, use it (not on cygwin)
> +#   3. Skip tests
> +proc _gdbtk_xvfb_init {} {
> +  global env spawn_id _xvfb_spawn_id _using_windows
> +
> +  if {[info exists env(GDB_DISPLAY)]} {
> +    set env(DISPLAY) $env(GDB_DISPLAY)
> +  } elseif {!$_using_windows && [which Xvfb] != 0} {
> +    set screen ":[getpid]"
> +    set pid [spawn  Xvfb $screen]
> +    set _xvfb_spawn_id $spawn_id
> +    set env(DISPLAY) $screen
> +  } else {
> +    # No Xvfb found -- skip test
> +    return 0
> +  }
> +
> +  return 1
> +}
> +
> +# Kill xvfb
> +proc _gdbtk_xvfb_exit {} {
> +  global objdir subdir env _xvfb_spawn_id
> +
> +  if {[info exists _xvfb_spawn_id]} {
> +    exec kill [exp_pid -i $_xvfb_spawn_id]
> +    wait -i $_xvfb_spawn_id
> +  }
> +}
> +
> +# help proc for setting tcl-style paths from unix-style paths
> +# pass "-abs" to make it an absolute path
> +proc to_tcl_path {unix_path {arg {}}} {
> +  global _using_windows
> +
> +  if {[string compare $unix_path "-abs"] == 0} {
> +    set unix_path $arg
> +    set wd [pwd]
> +    cd [file dirname $unix_path]
> +    set dirname [pwd]
> +    set unix_name [file join $dirname [file tail $unix_path]]
> +    cd $wd
> +  }
> +
> +  if {$_using_windows} {
> +    set unix_path [exec cygpath -aw $unix_path]
> +    set unix_path [join [split $unix_path \\] /]
> +  }
> +
> +  return $unix_path
> +}
> +
> +# Set information about the target into the environment
> +# variable TARGET_INFO. This array will contain a list
> +# of commands that are necessary to run a target.
> +#
> +# This is mostly derived from how dejagnu works, what
> +# procs are defined, and analyzing unix.exp, monitor.exp,
> +# and sim.exp.
> +#
> +# Array elements exported:
> +# Index   Meaning
> +# -----   -------
> +# init    list of target/board initialization commands
> +# target  target command for target/board
> +# load    load command for target/board
> +# run     run command for target_board
> +proc _gdbtk_export_target_info {} {
> +  global env
> +
> +  # Figure out what "target class" the testsuite is using,
> +  # i.e., sim, monitor, native
> +  if {[string compare [info proc gdb_target_monitor] gdb_target_monitor] == 0} {
> +    # Using a monitor/remote target
> +    set target monitor
> +  } elseif {[string compare [info proc gdb_target_sim] gdb_target_sim] == 0} {
> +    # Using a simulator target
> +    set target simulator
> +  } else {
> +    # Assume native
> +    set target native
> +  }
> +
> +  # Now setup the array to be exported.
> +  set info(init) {}
> +  set info(target) {}
> +  set info(load) {}
> +  set info(run) {}
> +
> +  switch $target {
> +    simulator {
> +      set opts "[target_info gdb,target_sim_options]"
> +      set info(target) "target sim $opts"
> +      set info(load) "load"
> +      set info(run) "run"
> +    }
> +
> +    monitor {
> +      # Setup options for the connection
> +      if {[target_info exists baud]} {
> +       lappend info(init) "set remotebaud [target_info baud]"
> +      }
> +      if {[target_info exists binarydownload]} {
> +       lappend info(init) "set remotebinarydownload [target_info binarydownload]"
> +      }
> +      if {[target_info exists disable_x_packet]} {
> +       lappend info(init) "set remote X-packet disable"
> +      }
> +      if {[target_info exists disable_z_packet]} {
> +       lappend info(init) "set remote Z-packet disable"
> +      }
> +
> +      # Get target name and connection info
> +      if {[target_info exists gdb_protocol]} {
> +       set targetname "[target_info gdb_protocol]"
> +      } else {
> +       set targetname "not_specified"
> +      }
> +      if {[target_info exists gdb_serial]} {
> +       set serialport "[target_info gdb_serial]"
> +      } elseif {[target_info exists netport]} {
> +       set serialport "[target_info netport]"
> +      } else {
> +       set serialport "[target_info serial]"
> +      }
> +
> +      set info(target) "target $targetname $serialport"
> +      set info(load) "load"
> +      set info(run) "continue"
> +    }
> +
> +    native {
> +      set info(run) "run"
> +    }
> +  }
> +
> +  # Export the array to the environment
> +  set env(TARGET_INFO) [array get info]
> +}
> +
>  # gdbtk tests call this function to print out the results of the
>  # tests. The argument is a proper list of lists of the form:
>  # {status name description msg}. All of these things typically
> @@ -1700,6 +1867,16 @@
>         xpass "$description ($name)"
>        }
>      }
> +  }
> +}
> +
> +proc gdbtk_done {{results {}}} {
> +  global _xvfb_spawn_id
> +  gdbtk_analyze_results $results
> +
> +  # Kill off xvfb if using it
> +  if {[info exists _xvfb_spawn_id]} {
> +    _gdbtk_xvfb_exit
>    }
>  }

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9