[RFA] Fix a memory corruption in mdebugread.c

[Daniel Jacobowitz]

On Tue, Dec 31, 2002 at 06:15:20PM +0400, Joel Brobecker wrote:
> This problem is happening on Tru64 5.1A.
> 
> GDB crashed while reading the debugging information of an application
> created by one of our customers. Their application is a mix of C++ and
> Ada. In order to investigate, this customer gave us the binaries of
> their application as we don't have a C++ compiler on Tru64. We managed
> to find the source of the problem, and fix it (hopefuly :), but not
> having a C++ compiler, we are not able to produce a testcase for it.
> 
> Here is what happened:

[snip]

> The following patch is, I admit, a minimal attempt at fixing the
> problem. It would probably be more complete to handle StProc symbol
> records in the counting loop and skip the whole stProc sequence, just
> as we do for stBlock et al:

So I assume the stProc is nested directly in the type, and we generate
a complaint() in the default below?  It's worth bearing in mind that we
don't actually support C++ in mdebugread; debugging the C++ part won't
work well at all.  We won't recognize any member methods for instance.

> 
>               case stBlock:
>               case [...]:
>               case stStruct:
>                 {
>                   if (tsym.index != 0)
>                     {
>                       /* This is something like a struct within a
>                          struct.  Skip over the fields of the inner
>                          struct.  The -1 is because the for loop will
>                          increment ext_tsym.  */
>                       ext_tsym = ((char *) debug_info->external_sym
>                                   + ((cur_fdr->isymBase + tsym.index - 1)
>                                      * external_sym_size));
>                     }
> 
> Unfornately, I lack the time to do this. Instead, I did the following
> trivial change, which just ignores any stEnd symbol records if they are
> not the one ending the struct definition. This is done by matching the
> symbol name associated to the stEnd SYMR against the name of the struct.
> 
> This change fixes the problem reported by our customer, and does not
> introduce any regression in the testsuite. Unfortunately, we don't have
> a C++ compiler, so the C++ part of the testsuite is inoperable for us,
> and we could not test the effect of this change on C++.

My concern from your description is that a constructor may have the
same name as the enclosing type.  Are these mangled names?  Qualified
names?  Base names, in which case the constructor is a problem?  You
should be able to figure this out from looking at a couple of the names
found by the check below.

> Given my current analysis, this change seems sensible. I am therefore
> recommending it for inclusion. If acceptable, we may also want to
> include it in the 5.3 branch as well, as it fixes a crash. Any feedback
> from somebody having a C++ compiler would be greatly appreciated. 
> 
> 2002-12-31  J. Brobecker  <brobecker@gnat.com>
> 
>         * mdebugread.c (parse_symbol): Make sure to identify the correct
>         stEnd symbol record while counting the number of fields when parsing
>         the debugging information for a structure. Otherwise, GDB sometimes
>         ends up under-counting the number of felds in the struct, and this
>         causes later a memory corruption responsible for a GDB crash when
>         running or attaching to the application.
>         Fixes [B927-009]
> 
> Ok to commit?

That's not a proper ChangeLog entry; it should be smeting like:

2002-12-31  J. Brobecker  <brobecker@gnat.com>

	* mdebugread.c (parse_symbol): Count until the stEnd matching
	the structure name.


[Don't blame me, blame GNU....]


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer