Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983

[Hui Zhu <hui_zhu@mentor.com>]

On 01/05/14 23:05, Sergio Durigan Junior wrote:
> On Saturday, January 04 2014, Hui Zhu wrote:
>
>> Got double free or corruption with new GDB:
>> (gdb) r
>> Starting program: /home/teawater/tmp/a.out
>> *** glibc detected *** /home/teawater/gdb/git/bgdbno/gdb/gdb: double free or corruption (out): 0x00000000011ed4d0 ***
>> ======= Backtrace: =========
>> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7ffff65f5b96]
>
> [...]
>
>> The reason is when GDB open bfd, it will call gdb_bfd_stash_filename to the memory of abfd.
>> But in binutils/11983, it add "(_bfd_delete_bfd): Free filename." So when gdb try to close the bfd, it will crash.
>>
>> I make a patch to remove all gdb_bfd_stash_filename to fix this issue.
>
> Hm, given what Tom said on
> <https://sourceware.org/bugzilla/show_bug.cgi?id=11983#c12>, I believe
> this is the right way to solve the problem.
>
> Edjunior posted a patch a few days ago that duplicated the name.  I'm
> adding him on the Cc.  I'm also adding Nick and Tom.
>
> BTW, Hui, your patch seems to have been mangled by your MUA.

Thanks.  Post a new version.

Best,
Hui

2014-01-05  Hui Zhu  <hui@codesourcery.com>

	* gdb_bfd.c (gdb_bfd_stash_filename): Removed.
	(gdb_bfd_open): Removed gdb_bfd_stash_filename.
	(gdb_bfd_fopen): Ditto.
	(gdb_bfd_openr): Ditto.
	(gdb_bfd_openw): Ditto.
	(gdb_bfd_openr_iovec): Ditto.
	(gdb_bfd_fdopenr): Ditto.
	* gdb_bfd.h (gdb_bfd_stash_filename): Removed.
	* symfile-mem.c (symbol_file_add_from_memory): Removed
	gdb_bfd_stash_filename.

--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -57,21 +57,6 @@ struct gdb_bfd_section_data
  
  static htab_t all_bfds;
  
-/* See gdb_bfd.h.  */
-
-void
-gdb_bfd_stash_filename (struct bfd *abfd)
-{
-  char *name = bfd_get_filename (abfd);
-  char *data;
-
-  data = bfd_alloc (abfd, strlen (name) + 1);
-  strcpy (data, name);
-
-  /* Unwarranted chumminess with BFD.  */
-  abfd->filename = data;
-}
-
  /* An object of this type is stored in each BFD's user data.  */
  
  struct gdb_bfd_data
@@ -204,7 +189,6 @@ gdb_bfd_open (const char *name, const ch
    gdb_assert (!*slot);
    *slot = abfd;
  
-  gdb_bfd_stash_filename (abfd);
    gdb_bfd_ref (abfd);
    return abfd;
  }
@@ -490,10 +474,7 @@ gdb_bfd_fopen (const char *filename, con
    bfd *result = bfd_fopen (filename, target, mode, fd);
  
    if (result)
-    {
-      gdb_bfd_stash_filename (result);
-      gdb_bfd_ref (result);
-    }
+    gdb_bfd_ref (result);
  
    return result;
  }
@@ -506,10 +487,7 @@ gdb_bfd_openr (const char *filename, con
    bfd *result = bfd_openr (filename, target);
  
    if (result)
-    {
-      gdb_bfd_stash_filename (result);
-      gdb_bfd_ref (result);
-    }
+    gdb_bfd_ref (result);
  
    return result;
  }
@@ -522,10 +500,7 @@ gdb_bfd_openw (const char *filename, con
    bfd *result = bfd_openw (filename, target);
  
    if (result)
-    {
-      gdb_bfd_stash_filename (result);
-      gdb_bfd_ref (result);
-    }
+    gdb_bfd_ref (result);
  
    return result;
  }
@@ -553,10 +528,7 @@ gdb_bfd_openr_iovec (const char *filenam
  				 pread_func, close_func, stat_func);
  
    if (result)
-    {
-      gdb_bfd_ref (result);
-      gdb_bfd_stash_filename (result);
-    }
+    gdb_bfd_ref (result);
  
    return result;
  }
@@ -603,10 +575,7 @@ gdb_bfd_fdopenr (const char *filename, c
    bfd *result = bfd_fdopenr (filename, target, fd);
  
    if (result)
-    {
-      gdb_bfd_ref (result);
-      gdb_bfd_stash_filename (result);
-    }
+    gdb_bfd_ref (result);
  
    return result;
  }
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -24,12 +24,6 @@
  
  DECLARE_REGISTRY (bfd);
  
-/* Make a copy ABFD's filename using bfd_alloc, and reassign it to the
-   BFD.  This ensures that the BFD's filename has the same lifetime as
-   the BFD itself.  */
-
-void gdb_bfd_stash_filename (struct bfd *abfd);
-
  /* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
     Returns NULL on error.  On success, returns a new reference to the
     BFD, which must be freed with gdb_bfd_unref.  BFDs returned by this
@@ -79,22 +73,22 @@ int gdb_bfd_crc (struct bfd *abfd, unsig
  \f
  
  /* A wrapper for bfd_fopen that initializes the gdb-specific reference
-   count and calls gdb_bfd_stash_filename.  */
+   count.  */
  
  bfd *gdb_bfd_fopen (const char *, const char *, const char *, int);
  
  /* A wrapper for bfd_openr that initializes the gdb-specific reference
-   count and calls gdb_bfd_stash_filename.  */
+   count.  */
  
  bfd *gdb_bfd_openr (const char *, const char *);
  
  /* A wrapper for bfd_openw that initializes the gdb-specific reference
-   count and calls gdb_bfd_stash_filename.  */
+   count.  */
  
  bfd *gdb_bfd_openw (const char *, const char *);
  
  /* A wrapper for bfd_openr_iovec that initializes the gdb-specific
-   reference count and calls gdb_bfd_stash_filename.  */
+   reference count.  */
  
  bfd *gdb_bfd_openr_iovec (const char *filename, const char *target,
  			  void *(*open_func) (struct bfd *nbfd,
@@ -112,12 +106,12 @@ bfd *gdb_bfd_openr_iovec (const char *fi
  					    struct stat *sb));
  
  /* A wrapper for bfd_openr_next_archived_file that initializes the
-   gdb-specific reference count and calls gdb_bfd_stash_filename.  */
+   gdb-specific reference count.  */
  
  bfd *gdb_bfd_openr_next_archived_file (bfd *archive, bfd *previous);
  
  /* A wrapper for bfd_fdopenr that initializes the gdb-specific
-   reference count and calls gdb_bfd_stash_filename.  */
+   reference count.  */
  
  bfd *gdb_bfd_fdopenr (const char *filename, const char *target, int fd);
  
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
    if (name == NULL)
      nbfd->filename = "shared object read from target memory";
    else
-    {
-      nbfd->filename = name;
-      gdb_bfd_stash_filename (nbfd);
-      xfree (name);
-    }
+    nbfd->filename = name;
  
    cleanup = make_cleanup_bfd_unref (nbfd);