gdb.mi/mi-cli.exp failures

gdb.mi/mi-cli.exp failures

[David Carlton]

[ I just got back from vacation, so my apologies if I've missed some
discussion relevant to this. ]

I'm getting a testsuite failure on gdb.mi/mi-cli.exp: the relevant bit
of gdb.log is as follows:

888-interpreter-exec console "set $pc=0x0"
&"frame.c:62: internal-error: get_frame_id: Assertion `!legacy_frame_p (current_gdbarch)' failed.\n"
&"A problem internal to GDB has been detected.  Further\ndebugging may prove unreliable.\n"
ERROR: Process no longer exists
UNRESOLVED: gdb.mi/mi-cli.exp: -interpreter-exec console "set $pc=0x0"
testcase ./gdb.mi/mi-cli.exp completed in 1 seconds

This is with GDB updated an hour or so ago, with i686-pc-linux-gnu,
GCC 3.1, DWARF 2.

David Carlton
carlton@math.stanford.edu

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

> [ I just got back from vacation, so my apologies if I've missed some
> discussion relevant to this. ]
> 
> I'm getting a testsuite failure on gdb.mi/mi-cli.exp: the relevant bit
> of gdb.log is as follows:
> 
> 888-interpreter-exec console "set $pc=0x0"
> &"frame.c:62: internal-error: get_frame_id: Assertion `!legacy_frame_p (current_gdbarch)' failed.\n"
> &"A problem internal to GDB has been detected.  Further\ndebugging may prove unreliable.\n"
> ERROR: Process no longer exists
> UNRESOLVED: gdb.mi/mi-cli.exp: -interpreter-exec console "set $pc=0x0"
> testcase ./gdb.mi/mi-cli.exp completed in 1 seconds
> 
> This is with GDB updated an hour or so ago, with i686-pc-linux-gnu,
> GCC 3.1, DWARF 2.

FYI, my test run from my just committed call_dummy_p change had the 
usual 21 failures.  Yet, mysteriously, for my next set of patches, I'm 
seeing the failures jump to 28.

While I'll note that I'm the prime suspect, did anything else get changed?

Andrew

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

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

>> [ I just got back from vacation, so my apologies if I've missed some
>> discussion relevant to this. ]
>> 
>> I'm getting a testsuite failure on gdb.mi/mi-cli.exp: the relevant bit
>> of gdb.log is as follows:
>> 
>> 888-interpreter-exec console "set $pc=0x0"
>> &"frame.c:62: internal-error: get_frame_id: Assertion `!legacy_frame_p (current_gdbarch)' failed.\n"
>> &"A problem internal to GDB has been detected.  Further\ndebugging may prove unreliable.\n"
>> ERROR: Process no longer exists
>> UNRESOLVED: gdb.mi/mi-cli.exp: -interpreter-exec console "set $pc=0x0"
>> testcase ./gdb.mi/mi-cli.exp completed in 1 seconds
>> 
>> This is with GDB updated an hour or so ago, with i686-pc-linux-gnu,
>> GCC 3.1, DWARF 2.
>> 

> FYI, my test run from my just committed call_dummy_p change had the usual 21 failures.  Yet, mysteriously, for my next set of patches, I'm seeing the failures jump to 28.
> 
> While I'll note that I'm the prime suspect, did anything else get changed?

David,

Applying the attached to BFD fixes the problem ....

CC'ing bfd.

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 17997 bytes --]

? diffs
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/bfd/ChangeLog,v
retrieving revision 1.1976
retrieving revision 1.1975
diff -u -r1.1976 -r1.1975
--- ChangeLog	31 Mar 2003 18:13:24 -0000	1.1976
+++ ChangeLog	29 Mar 2003 01:26:33 -0000	1.1975
@@ -1,20 +1,3 @@
-2003-03-31  David Heine  <dlheine@suif.stanford.edu>
-
-	* aoutx.h (aout_link_hash_table_create): Use bfd_malloc instead of
-	bfd_alloc.
-	* dwarf2.c (concat_filename): Always allocate space for the
-	returned filename.
-	(decode_line_info): Free the allocated filename returned by
-	concat_filename.
-	* elf-eh-frame.c (bfd_elf_write_section_eh_frame): Fix memory leaks.
-	* elf.c (copy_private_bfd_data): Likewise.
-	(_bfd_elf_slurp_version_tables): Fix bug freeing contents pointer.
-	* elflink.h (elf_link_sort_relocs): Fix memory leak.
-	* format.c (bfd_check_format_matches): Likewise.
-	* linker.c (bfd_generic_final_link): Likewise.
-	* opncls.c (find_separate_debug_info): Likewise.
-	* simple.c (bfd_simple_get_relocated_section_contents): Likewise.
-	
 2003-03-28  H.J. Lu <hjl@gnu.org>
 
 	* elflink.h (elf_link_add_object_symbols): Correctly combine
Index: aoutx.h
===================================================================
RCS file: /cvs/src/src/bfd/aoutx.h,v
retrieving revision 1.40
retrieving revision 1.39
diff -u -r1.40 -r1.39
--- aoutx.h	31 Mar 2003 18:13:24 -0000	1.40
+++ aoutx.h	31 Dec 2002 07:29:25 -0000	1.39
@@ -1,6 +1,6 @@
 /* BFD semi-generic back-end for a.out binaries.
    Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 2000,
-   2001, 2002, 2003
+   2001, 2002
    Free Software Foundation, Inc.
    Written by Cygnus Support.
 
@@ -105,7 +105,9 @@
 	in the @file{config/@var{XXX}.mt} file, and modify @file{configure.in}
 	to use the
 	@file{@var{XXX}.mt} file (by setting "<<bfd_target=XXX>>") when your
-	configuration is selected.  */
+	configuration is selected.
+
+*/
 
 /* Some assumptions:
    * Any BFD with D_PAGED set is ZMAGIC, and vice versa.
@@ -155,8 +157,9 @@
 	The standard records contain only an
 	address, a symbol index, and a type field. The extended records
 	(used on 29ks and sparcs) also have a full integer for an
-	addend.  */
+	addend.
 
+*/
 #ifndef CTOR_TABLE_RELOC_HOWTO
 #define CTOR_TABLE_RELOC_IDX 2
 #define CTOR_TABLE_RELOC_HOWTO(BFD)					\
@@ -227,8 +230,7 @@
 
 /* Convert standard reloc records to "arelent" format (incl byte swap).  */
 
