Re: [patch] Fix crash in read_pe_exported_syms

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Saturday, March 02 2013, Corinna Vinschen wrote:

> Hi,

Hi Corinna,

> when running GDB from current CVS on a PE/COFF target, and if this
> target has no debug symbols, nor any exported symbols, then GDB crashes
> with a SEGV in the first do_cleanup called from coff_symfile_read.
>
> The reason is that read_pe_exported_syms creates two cleanup handlers,
> one of them referring to a symbol on the local stack:
>
>   struct read_pe_section_data *section_data;
>   [...]
>   section_data = xzalloc (...)
>   make_cleanup (free_current_contents, &section_data);
>
> but then returns from the function early in three different scenarios
> without calling do_cleanup.  The subsequent do_cleanup call in
> coff_symfile_read now tries to dereference from an invalid stack address
> and ultimately crashes.
>
> Below is a patch.  Ok to apply?

[I am not a maintainer.]

You are right, the function returns without taking care of the possible
cleanups.  IMO the patch is almost obvious, but thanks anyway for
sending it here.

I don't want to create a flamewar here, but ISTR that the use of `goto'
is discouraged in GDB/GNU.  I remember I submitted a patch once that
made use of `goto' in a similar way that you are doing here, and I was
asked to rewrite it.

Anyway, recently I faced a similar issue (i.e., having to call
`do_cleanup' in several places, and what I did was to actually call it
in all those places instead of using `goto'.  You might want to wait
until a global maintainer emits some opinion about this.

Thanks,

-- 
Sergio