diff mbox

Delete temporary string within demangler even in failure cases.

Message ID 1401188253-15214-1-git-send-email-aburgess@broadcom.com
State Committed
Headers show

Commit Message

Andrew Burgess May 27, 2014, 10:57 a.m. UTC 
Spotted that a call to demangle_template might allocate storage within a
temporary string even if the call to demangle_template eventually returns
failure.
This will never cause the demangler to crash, but does leak memory, as a
result I've not added any tests for this.

Calling string_delete is safe, even if nothing is allocated into the
string, the string is initialised with string_init, so we know the internal
pointers are NULL.

I don't have gcc write privilages, so if this is approved could someone
please commit for me.

Thanks,
Andrew



libiberty/ChangeLog

	* cplus-dem.c (do_type): Call string_delete even if the call to
	demangle_template fails.
---
 libiberty/cplus-dem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Ian Lance Taylor May 27, 2014, 1:47 p.m. UTC | #1 
On Tue, May 27, 2014 at 3:57 AM, Andrew Burgess <aburgess@broadcom.com> wrote:
>
> libiberty/ChangeLog
>
>         * cplus-dem.c (do_type): Call string_delete even if the call to
>         demangle_template fails.

This is OK.

Thanks.

I have to ask: you know this code is not used, right?  You're looking
at the old demangler, for symbols generated by versions of g++ before
GCC 3.4 (released 2004).  The demangler for current versions of g++ is
in cp-demangle.c.

Ian
Andrew Burgess June 10, 2014, 6:08 p.m. UTC | #2 
On 27/05/2014 2:47 PM, Ian Lance Taylor wrote:
> On Tue, May 27, 2014 at 3:57 AM, Andrew Burgess <aburgess@broadcom.com> wrote:
>>
>> libiberty/ChangeLog
>>
>>         * cplus-dem.c (do_type): Call string_delete even if the call to
>>         demangle_template fails.
> 
> This is OK.
> 
> Thanks.
> 
> I have to ask: you know this code is not used, right?  You're looking
> at the old demangler, for symbols generated by versions of g++ before
> GCC 3.4 (released 2004).  The demangler for current versions of g++ is
> in cp-demangle.c.

Sorry for the delay.  Yes, I know it's not very old code, but it is
still shipped, and consumers such as gdb can still trigger this code
path.

Thanks for taking a look at this patch for me.

I don't have gcc write permissions, would you (or any other interested
maintainer) mind committing this for me please.

Thanks,
Andrew
Pedro Alves June 11, 2014, 11:02 a.m. UTC | #3 
On 06/10/2014 07:08 PM, Andrew Burgess wrote:

> I don't have gcc write permissions, would you (or any other interested
> maintainer) mind committing this for me please.

Done, and merged to binutils-gdb git repo.

Thanks,
diff mbox

Patch

diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index e948487..1c713aa 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -3656,7 +3656,10 @@  do_type (struct work_stuff *work, const char **mangled, string *result)
 		    string_delete (&temp);
 		  }
 		else
-		  break;
+		  {
+		    string_delete (&temp);
+		    break;
+		  }
 	      }
 	    else if (**mangled == 'Q')
 	      {