FYI: clean up symtab producer and debugformat lifetimes

FYI: clean up symtab producer and debugformat lifetimes

[Tom Tromey]

I'm checking this in on the trunk.

Right now the symtab `producer' and `debugformat' fields are copied
while building symtabs and then finally allocated on the objfile
obstack.  I think this is just a waste; in all existing cases, the
values are constants.  So, it is simpler to just require the caller to
guarantee the lifetime of the strings.  I think this clarifies the code,
and this is shown by the fact that this caught a bug in free_symtab.

Built and regtested on x86-64 (compile farm).

Tom

2011-04-04  Tom Tromey  <tromey@redhat.com>

	* xcoffread.c (read_xcoff_symtab): Make `debugfmt' const.
	* symtab.h (struct symtab) <producer, debugformat>: Now const.
	* symmisc.c (free_symtab): Don't free debugformat.
	* buildsym.h (struct subfile) <producer, debugformat>: Now const.
	(record_debugformat, record_producer): Document.
	* buildsym.c (end_symtab): Don't save debugformat and producer
	names on obstack.
	(end_symtab): Don't free debugformat and producer fields.
	(record_debugformat): Don't call xstrdup.
	(record_producer): Likewise.

Index: buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.85
diff -u -r1.85 buildsym.c
--- buildsym.c	23 Mar 2011 18:23:54 -0000	1.85
+++ buildsym.c	4 Apr 2011 14:21:40 -0000
@@ -1111,20 +1111,6 @@
 	     the symbols.  */
 	  symtab->language = subfile->language;
 
-	  /* Save the debug format string (if any) in the symtab.  */
-	  if (subfile->debugformat != NULL)
-	    {
-	      symtab->debugformat = obsavestring (subfile->debugformat,
-					      strlen (subfile->debugformat),
-						  &objfile->objfile_obstack);
-	    }
-
-	  /* Similarly for the producer.  */
-	  if (subfile->producer != NULL)
-	    symtab->producer = obsavestring (subfile->producer,
-					     strlen (subfile->producer),
-					     &objfile->objfile_obstack);
-
 	  /* All symtabs for the main file and the subfiles share a
 	     blockvector, so we need to clear primary for everything
 	     but the main file.  */
@@ -1169,12 +1155,6 @@
 	{
 	  xfree ((void *) subfile->line_vector);
 	}
-      if (subfile->debugformat != NULL)
-	{
-	  xfree ((void *) subfile->debugformat);
-	}
-      if (subfile->producer != NULL)
-	xfree (subfile->producer);
 
       nextsub = subfile->next;
       xfree ((void *) subfile);
@@ -1279,20 +1259,15 @@
 \f
 
 void
-record_debugformat (char *format)
+record_debugformat (const char *format)
 {
-  current_subfile->debugformat = xstrdup (format);
+  current_subfile->debugformat = format;
 }
 
 void
 record_producer (const char *producer)
 {
-  /* The producer is not always provided in the debugging info.
-     Do nothing if PRODUCER is NULL.  */
-  if (producer == NULL)
-    return;
-
-  current_subfile->producer = xstrdup (producer);
+  current_subfile->producer = producer;
 }
 
 /* Merge the first symbol list SRCLIST into the second symbol list
Index: buildsym.h
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.h,v
retrieving revision 1.27
diff -u -r1.27 buildsym.h
--- buildsym.h	1 Jan 2011 15:32:58 -0000	1.27
+++ buildsym.h	4 Apr 2011 14:21:40 -0000
@@ -70,8 +70,8 @@
     struct linetable *line_vector;
     int line_vector_length;
     enum language language;
-    char *producer;
-    char *debugformat;
+    const char *producer;
+    const char *debugformat;
     struct symtab *symtab;
   };
 
@@ -292,7 +292,15 @@
 				  struct block *block,
 				  struct pending_block *opblock);
 
-extern void record_debugformat (char *format);
+/* Record the name of the debug format in the current pending symbol
+   table.  FORMAT must be a string with a lifetime at least as long as
+   the symtab's objfile.  */
+
+extern void record_debugformat (const char *format);
+
+/* Record the name of the debuginfo producer (usually the compiler) in
+   the current pending symbol table.  PRODUCER must be a string with a
+   lifetime at least as long as the symtab's objfile.  */
 
 extern void record_producer (const char *producer);
 
Index: symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.77
diff -u -r1.77 symmisc.c
--- symmisc.c	28 Mar 2011 20:21:04 -0000	1.77
+++ symmisc.c	4 Apr 2011 14:21:40 -0000
@@ -115,8 +115,6 @@
     xfree (s->line_charpos);
   if (s->fullname != NULL)
     xfree (s->fullname);
-  if (s->debugformat != NULL)
-    xfree (s->debugformat);
   xfree (s);
 }
 
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.174
diff -u -r1.174 symtab.h
--- symtab.h	4 Apr 2011 14:10:12 -0000	1.174
+++ symtab.h	4 Apr 2011 14:21:41 -0000
@@ -817,11 +817,11 @@
      for automated testing of gdb but may also be information that is
      useful to the user.  */
 
-  char *debugformat;
+  const char *debugformat;
 
   /* String of producer version information.  May be zero.  */
 
-  char *producer;
+  const char *producer;
 
   /* Full name of file as found by searching the source path.
      NULL if not yet known.  */
Index: xcoffread.c
===================================================================
RCS file: /cvs/src/src/gdb/xcoffread.c,v
retrieving revision 1.84
diff -u -r1.84 xcoffread.c
--- xcoffread.c	7 Mar 2011 16:17:29 -0000	1.84
+++ xcoffread.c	4 Apr 2011 14:21:41 -0000
@@ -944,7 +944,7 @@
     ((struct coff_symfile_info *) objfile->deprecated_sym_private)->strtbl;
   char *debugsec =
     ((struct coff_symfile_info *) objfile->deprecated_sym_private)->debugsec;
-  char *debugfmt = bfd_xcoff_is_xcoff64 (abfd) ? "XCOFF64" : "XCOFF";
+  const char *debugfmt = bfd_xcoff_is_xcoff64 (abfd) ? "XCOFF64" : "XCOFF";
 
   struct internal_syment symbol[1];
   union internal_auxent main_aux;

[patch] Regression: Re: FYI: clean up symtab producer and debugformat lifetimes

[Jan Kratochvil]

Hi Tom,

On Mon, 04 Apr 2011 16:28:53 +0200, Tom Tromey wrote:
> I'm checking this in on the trunk.

symtab is not subfile.

This and several other regressions on all the tested platforms:
-PASS: gdb.base/included.exp: list integer
+XPASS: gdb.base/included.exp: list integer
-PASS: gdb.base/included.exp: info variables integer
+XPASS: gdb.base/included.exp: info variables integer

Due to:
(gdb) info source
Compiled with unknown debugging format.

4b11999f6f77676a5913d772e3b29e04fa057047 is the first bad commit
commit 4b11999f6f77676a5913d772e3b29e04fa057047
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Apr 4 14:29:26 2011 +0000

        * xcoffread.c (read_xcoff_symtab): Make `debugfmt' const.
        * symtab.h (struct symtab) <producer, debugformat>: Now const.
        * symmisc.c (free_symtab): Don't free debugformat.
        * buildsym.h (struct subfile) <producer, debugformat>: Now const.
        (record_debugformat, record_producer): Document.
        * buildsym.c (end_symtab): Don't save debugformat and producer
        names on obstack.
        (end_symtab): Don't free debugformat and producer fields.
        (record_debugformat): Don't call xstrdup.
        (record_producer): Likewise.

