[RFA] Use scoped_free_pendings in coff_symtab_read

  PR gdb/18624 concerns an assertion failure that occurs when setting a
breakpoint in a Go program on Windows.

What happens here is that coff_symtab_read uses buildsym but does not
instantiate scoped_free_pendings.  So, the struct pending objects are
never released.  Later, dwarf2read.c calls buildsym_init, which
asserts.

This patch fixes the problem by instantiating scoped_free_pendings in
coff_symtab_read.

Tested using the test executable from the PR.  I don't know how to
test this more fully.

gdb/ChangeLog
2018-06-29  Tom Tromey  <tom@tromey.com>

	PR gdb/18624:
	* coffread.c (coff_symtab_read): Use scoped_free_pendings.
---
 gdb/ChangeLog  | 5 +++++
 gdb/coffread.c | 3 +++
 2 files changed, 8 insertions(+)
  

Hi Tom,

On Fri, Jun 29, 2018 at 03:12:07PM -0600, Tom Tromey wrote:
> PR gdb/18624 concerns an assertion failure that occurs when setting a
> breakpoint in a Go program on Windows.
> 
> What happens here is that coff_symtab_read uses buildsym but does not
> instantiate scoped_free_pendings.  So, the struct pending objects are
> never released.  Later, dwarf2read.c calls buildsym_init, which
> asserts.
> 
> This patch fixes the problem by instantiating scoped_free_pendings in
> coff_symtab_read.
> 
> Tested using the test executable from the PR.  I don't know how to
> test this more fully.
> 
> gdb/ChangeLog
> 2018-06-29  Tom Tromey  <tom@tromey.com>
> 
> 	PR gdb/18624:
> 	* coffread.c (coff_symtab_read): Use scoped_free_pendings.

I don't know this area very well, so can't really comment much
on the patch itself. But I'm pretty sure you're part of the few
that know the general architecture of this part of GDB the most.
What I did was test this patch with AdaCore's testsuite on x86-windows,
and detected no regression :-). I hope that helps!


>  gdb/ChangeLog  | 5 +++++
>  gdb/coffread.c | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 0cbccf5f8a6..1605c6a7da9 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-06-29  Tom Tromey  <tom@tromey.com>
> +
> +	PR gdb/18624:
> +	* coffread.c (coff_symtab_read): Use scoped_free_pendings.
> +
>  2018-06-29  Pedro Alves  <palves@redhat.com>
>  
>  	* thread.c (thread_target_id_str): New, factored out from ...
> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index 30583e1dda4..9e2edde19a8 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -803,6 +803,9 @@ coff_symtab_read (minimal_symbol_reader &reader,
>    CORE_ADDR tmpaddr;
>    struct minimal_symbol *msym;
>  
> +  buildsym_init ();
> +  scoped_free_pendings free_pending;
> +
>    /* Work around a stdio bug in SunOS4.1.1 (this makes me nervous....
>       it's hard to know I've really worked around it.  The fix should
>       be harmless, anyway).  The symptom of the bug is that the first
> -- 
> 2.17.1
  

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

Tom> PR gdb/18624 concerns an assertion failure that occurs when setting a
Tom> breakpoint in a Go program on Windows.

Tom> What happens here is that coff_symtab_read uses buildsym but does not
Tom> instantiate scoped_free_pendings.  So, the struct pending objects are
Tom> never released.  Later, dwarf2read.c calls buildsym_init, which
Tom> asserts.

Tom> This patch fixes the problem by instantiating scoped_free_pendings in
Tom> coff_symtab_read.

Tom> Tested using the test executable from the PR.  I don't know how to
Tom> test this more fully.

Ping.  Joel replied to this, but I didn't take his reply to be an approval.
I think this patch would be an ok candidate for 8.2 as well, so please
consider that as well.

Tom
  

On 07/16/2018 05:38 PM, Tom Tromey wrote:

> Ping.  Joel replied to this, but I didn't take his reply to be an approval.
> I think this patch would be an ok candidate for 8.2 as well, so please
> consider that as well.

OK for master/8.2.

Thanks,
Pedro Alves