-reloc_howto_type howto_table_std[] =
-{
+reloc_howto_type howto_table_std[] = {
   /* type              rs size bsz  pcrel bitpos ovrf                     sf name     part_inpl readmask  setmask    pcdone.  */
 HOWTO ( 0,	       0,  0,  	8,  FALSE, 0, complain_overflow_bitfield,0,"8",		TRUE, 0x000000ff,0x000000ff, FALSE),
 HOWTO ( 1,	       0,  1, 	16, FALSE, 0, complain_overflow_bitfield,0,"16",	TRUE, 0x0000ffff,0x0000ffff, FALSE),
@@ -3065,10 +3067,9 @@
   struct aout_link_hash_table *ret;
   bfd_size_type amt = sizeof (struct aout_link_hash_table);
 
-  ret = (struct aout_link_hash_table *) bfd_malloc (amt);
+  ret = (struct aout_link_hash_table *) bfd_alloc (abfd, amt);
   if (ret == NULL)
     return (struct bfd_link_hash_table *) NULL;
-
   if (! NAME(aout,link_hash_table_init) (ret, abfd,
 					 NAME(aout,link_hash_newfunc)))
     {
@@ -3905,8 +3906,10 @@
   for (o = abfd->sections; o != NULL; o = o->next)
     {
       for (p = o->link_order_head; p != NULL; p = p->next)
-	if (p->type == bfd_indirect_link_order)
-	  p->u.indirect.section->linker_mark = TRUE;
+	{
+	  if (p->type == bfd_indirect_link_order)
+	    p->u.indirect.section->linker_mark = TRUE;
+	}
     }
 
   have_link_order_relocs = FALSE;
Index: dwarf2.c
===================================================================
RCS file: /cvs/src/src/bfd/dwarf2.c,v
retrieving revision 1.43
retrieving revision 1.42
diff -u -r1.43 -r1.42
--- dwarf2.c	31 Mar 2003 18:13:25 -0000	1.43
+++ dwarf2.c	12 Dec 2002 10:26:01 -0000	1.42
@@ -1,5 +1,5 @@
 /* DWARF 2 support.
-   Copyright 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003
+   Copyright 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002
    Free Software Foundation, Inc.
 
    Adapted from gdb/dwarf2read.c by Gavin Koch of Cygnus Solutions
@@ -911,9 +911,6 @@
   info->end_sequence = end_sequence;
 }
 
-/* Extract a fully qualified filename from a line info table.
-   The returned string has been xmalloc'ed.  */
-
 static char *
 concat_filename (table, file)
      struct line_info_table* table;
@@ -925,13 +922,12 @@
     {
       (*_bfd_error_handler)
 	(_("Dwarf Error: mangled line number section (bad file number)."));
-      return concat ("<unknown>");
+      return "<unknown>";
     }
 
   filename = table->files[file - 1].name;
-
-  if (IS_ABSOLUTE_PATH (filename))
-    return concat (filename);
+  if (IS_ABSOLUTE_PATH(filename))
+    return filename;
   else
     {
       char* dirname = (table->files[file - 1].dir
@@ -941,9 +937,9 @@
       /* Not all tools set DW_AT_comp_dir, so dirname may be unknown.  The
 	 best we can do is return the filename part.  */
       if (dirname == NULL)
-	return concat (filename);
+	return filename;
       else
-	return concat (dirname, "/", filename, NULL);
+	return (char*) concat (dirname, "/", filename, NULL);
     }
 }
 
@@ -1276,7 +1272,6 @@
 		   based, the references are 1 based.  */
 		file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 		line_ptr += bytes_read;
-		free (filename);
 		filename = concat_filename (table, file);
 		break;
 	      }
@@ -1301,7 +1296,6 @@
 	    default:
 	      {
 		int i;
-
 		/* Unknown standard opcode, ignore it.  */
 		for (i = 0; i < lh.standard_opcode_lengths[op_code]; i++)
 		  {
@@ -1311,8 +1305,6 @@
 	      }
 	    }
 	}
-
-      free (filename);
     }
 
   return table;
Index: elf-eh-frame.c
===================================================================
RCS file: /cvs/src/src/bfd/elf-eh-frame.c,v
retrieving revision 1.23
retrieving revision 1.22
diff -u -r1.23 -r1.22
--- elf-eh-frame.c	31 Mar 2003 18:13:25 -0000	1.23
+++ elf-eh-frame.c	6 Feb 2003 23:01:04 -0000	1.22
@@ -2,21 +2,21 @@
    Copyright 2001, 2002, 2003 Free Software Foundation, Inc.
    Written by Jakub Jelinek <jakub@redhat.com>.
 
-   This file is part of BFD, the Binary File Descriptor library.
+This file is part of BFD, the Binary File Descriptor library.
 
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
 
 #include "bfd.h"
 #include "sysdep.h"
@@ -39,7 +39,7 @@
 static int cie_compare
   PARAMS ((struct cie *, struct cie *));
 static int vma_compare
-  PARAMS ((const PTR, const PTR));
+  PARAMS ((const PTR a, const PTR b));
 
 /* Helper function for reading uleb128 encoded data.  */
 
@@ -1112,7 +1112,7 @@
    fde_count x [encoded] initial_loc, fde
 				(array of encoded pairs containing
 				 FDE initial_location field and FDE address,
-				 sorted by increasing initial_loc).  */
+				 sorted by increasing initial_loc)  */
 
 bfd_boolean
 _bfd_elf_write_section_eh_frame_hdr (abfd, info)
@@ -1125,7 +1125,6 @@
   bfd_byte *contents;
   asection *eh_frame_sec;
   bfd_size_type size;
-  bfd_boolean retval;
 
   htab = elf_hash_table (info);
   hdr_info = &htab->eh_info;
@@ -1142,18 +1141,15 @@
 
   eh_frame_sec = bfd_get_section_by_name (abfd, ".eh_frame");
   if (eh_frame_sec == NULL)
-    {
-      free (contents);
-      return FALSE;
-    }
+    return FALSE;
 
   memset (contents, 0, EH_FRAME_HDR_SIZE);
-  contents[0] = 1;				/* Version.  */
-  contents[1] = DW_EH_PE_pcrel | DW_EH_PE_sdata4; /* .eh_frame offset.  */
+  contents[0] = 1;				/* Version  */
+  contents[1] = DW_EH_PE_pcrel | DW_EH_PE_sdata4; /* .eh_frame offset  */
   if (hdr_info->array && hdr_info->array_count == hdr_info->fde_count)
     {
-      contents[2] = DW_EH_PE_udata4;		/* FDE count encoding.  */
-      contents[3] = DW_EH_PE_datarel | DW_EH_PE_sdata4; /* Search table enc.  */
+      contents[2] = DW_EH_PE_udata4;		/* FDE count encoding  */
+      contents[3] = DW_EH_PE_datarel | DW_EH_PE_sdata4; /* search table enc  */
     }
   else
     {
@@ -1181,9 +1177,7 @@
 	}
     }
 
-  retval = bfd_set_section_contents (abfd, sec->output_section,
-				     contents, (file_ptr) sec->output_offset,
-				     sec->_cooked_size);
-  free (contents);
-  return retval;
+  return bfd_set_section_contents (abfd, sec->output_section,
+				   contents, (file_ptr) sec->output_offset,
+                                   sec->_cooked_size);
 }
Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.181
retrieving revision 1.180
diff -u -r1.181 -r1.180
--- elf.c	31 Mar 2003 18:13:25 -0000	1.181
+++ elf.c	24 Feb 2003 18:07:22 -0000	1.180
@@ -5068,10 +5068,7 @@
 	      amt += ((bfd_size_type) section_count - 1) * sizeof (asection *);
 	      map = (struct elf_segment_map *) bfd_alloc (obfd, amt);
 	      if (map == NULL)
