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

[Sergio Durigan Junior <sergiodj@redhat.com>]

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,

> 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);

-- 
Sergio