[patch/hppa/rfa] unwind fix for functions with no debug info

[patch/hppa/rfa] unwind fix for functions with no debug info

[Randolph Chung]

The attached patch fixes a problem reported by Dave Anglin and others.
When compiled without debug information, static functions have no symbol
table entry in gdb. In this case, frame_func_unwind returns the starting
address of the previously visible function. Using that to do unwinding
causes all sorts of brokeness.... 

On hppa, each properly defined function has an unwind record which gives
the starting and ending address of the function. We can use that to
do unwinding more properly.

Tested on hppa-linux with no regressions.

Ok to apply?

randolph

2004-11-03  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.c (hppa_frame_cache): Avoid using frame_func_unwind () to
	locate the beginning of the function.  When objects are compiled without
	debug symbols, frame_func_unwind can return the wrong function.  We
	can do better than that by using unwind records.
	(hppa_frame_this_id): Likewise.


Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.175
diff -u -p -r1.175 hppa-tdep.c
--- hppa-tdep.c	31 Oct 2004 21:09:28 -0000	1.175
+++ hppa-tdep.c	3 Nov 2004 16:18:40 -0000
@@ -1580,7 +1580,7 @@ hppa_frame_cache (struct frame_info *nex
   cache->saved_regs = trad_frame_alloc_saved_regs (next_frame);
 
   /* Yow! */
-  u = find_unwind_entry (frame_func_unwind (next_frame));
+  u = find_unwind_entry (frame_pc_unwind (next_frame));
   if (!u)
     {
       if (hppa_debug)
@@ -1630,7 +1630,7 @@ hppa_frame_cache (struct frame_info *nex
        symbol information.  hppa_skip_prologue also bounds the returned
        pc by the passed in pc, so it will not return a pc in the next
        function.  */
-    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));
+    prologue_end = hppa_skip_prologue (u->region_start);
     end_pc = frame_pc_unwind (next_frame);
 
     if (prologue_end != 0 && end_pc > prologue_end)
@@ -1638,7 +1638,7 @@ hppa_frame_cache (struct frame_info *nex
 
     frame_size = 0;
 
-    for (pc = frame_func_unwind (next_frame);
+    for (pc = u->region_start;
 	 ((saved_gr_mask || saved_fr_mask
 	   || looking_for_sp || looking_for_rp
 	   || frame_size < (u->Total_frame_size << 3))
@@ -1881,8 +1881,14 @@ static void
 hppa_frame_this_id (struct frame_info *next_frame, void **this_cache,
 			   struct frame_id *this_id)
 {
-  struct hppa_frame_cache *info = hppa_frame_cache (next_frame, this_cache);
-  (*this_id) = frame_id_build (info->base, frame_func_unwind (next_frame));
+  struct hppa_frame_cache *info;
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  struct unwind_table_entry *u;
+
+  info = hppa_frame_cache (next_frame, this_cache);
+  u = find_unwind_entry (pc);
+
+  (*this_id) = frame_id_build (info->base, u->region_start);
 }
 
 static void
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/hppa/rfa] unwind fix for functions with no debug info

[Andrew Cagney]

Randolph Chung wrote:
> The attached patch fixes a problem reported by Dave Anglin and others.
> When compiled without debug information, static functions have no symbol
> table entry in gdb. In this case, frame_func_unwind returns the starting
> address of the previously visible function. Using that to do unwinding
> causes all sorts of brokeness.... 

(frame_func_unwind can also return zero.)

> On hppa, each properly defined function has an unwind record which gives
> the starting and ending address of the function. We can use that to
> do unwinding more properly.
> 
> Tested on hppa-linux with no regressions.
> 
> Ok to apply?
> 
> randolph
> 
> 2004-11-03  Randolph Chung  <tausq@debian.org>
> 
> 	* hppa-tdep.c (hppa_frame_cache): Avoid using frame_func_unwind () to
> 	locate the beginning of the function.  When objects are compiled without
> 	debug symbols, frame_func_unwind can return the wrong function.  We
> 	can do better than that by using unwind records.
> 	(hppa_frame_this_id): Likewise.

Ok for 6.3 and mainline with a comment/change log tweak:

The convention is for the ChangeLog to record what was changed while the 
code records why it was changed.  Consequently, the ChangeLog should 
read something like:

hppa-tdep.c (hppa_frame_cache): Use frame_pc_unwind instead of 
frame_func_unwind () to locate the unwind entry.

and the the corresponding code gets the rationale for doing things the 
current way:

/* Avoid using frame_func_unwind () to locate the beginning of the 
function.  When objects are compiled without debug symbols, 
frame_func_unwind can return the wrong function.  We can do better than 
that by using unwind records.  */

(the GNU coding standard discusses this further)

Andrew


> Index: hppa-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
> retrieving revision 1.175
> diff -u -p -r1.175 hppa-tdep.c
> --- hppa-tdep.c	31 Oct 2004 21:09:28 -0000	1.175
> +++ hppa-tdep.c	3 Nov 2004 16:18:40 -0000
> @@ -1580,7 +1580,7 @@ hppa_frame_cache (struct frame_info *nex
>    cache->saved_regs = trad_frame_alloc_saved_regs (next_frame);
>  
>    /* Yow! */
> -  u = find_unwind_entry (frame_func_unwind (next_frame));
> +  u = find_unwind_entry (frame_pc_unwind (next_frame));


>    if (!u)
>      {
>        if (hppa_debug)
> @@ -1630,7 +1630,7 @@ hppa_frame_cache (struct frame_info *nex
>         symbol information.  hppa_skip_prologue also bounds the returned
>         pc by the passed in pc, so it will not return a pc in the next
>         function.  */
> -    prologue_end = hppa_skip_prologue (frame_func_unwind (next_frame));
> +    prologue_end = hppa_skip_prologue (u->region_start);
>      end_pc = frame_pc_unwind (next_frame);
>  
>      if (prologue_end != 0 && end_pc > prologue_end)
> @@ -1638,7 +1638,7 @@ hppa_frame_cache (struct frame_info *nex
>  
>      frame_size = 0;
>  
> -    for (pc = frame_func_unwind (next_frame);
> +    for (pc = u->region_start;
>  	 ((saved_gr_mask || saved_fr_mask
>  	   || looking_for_sp || looking_for_rp
>  	   || frame_size < (u->Total_frame_size << 3))
> @@ -1881,8 +1881,14 @@ static void
>  hppa_frame_this_id (struct frame_info *next_frame, void **this_cache,
>  			   struct frame_id *this_id)
>  {
> -  struct hppa_frame_cache *info = hppa_frame_cache (next_frame, this_cache);
> -  (*this_id) = frame_id_build (info->base, frame_func_unwind (next_frame));
> +  struct hppa_frame_cache *info;
> +  CORE_ADDR pc = frame_pc_unwind (next_frame);
> +  struct unwind_table_entry *u;
> +
> +  info = hppa_frame_cache (next_frame, this_cache);
> +  u = find_unwind_entry (pc);
> +
> +  (*this_id) = frame_id_build (info->base, u->region_start);
>  }
>  
>  static void

Re: [patch/hppa/rfa] unwind fix for functions with no debug info

[Randolph Chung]

> Ok for 6.3 and mainline with a comment/change log tweak:
> 
> The convention is for the ChangeLog to record what was changed while the 
> code records why it was changed.

will do. thanks.

I'm still looking at another aspect of this problem:

For example, in this backtrace:
(gdb) bt
#0  0x406510a8 in Tcl_Finalize () from /usr/lib/libtcl8.4.so.0
#1  0x40650de0 in Tcl_Exit () from /usr/lib/libtcl8.4.so.0
#2  0x40131224 in exp_new_i_simple () from /usr/lib/libexpect5.42.so.1
#3  0x406273b8 in TclInvokeStringCommand () from /usr/lib/libtcl8.4.so.0
#4  0x40628730 in TclEvalObjvInternal () from /usr/lib/libtcl8.4.so.0
#5  0x406293e8 in Tcl_EvalEx () from /usr/lib/libtcl8.4.so.0
#6  0x406297bc in Tcl_Eval () from /usr/lib/libtcl8.4.so.0
#7  0x00010bc0 in main ()

at frame #2, 0x40131224 is not actually in exp_new_i_simple, but in
another function with no recorded name. this backtrace is confusing;
possibly we should show "#2  0x40131224 in ??? from ..." instead?

thoughts?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/hppa/rfa] unwind fix for functions with no debug info

[Andrew Cagney]

Randolph Chung wrote:
>>Ok for 6.3 and mainline with a comment/change log tweak:
>>
>>The convention is for the ChangeLog to record what was changed while the 
>>code records why it was changed.
> 
> 
> will do. thanks.
> 
> I'm still looking at another aspect of this problem:
> 
> For example, in this backtrace:
> (gdb) bt
> #0  0x406510a8 in Tcl_Finalize () from /usr/lib/libtcl8.4.so.0
> #1  0x40650de0 in Tcl_Exit () from /usr/lib/libtcl8.4.so.0
> #2  0x40131224 in exp_new_i_simple () from /usr/lib/libexpect5.42.so.1
> #3  0x406273b8 in TclInvokeStringCommand () from /usr/lib/libtcl8.4.so.0
> #4  0x40628730 in TclEvalObjvInternal () from /usr/lib/libtcl8.4.so.0
> #5  0x406293e8 in Tcl_EvalEx () from /usr/lib/libtcl8.4.so.0
> #6  0x406297bc in Tcl_Eval () from /usr/lib/libtcl8.4.so.0
> #7  0x00010bc0 in main ()
> 
> at frame #2, 0x40131224 is not actually in exp_new_i_simple, but in
> another function with no recorded name. this backtrace is confusing;
> possibly we should show "#2  0x40131224 in ??? from ..." instead?
> 
> thoughts?

First check frame_unwind_address_in_block, perhaphs the "pc" is past the 
end of the "???" function and into exp_new_i_simple?

Beyond that though things get messy.  The symbol table would need to 
fudge up a "???" symbol for that non-recorded function - something we're 
not yet in a position to do.

Andrew