This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
- From: Doug Evans <dje at google dot com>
- To: Hui Zhu <hui_zhu at mentor dot com>
- Cc: Tom Tromey <tromey at redhat dot com>, Sergio Durigan Junior <sergiodj at redhat dot com>, gdb-patches ml <gdb-patches at sourceware dot org>, Edjunior Barbosa Machado <emachado at linux dot vnet dot ibm dot com>, Nick Clifton <nickc at redhat dot com>
- Date: Mon, 6 Jan 2014 09:12:43 -0800
- Subject: Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983
- Authentication-results: sourceware.org; auth=none
- References: <52C8358B dot 7080101 at mentor dot com> <m3mwja5v01 dot fsf at redhat dot com> <52C97EC0 dot 3080807 at mentor dot com> <87k3edseia dot fsf at fleche dot redhat dot com> <52CA8A7F dot 7090907 at mentor dot com>
On Mon, Jan 6, 2014 at 2:50 AM, Hui Zhu <hui_zhu@mentor.com> wrote:
> On 01/06/14 16:25, Tom Tromey wrote:
>>>>>>>
>>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>>
>>
>> Hui> Thanks. Post a new version.
>>
>> Thanks Hui. This is definitely the direction I think the code should
>> go.
>>
>> Hui> --- a/gdb/symfile-mem.c
>> Hui> +++ b/gdb/symfile-mem.c
>> Hui> @@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
>> Hui> if (name == NULL)
>> Hui> nbfd-> filename = "shared object read from target memory";
>> Hui> else
>> Hui> - {
>> Hui> - nbfd->filename = name;
>> Hui> - gdb_bfd_stash_filename (nbfd);
>> Hui> - xfree (name);
>> Hui> - }
>> Hui> + nbfd->filename = name;
>> Hui> cleanup = make_cleanup_bfd_unref (nbfd);
>>
>> In this hunk there are two things to note.
>>
>> First, there is an earlier assignment to filename (in the context above)
>> that should use xstrdup.
>>
>> Second, the new assignment really ought to free the old nbfd->filename
>> first.
>
>
> I changed this part to:
> xfree (bfd_get_filename (nbfd));
> if (name == NULL)
> nbfd->filename = xstrdup ("shared object read from target memory");
> else
> nbfd->filename = name;
I would prefer a new bfd routine to set the file name.
Then *it* is responsible for freeing the old name.
Any reason to not go that route?
> --- a/gdb/symfile-mem.c
> +++ b/gdb/symfile-mem.c
> @@ -101,14 +101,11 @@ symbol_file_add_from_memory (struct bfd
> error (_("Failed to read a valid object file image from memory."));
> gdb_bfd_ref (nbfd);
> + xfree (bfd_get_filename (nbfd));
This line still screams of excessive chumminess with bfd.
> if (name == NULL)
> - nbfd->filename = "shared object read from target memory";
> + nbfd->filename = xstrdup ("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);
>