[PATCH 0/3] a few more fixes from the cleanup checker

[PATCH 0/3] a few more fixes from the cleanup checker

[Tom Tromey]

This series is a few remaining buglets caught by the cleanup checker.

I built and regtested this series on x86-64 Fedora 18.

Tom

[PATCH 3/3] fix solib-som.c

[Tom Tromey]

som_open_symbol_file_object was leaking a cleanup.

	* solib-som.c (som_open_symbol_file_object): Call do_cleanups.
---
 gdb/solib-som.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/solib-som.c b/gdb/solib-som.c
index f88b539..457e464 100644
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
@@ -698,6 +698,7 @@ som_open_symbol_file_object (void *from_ttyp)
   int errcode;
   int from_tty = *(int *)from_ttyp;
   gdb_byte buf[4];
+  struct cleanup *cleanup;
 
   if (symfile_objfile)
     if (!query (_("Attempt to reload symbols from process? ")))
@@ -727,10 +728,11 @@ som_open_symbol_file_object (void *from_ttyp)
       return 0;
     }
 
-  make_cleanup (xfree, filename);
+  cleanup = make_cleanup (xfree, filename);
   /* Have a pathname: read the symbol file.  */
   symbol_file_add_main (filename, from_tty);
 
+  do_cleanups (cleanup);
   return 1;
 }
 
-- 
1.8.1.4

[PATCH 1/3] fix dwarf_decode_line_header

[Tom Tromey]

dwarf_decode_line_header has one return that is missing a call to
do_cleanups.  This patch adds it.

This was found using the cleanup checker.

	* dwarf2read.c (dwarf_decode_line_header): Call do_cleanups
	on all return paths.
---
 gdb/dwarf2read.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4cd0c0e..bbb8584 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -15655,6 +15655,7 @@ dwarf_decode_line_header (unsigned int offset, struct dwarf2_cu *cu)
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
+      do_cleanups (back_to);
       return 0;
     }
   lh->statement_program_end = line_ptr + lh->total_length;
-- 
1.8.1.4

[PATCH 2/3] fix init_cutu_and_read_dies

[Tom Tromey]

init_cutu_and_read_dies creates an outer cleanup and uses its
immediately nested inner cleanup conditionally.  This seems overly
complicated to me; simpler is to just deal with the outer cleanup and
drop the inner one.  That is what this patch implements.

	* dwarf2read.c (init_cutu_and_read_dies): Remove 'free_cu_cleanup'.
	Simplify cleanup handling.
---
 gdb/dwarf2read.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bbb8584..484a307 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4988,7 +4988,7 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu,
   struct die_info *comp_unit_die;
   int has_children;
   struct attribute *attr;
-  struct cleanup *cleanups, *free_cu_cleanup = NULL;
+  struct cleanup *cleanups;
   struct signatured_type *sig_type = NULL;
   struct dwarf2_section_info *abbrev_section;
   /* Non-zero if CU currently points to a DWO file and we need to
@@ -5047,7 +5047,7 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu,
       init_one_comp_unit (cu, this_cu);
 
       /* If an error occurs while loading, release our storage.  */
-      free_cu_cleanup = make_cleanup (free_heap_comp_unit, cu);
+      make_cleanup (free_heap_comp_unit, cu);
     }
 
   /* Get the header.  */
@@ -5176,27 +5176,22 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu,
   die_reader_func (&reader, info_ptr, comp_unit_die, has_children, data);
 
   /* Done, clean up.  */
-  if (free_cu_cleanup != NULL)
+  if (keep)
     {
-      if (keep)
-	{
-	  /* We've successfully allocated this compilation unit.  Let our
-	     caller clean it up when finished with it.  */
-	  discard_cleanups (free_cu_cleanup);
+      /* We've successfully allocated this compilation unit.  Let our
+	 caller clean it up when finished with it.  */
+      discard_cleanups (cleanups);
 
-	  /* We can only discard free_cu_cleanup and all subsequent cleanups.
-	     So we have to manually free the abbrev table.  */
-	  dwarf2_free_abbrev_table (cu);
+      /* We can only discard free_cu_cleanup and all subsequent cleanups.
+	 So we have to manually free the abbrev table.  */
+      dwarf2_free_abbrev_table (cu);
 
-	  /* Link this CU into read_in_chain.  */
-	  this_cu->cu->read_in_chain = dwarf2_per_objfile->read_in_chain;
-	  dwarf2_per_objfile->read_in_chain = this_cu;
-	}
-      else
-	do_cleanups (free_cu_cleanup);
+      /* Link this CU into read_in_chain.  */
+      this_cu->cu->read_in_chain = dwarf2_per_objfile->read_in_chain;
+      dwarf2_per_objfile->read_in_chain = this_cu;
     }
