[1/3] Fix leak in mdebugread.c
Commit Message
Coverity points out that all the "continue;" statements in the switch
case in parse_partial_symbols leak STABSTRING. This is because we
only release STABSTRING at the end of the scope, with:
if (stabstring
&& stabstring != debug_info->ss + fh->issBase + sh.iss)
xfree (stabstring);
but that bit of code is skipped if a case in the switch statement ends
with "continue".
Fix this by using gdb::unique_xmalloc_ptr to manage the heap-allocated
version of 'stabsstring'.
I don't know how to test this.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* mdebugread.c (parse_partial_symbols): Use
gdb::unique_xmalloc_ptr to manage heap-allocated 'stabsstring'.
---
gdb/mdebugread.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
Comments
On 2018-12-14 14:12, Pedro Alves wrote:
> Coverity points out that all the "continue;" statements in the switch
> case in parse_partial_symbols leak STABSTRING. This is because we
> only release STABSTRING at the end of the scope, with:
>
> if (stabstring
> && stabstring != debug_info->ss + fh->issBase + sh.iss)
> xfree (stabstring);
>
> but that bit of code is skipped if a case in the switch statement ends
> with "continue".
>
> Fix this by using gdb::unique_xmalloc_ptr to manage the heap-allocated
> version of 'stabsstring'.
This LGTM, AFAICT.
> I don't know how to test this.
Go back in time?
Simon
@@ -2767,6 +2767,9 @@ parse_partial_symbols (minimal_symbol_reader &reader,
/* Handle stabs continuation. */
{
char *stabstring = debug_info->ss + fh->issBase + sh.iss;
+ /* If we need to heap-allocate STABSTRING, this owns
+ it. */
+ gdb::unique_xmalloc_ptr<char> stabstring_storage;
int len = strlen (stabstring);
while (stabstring[len - 1] == '\\')
@@ -2789,14 +2792,19 @@ parse_partial_symbols (minimal_symbol_reader &reader,
stabstring2 = debug_info->ss + fh->issBase + sh2.iss;
len2 = strlen (stabstring2);
- /* Concatinate stabstring2 with stabstring1. */
- if (stabstring
- && stabstring != debug_info->ss + fh->issBase + sh.iss)
- stabstring
- = (char *) xrealloc (stabstring, len + len2 + 1);
+ /* Concatenate stabstring2 with stabstring1. */
+ if (stabstring_storage != nullptr)
+ {
+ stabstring_storage.reset
+ ((char *) xrealloc (stabstring_storage.release (),
+ len + len2 + 1));
+ stabstring = stabstring_storage.get ();
+ }
else
{
- stabstring = (char *) xmalloc (len + len2 + 1);
+ stabstring_storage.reset
+ ((char *) xmalloc (len + len2 + 1));
+ stabstring = stabstring_storage.get ();
strcpy (stabstring, stabstring1);
}
strcpy (stabstring + len, stabstring2);
@@ -3332,9 +3340,6 @@ parse_partial_symbols (minimal_symbol_reader &reader,
hex_string (type_code)); /* CUR_SYMBOL_TYPE */
continue;
}
- if (stabstring
- && stabstring != debug_info->ss + fh->issBase + sh.iss)
- xfree (stabstring);
}
/* end - Handle continuation */
}