-		{
-		  free (sections);
-		  return FALSE;
-		}
+		return FALSE;
 
 	      /* Initialise the fields of the segment map.  Set the physical
 		 physical address to the LMA of the first section that has
@@ -5306,10 +5303,7 @@
   amt = (bfd_size_type) (1 + symcount) * bed->s->sizeof_sym;
   outbound_syms = bfd_alloc (abfd, amt);
   if (outbound_syms == NULL)
-    {
-      _bfd_stringtab_free (stt);
-      return FALSE;
-    }
+    return FALSE;
   symtab_hdr->contents = (PTR) outbound_syms;
 
   outbound_shndx = NULL;
@@ -5319,11 +5313,7 @@
       amt = (bfd_size_type) (1 + symcount) * sizeof (Elf_External_Sym_Shndx);
       outbound_shndx = bfd_zalloc (abfd, amt);
       if (outbound_shndx == NULL)
-	{
-	  _bfd_stringtab_free (stt);
-	  return FALSE;
-	}
-
+	return FALSE;
       symtab_shndx_hdr->contents = outbound_shndx;
       symtab_shndx_hdr->sh_type = SHT_SYMTAB_SHNDX;
       symtab_shndx_hdr->sh_size = amt;
@@ -5367,10 +5357,7 @@
 							    syms[idx]->name,
 							    TRUE, FALSE);
 	  if (sym.st_name == (unsigned long) -1)
-	    {
-	      _bfd_stringtab_free (stt);
-	      return FALSE;
-	    }
+	    return FALSE;
 	}
 
       type_ptr = elf_symbol_from (abfd, syms[idx]);
@@ -5459,7 +5446,6 @@
 					  syms[idx]->name ? syms[idx]->name : "<Local sym>",
 					  sec->name);
 		      bfd_set_error (bfd_error_invalid_operation);      
-		      _bfd_stringtab_free (stt);
 		      return FALSE;
 		    }
   
@@ -5920,7 +5906,7 @@
   return TRUE;
 
  error_return:
-  if (contents != NULL)
+  if (contents == NULL)
     free (contents);
   return FALSE;
 }
Index: elflink.h
===================================================================
RCS file: /cvs/src/src/bfd/elflink.h,v
retrieving revision 1.211
retrieving revision 1.210
diff -u -r1.211 -r1.210
--- elflink.h	31 Mar 2003 18:13:25 -0000	1.211
+++ elflink.h	29 Mar 2003 01:26:33 -0000	1.210
@@ -4855,7 +4855,6 @@
 	  }
       }
 
-  free (sort);
   *psec = reldyn;
   return ret;
 }
Index: format.c
===================================================================
RCS file: /cvs/src/src/bfd/format.c,v
retrieving revision 1.13
retrieving revision 1.12
diff -u -r1.13 -r1.12
--- format.c	31 Mar 2003 18:13:25 -0000	1.13
+++ format.c	14 Feb 2003 11:16:09 -0000	1.12
@@ -163,11 +163,7 @@
   if (!abfd->target_defaulted)
     {
       if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)	/* rewind! */
-	{
-	  if (matching)
-	    free ((PTR) matching_vector);
-	  return FALSE;
-	}
+	return FALSE;
 
       right_targ = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
 
@@ -218,11 +214,7 @@
       abfd->xvec = *target;	/* Change BFD's target temporarily.  */
 
       if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)
