diff mbox

[RFA] PR python/18565 - make Frame.function work for inline frames

Message ID 87eg65lwb2.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Aug. 3, 2016, 3:13 p.m. UTC 
  >>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

>> In any case, IMO this would be the subject of a separate patch.

Yao> This is the prerequisite of Tom's patch, no?  It guarantees that *FUNNAME
Yao> is not allocated if exception is thrown, otherwise *FUNAME is allocated.
Yao> Tom's patch relies on this behaviour.

Yao> I am OK with Tom's original patch, but we need to fix find_frame_funname
Yao> first.

Here's a patch.  Please review.

I'd like to check this and the other into the 7.12 branch as well.

Tom

commit d10153cfc6cb319a75ceb3e3d34ac30efa17e518
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Aug 3 08:27:56 2016 -0600

    Avoid potential memory leak in find_frame_funname
    
    The PR 18565 thread pointed out that, if cp_remove_params can throw
    (we aren't quite sure), then find_frame_funname could leak some
    memory.  This patch avoids any potential issue by rearranging some
    code in find_frame_funname.
    
    Built and regtested on x86-64 Fedora 24.
    
    2016-08-03  Tom Tromey  <tom@tromey.com>
    
    	* stack.c (find_frame_funname): Avoid any possible leak in case
    	cp_remove_params can throw.

Comments

Pedro Alves Aug. 3, 2016, 5:57 p.m. UTC | #1 
On 08/03/2016 04:13 PM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
> 
>>> In any case, IMO this would be the subject of a separate patch.
> 
> Yao> This is the prerequisite of Tom's patch, no?  It guarantees that *FUNNAME
> Yao> is not allocated if exception is thrown, otherwise *FUNAME is allocated.
> Yao> Tom's patch relies on this behaviour.
> 
> Yao> I am OK with Tom's original patch, but we need to fix find_frame_funname
> Yao> first.
> 
> Here's a patch.  Please review.
> 

LGTM.

> I'd like to check this and the other into the 7.12 branch as well.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6b2fb9c..48ecd67 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@ 
 2016-08-03  Tom Tromey  <tom@tromey.com>
 
+	* stack.c (find_frame_funname): Avoid any possible leak in case
+	cp_remove_params can throw.
+
+2016-08-03  Tom Tromey  <tom@tromey.com>
+
 	* NEWS: Mention new Python breakpoint events.
 
 2016-08-02  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/stack.c b/gdb/stack.c
index b9e74df..e890949 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1101,7 +1101,8 @@  find_frame_funname (struct frame_info *frame, char **funname,
 	}
       else
 	{
-	  *funname = xstrdup (SYMBOL_PRINT_NAME (func));
+	  const char *print_name = SYMBOL_PRINT_NAME (func);
+
 	  *funlang = SYMBOL_LANGUAGE (func);
 	  if (funcp)
 	    *funcp = func;
@@ -1112,14 +1113,17 @@  find_frame_funname (struct frame_info *frame, char **funname,
 		 stored in the symbol table, but we stored a version
 		 with DMGL_PARAMS turned on, and here we don't want to
 		 display parameters.  So remove the parameters.  */
-	      char *func_only = cp_remove_params (*funname);
+	      char *func_only = cp_remove_params (print_name);
 
 	      if (func_only)
-		{
-		  xfree (*funname);
-		  *funname = func_only;
-		}
+		*funname = func_only;
 	    }
+
+	  /* If we didn't hit the C++ case above, set *funname here.
+	     This approach is taken to avoid having to install a
+	     cleanup in case cp_remove_params can throw.  */
+	  if (*funname == NULL)
+	    *funname = xstrdup (print_name);
 	}
     }
   else