-
-  do_cleanups (cleanups);
+  else
+    do_cleanups (cleanups);
 }
 
 /* Read CU/TU THIS_CU in section SECTION,
-- 
1.8.1.4

Regression for fission-reread.exp and pr13961.exp [Re: [PATCH 2/3] fix init_cutu_and_read_dies]

[Jan Kratochvil]

On Tue, 18 Jun 2013 21:44:28 +0200, Tom Tromey wrote:
> 	* dwarf2read.c (init_cutu_and_read_dies): Remove 'free_cu_cleanup'.
> 	Simplify cleanup handling.

97b6b07c3c5847caf456f267f16b5752ac8cc7c6 is the first bad commit
commit 97b6b07c3c5847caf456f267f16b5752ac8cc7c6
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Jul 22 17:58:51 2013 +0000

    fix init_cutu_and_read_dies

This seems unrelated to
	Runtime regression for gdb.base/reread.exp & co. [Re: [RFA] Remove target_section.bfd]
	http://sourceware.org/ml/gdb-patches/2013-07/msg00443.html

although the affected testcases look similar.

Even with pure FSF GDB HEAD I get regression for:

 Running gdb/testsuite/gdb.dwarf2/fission-reread.exp ...
-PASS: gdb.dwarf2/fission-reread.exp: break main
+ERROR: Couldn't load /unsafegdb/testsuite.unix.-m64/gdb.dwarf2/fission-reread into /unsafegdb/testsuite.unix.-m64/../../gdb/gdb (eof).
+ERROR: Couldn't send break main to GDB.
+UNRESOLVED: gdb.dwarf2/fission-reread.exp: break main
 PASS: gdb.dwarf2/fission-reread.exp: fission-reread
-PASS: gdb.dwarf2/fission-reread.exp: fission-reread - unload
+ERROR: couldn't unload file in /unsafegdb/testsuite.unix.-m64/../../gdb/gdb (timeout).
+UNRESOLVED: gdb.dwarf2/fission-reread.exp: fission-reread - unload

 Running gdb/testsuite/gdb.dwarf2/pr13961.exp ...
-PASS: gdb.dwarf2/pr13961.exp: break main
+ERROR: Couldn't load /unsafegdb/testsuite.unix.-m64/gdb.dwarf2/pr13961 into /unsafegdb/testsuite.unix.-m64/../../gdb/gdb (eof).
+ERROR: Couldn't send break main to GDB.
+UNRESOLVED: gdb.dwarf2/pr13961.exp: break main
 PASS: gdb.dwarf2/pr13961.exp: pr13961

on Fedora 19 x86_64/x86_64-m32/i686 with -lmcheck, I did not try other
OSes/configurations.


Jan

Re: Regression for fission-reread.exp and pr13961.exp [Re: [PATCH 2/3] fix init_cutu_and_read_dies]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> On Tue, 18 Jun 2013 21:44:28 +0200, Tom Tromey wrote:
>> * dwarf2read.c (init_cutu_and_read_dies): Remove 'free_cu_cleanup'.
>> Simplify cleanup handling.

Jan> on Fedora 19 x86_64/x86_64-m32/i686 with -lmcheck, I did not try other
Jan> OSes/configurations.

Thanks Jan.
I will fix it tomorrow.

Tom

Re: Regression for fission-reread.exp and pr13961.exp [Re: [PATCH 2/3] fix init_cutu_and_read_dies]

[Tom Tromey]

>>> * dwarf2read.c (init_cutu_and_read_dies): Remove 'free_cu_cleanup'.
>>> Simplify cleanup handling.

That patch was just crap.  I'm reverting it.  I've added
--enable-libmcheck to my standard configure options besides.

Tom

Re: Regression for fission-reread.exp and pr13961.exp [Re: [PATCH 2/3] fix init_cutu_and_read_dies]

[Jan Kratochvil]

On Tue, 23 Jul 2013 08:50:29 +0200, Tom Tromey wrote:
> That patch was just crap.  I'm reverting it.  I've added
> --enable-libmcheck to my standard configure options besides.

Confirming all the regressions are fixed now.


Thanks,
Jan

Re: [PATCH 0/3] a few more fixes from the cleanup checker

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This series is a few remaining buglets caught by the cleanup checker.
Tom> I built and regtested this series on x86-64 Fedora 18.

I'm checking these in now.

Tom