-	{
-	  if (matching)
-	    free ((PTR) matching_vector);
-	  return FALSE;
-	}
+	return FALSE;
 
       /* If _bfd_check_format neglects to set bfd_error, assume
 	 bfd_error_wrong_format.  We didn't used to even pay any
Index: linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.30
retrieving revision 1.29
diff -u -r1.30 -r1.29
--- linker.c	31 Mar 2003 18:13:25 -0000	1.30
+++ linker.c	20 Dec 2002 22:41:13 -0000	1.29
@@ -1,23 +1,23 @@
 /* linker.c -- BFD linker routines
-   Copyright 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003
+   Copyright 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002
    Free Software Foundation, Inc.
    Written by Steve Chamberlain and Ian Lance Taylor, Cygnus Support
 
-   This file is part of BFD, the Binary File Descriptor library.
+This file is part of BFD, the Binary File Descriptor library.
 
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
 
 #include "bfd.h"
 #include "sysdep.h"
@@ -2081,12 +2081,12 @@
 							input_section,
 							relocs,
 							symbols);
-		  free (relocs);
 		  if (reloc_count < 0)
 		    return FALSE;
 		  BFD_ASSERT ((unsigned long) reloc_count
 			      == input_section->reloc_count);
 		  o->reloc_count += reloc_count;
+		  free (relocs);
 		}
 	    }
 	  if (o->reloc_count > 0)
Index: opncls.c
===================================================================
RCS file: /cvs/src/src/bfd/opncls.c,v
retrieving revision 1.15
retrieving revision 1.14
diff -u -r1.15 -r1.14
--- opncls.c	31 Mar 2003 18:13:25 -0000	1.15
+++ opncls.c	31 Jan 2003 10:04:16 -0000	1.14
@@ -931,13 +931,8 @@
 
   basename = get_debug_link_info (abfd, & crc32);
 
-  if (basename == NULL)
+  if (basename == NULL || strlen (basename) < 1)
     return NULL;
-  if (strlen (basename) < 1)
-    {
-      free (basename);
-      return NULL;
-    }
 
   dir = xstrdup (abfd->filename);
   BFD_ASSERT (strlen (dir) != 0);
Index: simple.c
===================================================================
RCS file: /cvs/src/src/bfd/simple.c,v
retrieving revision 1.6
retrieving revision 1.5
diff -u -r1.6 -r1.5
--- simple.c	31 Mar 2003 18:13:25 -0000	1.6
+++ simple.c	30 Nov 2002 08:39:40 -0000	1.5
@@ -1,5 +1,5 @@
 /* simple.c -- BFD simple client routines
-   Copyright 2002, 2003
+   Copyright 2002
    Free Software Foundation, Inc.
    Contributed by MontaVista Software, Inc.
 
@@ -135,7 +135,7 @@
   struct bfd_link_order link_order;
   struct bfd_link_callbacks callbacks;
   bfd_byte *contents, *data;
-  int storage_needed;
+  int storage_needed, number_of_symbols;
   asymbol **symbol_table;
 
   if (! (sec->flags & SEC_RELOC))
@@ -187,7 +187,7 @@
 
   storage_needed = bfd_get_symtab_upper_bound (abfd);
   symbol_table = (asymbol **) bfd_malloc (storage_needed);
-  bfd_canonicalize_symtab (abfd, symbol_table);
+  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
 
   contents = bfd_get_relocated_section_contents (abfd,
 						 &link_info,
@@ -208,6 +208,5 @@
 
   bfd_link_hash_table_free (abfd, link_info.hash);
 
-  free (symbol_table);
   return contents;
 }
Index: version.h
===================================================================
RCS file: /cvs/src/src/bfd/version.h,v
retrieving revision 1.521
retrieving revision 1.520
diff -u -r1.521 -r1.520
--- version.h	31 Mar 2003 00:00:06 -0000	1.521
+++ version.h	30 Mar 2003 00:00:05 -0000	1.520
@@ -1,3 +1,3 @@
-#define BFD_VERSION_DATE 20030331
+#define BFD_VERSION_DATE 20030330
 #define BFD_VERSION @bfd_version@
 #define BFD_VERSION_STRING @bfd_version_string@

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

And this:

> -	return concat (filename);
> +	return filename;

(remembering that the patch is backwards) doesn't look so good.  It 
should be:

	concat (filename, NULL);

perhaps

	xstrdup (filename)

would be easier to read :-)

Doesn't get gdb off the hook though.

Andrew

Re: gdb.mi/mi-cli.exp failures

[Hans-Peter Nilsson]

On Mon, 31 Mar 2003, Andrew Cagney wrote:
> And this:
>
> > -	return concat (filename);
> > +	return filename;
>
> (remembering that the patch is backwards) doesn't look so good.  It
> should be:
>
> 	concat (filename, NULL);
>
> perhaps
>
> 	xstrdup (filename)
>
> would be easier to read :-)

\begin{chant} There must be no xstrdup calls in bfd.  BFD must
not abort; it's a library and the caller must handle allocation
failures.  There may be xstrdup calls in bfd already, but please
don't add new ones. \end{chant}

brgds, H-P

Re: gdb.mi/mi-cli.exp failures

[Ian Lance Taylor]

Hans-Peter Nilsson <hp@bitrange.com> writes:

> On Mon, 31 Mar 2003, Andrew Cagney wrote:
> > And this:
> >
> > > -	return concat (filename);
> > > +	return filename;
> >
> > (remembering that the patch is backwards) doesn't look so good.  It
> > should be:
> >
> > 	concat (filename, NULL);
> >
> > perhaps
> >
> > 	xstrdup (filename)
> >
> > would be easier to read :-)
> 
> \begin{chant} There must be no xstrdup calls in bfd.  BFD must
> not abort; it's a library and the caller must handle allocation
> failures.  There may be xstrdup calls in bfd already, but please
> don't add new ones. \end{chant}

Could use strdup, though, not that I know the context here.

Ian

Re: gdb.mi/mi-cli.exp failures

[Nick Clifton]

Hi Andrew, Hi David

> > FYI, my test run from my just committed call_dummy_p change had
> > the usual 21 failures.  Yet, mysteriously, for my next set of
> > patches, I'm seeing the failures jump to 28. While I'll note that
> > I'm the prime suspect, did anything else get changed?

Which 21 vs 28 failures are these ?  (I could reproduce the mi-cli.exp
failure, but I do not know if this is the problem Andrew was
referring to).


> Applying the attached to BFD fixes the problem ....

Although applying it would be a bad idea because it reverts several
bug fixes that plug memory leaks.

Instead here is a version that removes the use of "concat()" inside
dwarf2.c:concat_filename() which is a good thing, but which probably
does not solve the problem that you encountered in GDB.

Can you narrow down which part of David Heine's patch (altered by me)
is causing you problems, or tell me which tests in GDB are now failing
so that I can try to track it down myself.

Cheers
        Nick

2003-04-01  Nick Clifton  <nickc@redhat.com>

	* dwarf2.c (concat_filename): Use bfd_malloc() and strdup()
	instead of concat().
	(decode_line_info): Only free filename if it is not NULL.

Index: bfd/dwarf2.c
===================================================================
RCS file: /cvs/src/src/bfd/dwarf2.c,v
retrieving revision 1.44
diff -c -3 -p -w -r1.44 dwarf2.c
*** bfd/dwarf2.c	1 Apr 2003 00:12:12 -0000	1.44
--- bfd/dwarf2.c	1 Apr 2003 10:13:15 -0000
*************** add_line_info (table, address, filename,
*** 908,914 ****
  }
  
  /* Extract a fully qualified filename from a line info table.
!    The returned string has been xmalloc'ed.  */
  
  static char *
  concat_filename (table, file)
--- 908,915 ----
  }
  
  /* Extract a fully qualified filename from a line info table.
!    The returned string has been malloc'ed and it is the caller's
!    responsibility to free it.  */
  
  static char *
  concat_filename (table, file)
*************** concat_filename (table, file)
*** 921,946 ****
      {
        (*_bfd_error_handler)
  	(_("Dwarf Error: mangled line number section (bad file number)."));
!       return concat ("<unknown>");
      }
  
    filename = table->files[file - 1].name;
  
!   if (IS_ABSOLUTE_PATH (filename))
!     return concat (filename);
!   else
      {
        char* dirname = (table->files[file - 1].dir
  		       ? table->dirs[table->files[file - 1].dir - 1]
  		       : table->comp_dir);
  
!       /* Not all tools set DW_AT_comp_dir, so dirname may be unknown.  The
! 	 best we can do is return the filename part.  */
!       if (dirname == NULL)
! 	return concat (filename);
!       else
! 	return concat (dirname, "/", filename, NULL);
      }
  }
  
  static void
--- 922,953 ----
      {
        (*_bfd_error_handler)
  	(_("Dwarf Error: mangled line number section (bad file number)."));
!       return strdup ("<unknown>");
      }
  
    filename = table->files[file - 1].name;
  
!   if (! IS_ABSOLUTE_PATH (filename))
      {
        char* dirname = (table->files[file - 1].dir
  		       ? table->dirs[table->files[file - 1].dir - 1]
  		       : table->comp_dir);
  
!       /* Not all tools set DW_AT_comp_dir, so dirname may be unknown.
! 	 The best we can do is return the filename part.  */
!       if (dirname != NULL)
! 	{
! 	  unsigned int len = strlen (dirname) + strlen (filename) + 2;
! 	  char * name;
! 
! 	  name = bfd_malloc (len);
! 	  if (name)
! 	    sprintf (name, "%s/%s", dirname, filename);
! 	  return name;
! 	}
      }
+ 
+   return strdup (filename);
  }
  
  static void
*************** decode_line_info (unit, stash)
*** 1266,1271 ****
--- 1273,1279 ----
  		   based, the references are 1 based.  */
  		file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
  		line_ptr += bytes_read;
+ 		if (filename)
  		  free (filename);
  		filename = concat_filename (table, file);
  		break;
*************** decode_line_info (unit, stash)
*** 1302,1307 ****
--- 1310,1316 ----
  	    }
  	}
  
+       if (filename)
  	free (filename);
      }
  

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

>> Applying the attached to BFD fixes the problem ....
> 
> 
> Although applying it would be a bad idea because it reverts several
> bug fixes that plug memory leaks.

Leaky memory is a lesser evil to corrupt memory.

> Instead here is a version that removes the use of "concat()" inside
> dwarf2.c:concat_filename() which is a good thing, but which probably
> does not solve the problem that you encountered in GDB.
> 
> Can you narrow down which part of David Heine's patch (altered by me)
> is causing you problems, or tell me which tests in GDB are now failing
> so that I can try to track it down myself.