:040000 040000 264d42cf955972c5478af0b3515c24683d8d6521 cf07823a615c5afdf0a0f14a60ea77fe5e7baddd M      gdb
bisect run success

No regressions on {x86_64,x86_64-m32,i686}-fedora{13,14,15,-rawhide}-linux-gnu
together with the objfile-data (to be posted along) fix against yesterday.

I will check it in today.


Thanks,
Jan


gdb/
2011-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix regression of displaying the debug format.
	* buildsym.c (end_symtab): Set symtab's debugformat and producer from
	subfile.

--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -1109,6 +1109,12 @@ end_symtab (CORE_ADDR end_addr, struct objfile *objfile, int section)
 	     the symbols.  */
 	  symtab->language = subfile->language;
 
+	  /* Save the debug format string (if any) in the symtab.  */
+	  symtab->debugformat = subfile->debugformat;
+
+	  /* Similarly for the producer.  */
+	  symtab->producer = subfile->producer;
+
 	  /* All symtabs for the main file and the subfiles share a
 	     blockvector, so we need to clear primary for everything
 	     but the main file.  */

Re: [patch] Regression: Re: FYI: clean up symtab producer and debugformat lifetimes

[Jan Kratochvil]

On Tue, 05 Apr 2011 10:47:24 +0200, Jan Kratochvil wrote:
> gdb/
> 2011-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix regression of displaying the debug format.
> 	* buildsym.c (end_symtab): Set symtab's debugformat and producer from
> 	subfile.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2011-04/msg00040.html


Thanks,
Jan