Unfortunatly BFD changed an interface right in the middle of this - it's 
put GDB/BFD into a death spiral :-(  I'm currently reverting a directory 
tree to see what can be seen ...

FAIL: gdb.base/relocate.exp: get address of static_bar (timeout)
FAIL: gdb.base/relocate.exp: static variables have different addresses
FAIL: gdb.base/relocate.exp: get address of global_foo (timeout)
FAIL: gdb.base/relocate.exp: get address of global_bar (timeout)
FAIL: gdb.base/relocate.exp: global variables have different addresses
FAIL: gdb.base/relocate.exp: get address of function_foo (timeout)
FAIL: gdb.base/relocate.exp: get address of function_bar (timeout)
FAIL: gdb.base/relocate.exp: functions have different addresses

Andrew

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Tue, Apr 01, 2003 at 10:08:41AM -0500, Andrew Cagney wrote:
> 
> >>Applying the attached to BFD fixes the problem ....
> >
> >
> >Although applying it would be a bad idea because it reverts several
> >bug fixes that plug memory leaks.
> 
> Leaky memory is a lesser evil to corrupt memory.
> 
> >Instead here is a version that removes the use of "concat()" inside
> >dwarf2.c:concat_filename() which is a good thing, but which probably
> >does not solve the problem that you encountered in GDB.
> >
> >Can you narrow down which part of David Heine's patch (altered by me)
> >is causing you problems, or tell me which tests in GDB are now failing
> >so that I can try to track it down myself.
> 
> Unfortunatly BFD changed an interface right in the middle of this - it's 
> put GDB/BFD into a death spiral :-(  I'm currently reverting a directory 
> tree to see what can be seen ...
> 
> FAIL: gdb.base/relocate.exp: get address of static_bar (timeout)
> FAIL: gdb.base/relocate.exp: static variables have different addresses
> FAIL: gdb.base/relocate.exp: get address of global_foo (timeout)
> FAIL: gdb.base/relocate.exp: get address of global_bar (timeout)
> FAIL: gdb.base/relocate.exp: global variables have different addresses
> FAIL: gdb.base/relocate.exp: get address of function_foo (timeout)
> FAIL: gdb.base/relocate.exp: get address of function_bar (timeout)
> FAIL: gdb.base/relocate.exp: functions have different addresses

On what target - I don't see these...

This looks like a crash in the same function that changed interface...
perhaps the memory leak fix for simple.c was wrong, although I can't
quite see why.  By the way, adding or removing the NULL at the end is
all that GDB needs to do to work with both interfaces.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

> On what target - I don't see these...

RH 7.2 i386, PPC and d10v also show a jump in failures but I've not 
checked that they are 100% identical.

> This looks like a crash in the same function that changed interface...
> perhaps the memory leak fix for simple.c was wrong, although I can't

Applying the simple.c change causes the problem.

Index: simple.c
===================================================================
RCS file: /cvs/src/src/bfd/simple.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- simple.c    30 Nov 2002 08:39:40 -0000      1.5
+++ simple.c    31 Mar 2003 18:13:25 -0000      1.6
@@ -1,5 +1,5 @@
  /* simple.c -- BFD simple client routines
-   Copyright 2002
+   Copyright 2002, 2003
     Free Software Foundation, Inc.
     Contributed by MontaVista Software, Inc.

@@ -135,7 +135,7 @@
    struct bfd_link_order link_order;
    struct bfd_link_callbacks callbacks;
    bfd_byte *contents, *data;
-  int storage_needed, number_of_symbols;
+  int storage_needed;
    asymbol **symbol_table;

    if (! (sec->flags & SEC_RELOC))
@@ -187,7 +187,7 @@

    storage_needed = bfd_get_symtab_upper_bound (abfd);
    symbol_table = (asymbol **) bfd_malloc (storage_needed);
-  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
+  bfd_canonicalize_symtab (abfd, symbol_table);

    contents = bfd_get_relocated_section_contents (abfd,
                                                  &link_info,
@@ -208,5 +208,6 @@

    bfd_link_hash_table_free (abfd, link_info.hash);

+  free (symbol_table);
    return contents;
  }

A guess is that something still has a reference to the symbab.

> quite see why.  By the way, adding or removing the NULL at the end is
> all that GDB needs to do to work with both interfaces.

Doesn't work.  To test my patches, I've had to adjust upwards the 
expected failure list.

Andrew

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Tue, Apr 01, 2003 at 11:22:09AM -0500, Andrew Cagney wrote:
> 
> >On what target - I don't see these...
> 
> RH 7.2 i386, PPC and d10v also show a jump in failures but I've not 
> checked that they are 100% identical.

Bizarre, I can not reproduce this.

> >This looks like a crash in the same function that changed interface...
> >perhaps the memory leak fix for simple.c was wrong, although I can't
> 
> Applying the simple.c change causes the problem.
> 
> Index: simple.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/simple.c,v
> retrieving revision 1.5
> retrieving revision 1.6
> diff -u -r1.5 -r1.6
> --- simple.c    30 Nov 2002 08:39:40 -0000      1.5
> +++ simple.c    31 Mar 2003 18:13:25 -0000      1.6
> @@ -1,5 +1,5 @@
>  /* simple.c -- BFD simple client routines
> -   Copyright 2002
> +   Copyright 2002, 2003
>     Free Software Foundation, Inc.
>     Contributed by MontaVista Software, Inc.
> 
> @@ -135,7 +135,7 @@
>    struct bfd_link_order link_order;
>    struct bfd_link_callbacks callbacks;
>    bfd_byte *contents, *data;
> -  int storage_needed, number_of_symbols;
> +  int storage_needed;
>    asymbol **symbol_table;
> 
>    if (! (sec->flags & SEC_RELOC))
> @@ -187,7 +187,7 @@
> 
>    storage_needed = bfd_get_symtab_upper_bound (abfd);
>    symbol_table = (asymbol **) bfd_malloc (storage_needed);
> -  number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
> +  bfd_canonicalize_symtab (abfd, symbol_table);
> 
>    contents = bfd_get_relocated_section_contents (abfd,
>                                                  &link_info,
> @@ -208,5 +208,6 @@
> 
>    bfd_link_hash_table_free (abfd, link_info.hash);
> 
> +  free (symbol_table);
>    return contents;
>  }
> 
> A guess is that something still has a reference to the symbab.

OK, that's pretty strange.  It must be cached in the BFD somewhere.  Do
you have valgrind available?  It will identify the access to a freed
resource.

I can't find the problem here, and reading the code I can not find the
reference.

> >quite see why.  By the way, adding or removing the NULL at the end is
> >all that GDB needs to do to work with both interfaces.
> 
> Doesn't work.  To test my patches, I've had to adjust upwards the 
> expected failure list.

Don't follow... if you add/remove the NULL so that GDB compiles, it
should work fine.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

> Don't follow... if you add/remove the NULL so that GDB compiles, it
> should work fine.

So your expecting developers to hand edit, re-build, re-test against two 
trees before committing?  That is the only way they can be certain that 
their GDB change hasn't caused a regression :-/

Andrew

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Tue, Apr 01, 2003 at 12:01:25PM -0500, Andrew Cagney wrote:
> 
> >Don't follow... if you add/remove the NULL so that GDB compiles, it
> >should work fine.
> 
> So your expecting developers to hand edit, re-build, re-test against two 
> trees before committing?  That is the only way they can be certain that 
> their GDB change hasn't caused a regression :-/

No, I'm "expecting" for the BFD bug which is causing you to play with
the older BFD version to be fixed.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Andrew Cagney]

> Hi Andrew,
> 
> 
>> Unfortunatly BFD changed an interface right in the middle of this -
>> it's put GDB/BFD into a death spiral :-(
> 
> 
> Die evil GDB die ... :-)
> 
> 
>> FAIL: gdb.base/relocate.exp: get address of static_bar (timeout)
>> FAIL: gdb.base/relocate.exp: static variables have different addresses
>> FAIL: gdb.base/relocate.exp: get address of global_foo (timeout)
>> FAIL: gdb.base/relocate.exp: get address of global_bar (timeout)
>> FAIL: gdb.base/relocate.exp: global variables have different addresses
>> FAIL: gdb.base/relocate.exp: get address of function_foo (timeout)
>> FAIL: gdb.base/relocate.exp: get address of function_bar (timeout)
>> FAIL: gdb.base/relocate.exp: functions have different addresses
> 
> 
> Hmm, I currently do not see these failures.  In fact I do not see any
> failures running the relocate.exp script with the latest GDB and
> BINUTILS sources.  Have you fixed something already ?

I see these failures on both i386 RH 7.2 native and PPC X d10v-elf using 
a gdb+dejagnu source tree.  It occures in both current and with a 
2003-03-31-gmt tree containing just that simple.c change.

I should note that these are both elf + stab targets.

Andrew

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Tue, Apr 01, 2003 at 12:09:47PM -0500, Andrew Cagney wrote:
> >Hi Andrew,
> >
> >
> >>Unfortunatly BFD changed an interface right in the middle of this -
> >>it's put GDB/BFD into a death spiral :-(
> >
> >
> >Die evil GDB die ... :-)
> >
> >
> >>FAIL: gdb.base/relocate.exp: get address of static_bar (timeout)
> >>FAIL: gdb.base/relocate.exp: static variables have different addresses
> >>FAIL: gdb.base/relocate.exp: get address of global_foo (timeout)
> >>FAIL: gdb.base/relocate.exp: get address of global_bar (timeout)
> >>FAIL: gdb.base/relocate.exp: global variables have different addresses
> >>FAIL: gdb.base/relocate.exp: get address of function_foo (timeout)
> >>FAIL: gdb.base/relocate.exp: get address of function_bar (timeout)
> >>FAIL: gdb.base/relocate.exp: functions have different addresses
> >
> >
> >Hmm, I currently do not see these failures.  In fact I do not see any
> >failures running the relocate.exp script with the latest GDB and
> >BINUTILS sources.  Have you fixed something already ?
> 
> I see these failures on both i386 RH 7.2 native and PPC X d10v-elf using 
> a gdb+dejagnu source tree.  It occures in both current and with a 
> 2003-03-31-gmt tree containing just that simple.c change.
> 
> I should note that these are both elf + stab targets.

Aha!  That was the missing piece.  I see two problems and I'd like
feedback from binutils maintainers on them.

1.  stabs.c can create a new section while reading stabs from an input
BFD.  This means that I can't save and restore a per-section datum
(output_offset and output_section) around a call to
bfd_link_add_symbols.

#0  bfd_section_init (abfd=0x8277980, newsect=0x827b9b4) at /opt/src/gdb/src/bfd/section.c:734
#1  0x0818039d in _bfd_link_section_stabs (abfd=0x8277980, psinfo=0x827855c, stabsec=0x827cae8,
    stabstrsec=0x827cb7c, psecinfo=0x827b9c0) at /opt/src/gdb/src/bfd/stabs.c:240
#2  0x081501d6 in elf_link_add_object_symbols (abfd=0x8277980, info=0xbfffe0b0)
    at /opt/src/gdb/src/bfd/elflink.h:2305
#3  0x08148f8b in bfd_simple_get_relocated_section_contents (abfd=0x8277980, sec=0x827cae8,
    outbuf=0x828f868 "", symbol_table=0x0) at /opt/src/gdb/src/bfd/simple.c:247

It's doing this:
	sinfo->stabstr = bfd_make_section_anyway (abfd, ".stabstr");
which doesn't make much sense to me; there's _already_ a section named
.stabstr in the executable, why not use that one?

Can I not rely on section_count remaining stable for an input BFD?


2.  We call bfd_simple_get_relocated_section_contents twice on the same
section.  The second time, it crashes.  It appears that pointers to the
symbol table are stored in the canonicalized relocations, in
elf_slurp_reloc_table_from_section.  This means that the symbol table
can no longer be freed.  Should it be allocated via bfd_alloc instead
of bfd_malloc?

Note that this is a significant change, since we're talking about the
pointer table, i.e. the area of size bfd_get_symtab_upper_bound ().

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Nick Clifton]

Hi Daniel,

> 1.  stabs.c can create a new section while reading stabs from an input
> BFD.  This means that I can't save and restore a per-section datum
> (output_offset and output_section) around a call to
> bfd_link_add_symbols.
> 
> #0  bfd_section_init (abfd=0x8277980, newsect=0x827b9b4) at /opt/src/gdb/src/bfd/section.c:734
> #1  0x0818039d in _bfd_link_section_stabs (abfd=0x8277980, psinfo=0x827855c, stabsec=0x827cae8,
>     stabstrsec=0x827cb7c, psecinfo=0x827b9c0) at /opt/src/gdb/src/bfd/stabs.c:240
> #2  0x081501d6 in elf_link_add_object_symbols (abfd=0x8277980, info=0xbfffe0b0)
>     at /opt/src/gdb/src/bfd/elflink.h:2305
> #3  0x08148f8b in bfd_simple_get_relocated_section_contents (abfd=0x8277980, sec=0x827cae8,
>     outbuf=0x828f868 "", symbol_table=0x0) at /opt/src/gdb/src/bfd/simple.c:247
> 
> It's doing this:
> 	sinfo->stabstr = bfd_make_section_anyway (abfd, ".stabstr");
> which doesn't make much sense to me; there's _already_ a section named
> .stabstr in the executable, why not use that one?

Hmm, agreed - this probably ought to be a call to
bfd_get_section_by_name().  And if that fails then the code should try
to find the output bfd and create the section there.  (Either that or
else return a failure result.  Can you have a .stabs section without a
.stabsstr section ?)

> Can I not rely on section_count remaining stable for an input BFD?

I think that you ought to be able to rely on this.


> 2.  We call bfd_simple_get_relocated_section_contents twice on the same
> section.  The second time, it crashes.  It appears that pointers to the
> symbol table are stored in the canonicalized relocations, in
> elf_slurp_reloc_table_from_section.  This means that the symbol table
> can no longer be freed.  Should it be allocated via bfd_alloc instead
> of bfd_malloc?

> Note that this is a significant change, since we're talking about the
> pointer table, i.e. the area of size bfd_get_symtab_upper_bound ().

Plus we would now have to ensure that the caller has allocated the
memory in the same bfd, it is passing in the 'symbol_table' pointer.

However, since the relocs are persistent, I think that the symbols
that they use need to be persistent too.  Hence we need to use
bfd_alloc.

Cheers
        Nick

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Wed, Apr 02, 2003 at 06:06:05PM +0100, Nick Clifton wrote:
> Hi Daniel,
> 
> > 1.  stabs.c can create a new section while reading stabs from an input
> > BFD.  This means that I can't save and restore a per-section datum
> > (output_offset and output_section) around a call to
> > bfd_link_add_symbols.
> > 
> > #0  bfd_section_init (abfd=0x8277980, newsect=0x827b9b4) at /opt/src/gdb/src/bfd/section.c:734
> > #1  0x0818039d in _bfd_link_section_stabs (abfd=0x8277980, psinfo=0x827855c, stabsec=0x827cae8,
> >     stabstrsec=0x827cb7c, psecinfo=0x827b9c0) at /opt/src/gdb/src/bfd/stabs.c:240
> > #2  0x081501d6 in elf_link_add_object_symbols (abfd=0x8277980, info=0xbfffe0b0)
> >     at /opt/src/gdb/src/bfd/elflink.h:2305
> > #3  0x08148f8b in bfd_simple_get_relocated_section_contents (abfd=0x8277980, sec=0x827cae8,
> >     outbuf=0x828f868 "", symbol_table=0x0) at /opt/src/gdb/src/bfd/simple.c:247
> > 
> > It's doing this:
> > 	sinfo->stabstr = bfd_make_section_anyway (abfd, ".stabstr");
> > which doesn't make much sense to me; there's _already_ a section named
> > .stabstr in the executable, why not use that one?
> 
> Hmm, agreed - this probably ought to be a call to
> bfd_get_section_by_name().  And if that fails then the code should try
> to find the output bfd and create the section there.  (Either that or
> else return a failure result.  Can you have a .stabs section without a
> .stabsstr section ?)

Not one that makes much sense.  The stabs data contains string table
indices... Even N_SO will have a string reference.

> > Can I not rely on section_count remaining stable for an input BFD?
> 
> I think that you ought to be able to rely on this.

OK, then fixing stabs.c will take care of one of the problems.

> > 2.  We call bfd_simple_get_relocated_section_contents twice on the same
> > section.  The second time, it crashes.  It appears that pointers to the
> > symbol table are stored in the canonicalized relocations, in
> > elf_slurp_reloc_table_from_section.  This means that the symbol table
> > can no longer be freed.  Should it be allocated via bfd_alloc instead
> > of bfd_malloc?
> 
> > Note that this is a significant change, since we're talking about the
> > pointer table, i.e. the area of size bfd_get_symtab_upper_bound ().
> 
> Plus we would now have to ensure that the caller has allocated the
> memory in the same bfd, it is passing in the 'symbol_table' pointer.
> 
> However, since the relocs are persistent, I think that the symbols
> that they use need to be persistent too.  Hence we need to use
> bfd_alloc.

However, bfd_get_symtab_upper_bound is an exported interface. 
bfd_alloc is not.  We'd have to say that a caller could not free the
result of bfd_canonicalize_symtab until after closing the BFD.  Gross!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Ian Lance Taylor]

Nick Clifton <nickc@redhat.com> writes:

> > It's doing this:
> > 	sinfo->stabstr = bfd_make_section_anyway (abfd, ".stabstr");
> > which doesn't make much sense to me; there's _already_ a section named
> > .stabstr in the executable, why not use that one?
> 
> Hmm, agreed - this probably ought to be a call to
> bfd_get_section_by_name().  And if that fails then the code should try
> to find the output bfd and create the section there.  (Either that or
> else return a failure result.  Can you have a .stabs section without a
> .stabsstr section ?)

I created a .stabstr section in the input file because I needed to
have a section in an input file which the linker script could put into
the output file correctly.  This is not the only place where this
trick is used.

Using an existing .stabstr section would have to be handled carefully.
The code would have to extract the information, and arrange to replace
it in the output file.  This might not be too hard.

Creating the .stabstr section in the output BFD doesn't work, because
it won't let the linker script function properly.

> > Can I not rely on section_count remaining stable for an input BFD?
> 
> I think that you ought to be able to rely on this.

Well, I'm afraid that you will have to deal with a number of other
cases if you want to avoid adding sections to input files.  Take a
look at elf_link_create_dynamic_sections().

Ian

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Wed, Apr 02, 2003 at 09:21:13AM -0800, Ian Lance Taylor wrote:
> Nick Clifton <nickc@redhat.com> writes:
> 
> > > It's doing this:
> > > 	sinfo->stabstr = bfd_make_section_anyway (abfd, ".stabstr");
> > > which doesn't make much sense to me; there's _already_ a section named
> > > .stabstr in the executable, why not use that one?
> > 
> > Hmm, agreed - this probably ought to be a call to
> > bfd_get_section_by_name().  And if that fails then the code should try
> > to find the output bfd and create the section there.  (Either that or
> > else return a failure result.  Can you have a .stabs section without a
> > .stabsstr section ?)
> 
> I created a .stabstr section in the input file because I needed to
> have a section in an input file which the linker script could put into
> the output file correctly.  This is not the only place where this
> trick is used.
> 
> Using an existing .stabstr section would have to be handled carefully.
> The code would have to extract the information, and arrange to replace
> it in the output file.  This might not be too hard.
> 
> Creating the .stabstr section in the output BFD doesn't work, because
> it won't let the linker script function properly.

Well, do you have another suggestion for how to approach this?  We're
not actually linking; but I need to get the symbols from the input file
into a symbol table with forged offsets in order to apply relocations
against them.

> > > Can I not rely on section_count remaining stable for an input BFD?
> > 
> > I think that you ought to be able to rely on this.
> 
> Well, I'm afraid that you will have to deal with a number of other
> cases if you want to avoid adding sections to input files.  Take a
> look at elf_link_create_dynamic_sections().

In any case I can remove the assumption; it's not hard.  I assume that
if I save pointers to the sections present before calling
elf_link_add_object_symbols, that they'll still be valid when it
returns?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Ian Lance Taylor]

Daniel Jacobowitz <drow@mvista.com> writes:

> Well, do you have another suggestion for how to approach this?  We're
> not actually linking; but I need to get the symbols from the input file
> into a symbol table with forged offsets in order to apply relocations
> against them.

Well, I don't really know the context.  If you're not linking, then it
seems to me that you'll be better off avoiding the linking calls.  The
add_symbols() call is the first phase of a link, and is expected to be
followed by the second phase of a link; despite the name
add_symbols(), it doesn't just add symbols to a hash table.

If you really just want to put the symbols into a hash table, can you
just get the symbol table generically and add them to a hash table
yourself?

> > Well, I'm afraid that you will have to deal with a number of other
> > cases if you want to avoid adding sections to input files.  Take a
> > look at elf_link_create_dynamic_sections().
> 
> In any case I can remove the assumption; it's not hard.  I assume that
> if I save pointers to the sections present before calling
> elf_link_add_object_symbols, that they'll still be valid when it
> returns?

Probably.  But I personally don't think the add_symbols() routine
needs to make any guarantees at all, except that if you hook up and
the sections and call final_link() you will get a linked output file.
Maybe we should change the name add_symbols() to initial_link().

Ian

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Wed, Apr 02, 2003 at 09:44:07AM -0800, Ian Lance Taylor wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> 
> > Well, do you have another suggestion for how to approach this?  We're
> > not actually linking; but I need to get the symbols from the input file
> > into a symbol table with forged offsets in order to apply relocations
> > against them.
> 
> Well, I don't really know the context.  If you're not linking, then it
> seems to me that you'll be better off avoiding the linking calls.  The
> add_symbols() call is the first phase of a link, and is expected to be
> followed by the second phase of a link; despite the name
> add_symbols(), it doesn't just add symbols to a hash table.
> 
> If you really just want to put the symbols into a hash table, can you
> just get the symbol table generically and add them to a hash table
> yourself?

IIRC, then we may get a different kind of hash table than the
platform's relocation application functions expect.  It's been a little
while though.

The context is in bfd/simple.c if you want to look at it.  The
intention is to use this code for both gdb and objdump (they do use it
now, to be more accurate) to relocate the contents of debug sections.
This is necessary for the general cases of debugging shared objects and
unlinked object modules.

> > > Well, I'm afraid that you will have to deal with a number of other
> > > cases if you want to avoid adding sections to input files.  Take a
> > > look at elf_link_create_dynamic_sections().
> > 
> > In any case I can remove the assumption; it's not hard.  I assume that
> > if I save pointers to the sections present before calling
> > elf_link_add_object_symbols, that they'll still be valid when it
> > returns?
> 
> Probably.  But I personally don't think the add_symbols() routine
> needs to make any guarantees at all, except that if you hook up and
> the sections and call final_link() you will get a linked output file.
> Maybe we should change the name add_symbols() to initial_link().
> 
> Ian
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Ian Lance Taylor]

Daniel Jacobowitz <drow@mvista.com> writes:

> > > Well, do you have another suggestion for how to approach this?  We're
> > > not actually linking; but I need to get the symbols from the input file
> > > into a symbol table with forged offsets in order to apply relocations
> > > against them.
> > 
> > Well, I don't really know the context.  If you're not linking, then it
> > seems to me that you'll be better off avoiding the linking calls.  The
> > add_symbols() call is the first phase of a link, and is expected to be
> > followed by the second phase of a link; despite the name
> > add_symbols(), it doesn't just add symbols to a hash table.
> > 
> > If you really just want to put the symbols into a hash table, can you
> > just get the symbol table generically and add them to a hash table
> > yourself?
> 
> IIRC, then we may get a different kind of hash table than the
> platform's relocation application functions expect.  It's been a little
> while though.
> 
> The context is in bfd/simple.c if you want to look at it.  The
> intention is to use this code for both gdb and objdump (they do use it
> now, to be more accurate) to relocate the contents of debug sections.
> This is necessary for the general cases of debugging shared objects and
> unlinked object modules.

In principle, bfd_get_relocated_section_contents() should not expect
to see the exact same sort of hash table that is generated by
add_symbols().  It should work with any type of linker hash table.  If
it doesn't work, then linking to a different object file format will
not work.  The same applies to the HOWTO functions, of course.

Of course, in practice linking to a different object file format may
not be supported.  But in general the HOWTO functions can't expect to
see a linker hash table, since they are also called by the assembler.
And there is no reason to write get_relocated_section_contents() to
see a particular type of hash table, because it will never be called
if you use add_symbols() and final_link().

So while I'm perfectly willing to believe that there is a problem, I
don't know what it is.  It seems to me that the simple.c code ought to
be able to call _bfd_generic_link_add_symbols(), and we could make
some guarantees about that specific function.  If that doesn't work,
then why doesn't it?

Ian

Re: gdb.mi/mi-cli.exp failures

[Daniel Jacobowitz]

On Wed, Apr 02, 2003 at 10:53:08AM -0800, Ian Lance Taylor wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> 
> > > > Well, do you have another suggestion for how to approach this?  We're
> > > > not actually linking; but I need to get the symbols from the input file
> > > > into a symbol table with forged offsets in order to apply relocations
> > > > against them.
> > > 
> > > Well, I don't really know the context.  If you're not linking, then it
> > > seems to me that you'll be better off avoiding the linking calls.  The
> > > add_symbols() call is the first phase of a link, and is expected to be
> > > followed by the second phase of a link; despite the name
> > > add_symbols(), it doesn't just add symbols to a hash table.
> > > 
> > > If you really just want to put the symbols into a hash table, can you
> > > just get the symbol table generically and add them to a hash table
> > > yourself?
> > 
> > IIRC, then we may get a different kind of hash table than the
> > platform's relocation application functions expect.  It's been a little
> > while though.
> > 
> > The context is in bfd/simple.c if you want to look at it.  The
> > intention is to use this code for both gdb and objdump (they do use it
> > now, to be more accurate) to relocate the contents of debug sections.
> > This is necessary for the general cases of debugging shared objects and
> > unlinked object modules.
> 
> In principle, bfd_get_relocated_section_contents() should not expect
> to see the exact same sort of hash table that is generated by
> add_symbols().  It should work with any type of linker hash table.  If
> it doesn't work, then linking to a different object file format will
> not work.  The same applies to the HOWTO functions, of course.
> 
> Of course, in practice linking to a different object file format may
> not be supported.  But in general the HOWTO functions can't expect to
> see a linker hash table, since they are also called by the assembler.
> And there is no reason to write get_relocated_section_contents() to
> see a particular type of hash table, because it will never be called
> if you use add_symbols() and final_link().
> 
> So while I'm perfectly willing to believe that there is a problem, I
> don't know what it is.  It seems to me that the simple.c code ought to
> be able to call _bfd_generic_link_add_symbols(), and we could make
> some guarantees about that specific function.  If that doesn't work,
> then why doesn't it?

It appears to work, and fixes one of the two problems.  I'll post a
patch in a little while.  This doesn't solve the question of freeing
the symbol table; I suppose that using bfd_alloc for it may be the way
to go.

Note that this still isn't ideal, because the symbol table is not
clearly cached in the BFD anywhere; so we'll get a new one each time we
relocate a section.  What we really need is to cache the canonicalized
symbol table, I suppose.

Really, BFD isn't set up to work the way I want it to for this
functionality.  I don't know what to do to make it better.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb.mi/mi-cli.exp failures

[Ian Lance Taylor]

Daniel Jacobowitz <drow@mvista.com> writes:

> Note that this still isn't ideal, because the symbol table is not
> clearly cached in the BFD anywhere; so we'll get a new one each time we
> relocate a section.  What we really need is to cache the canonicalized
> symbol table, I suppose.

Can we use the existing bfd_free_cached_info() interface in some way?
That function is probably only implemented for a.out, but it could be
implemented for other targets as well.

Actually, I see one issue, which is that calling this function will
leave information lying around in memory which you might prefer to
free.  And there is the converse issue, which is that every time you
call this function you need to canonicalize the symbol table, and that
is a pain if you call it a lot for the same BFD.  You can solve one or
the other issue, but I don't think you can solve both, since for most
targets the canonical symbol table will point into the internal symbol
information.

Ian