RFA: patch to display ia64 function pointers

RFA: patch to display ia64 function pointers

[J. Johnston]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

The following patch fixes the problem when displaying structs or function
pointers that actually point to a function descriptor per the ia64 ABI.
Currently, the function descriptor pointer is erroneously interpreted as
pointing to a function and so gdb spits out some meaningless information.

With the patch, you get to see the actual function that the pointer
is referencing via the function descriptor.

Ok to commit?

2003-07-31  Jeff Johnston  <jjohnstn@redhat.com>

	* ia64-tdep.c (ia64_convert_from_func_addr): New function.
	(ia64_gdbarch_init): Call set_gdbarch_convert_from_func_addr().

[-- Attachment #2: ia64-tdep.funcdesc.patch --]
[-- Type: text/plain, Size: 1575 bytes --]

Index: ia64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-tdep.c,v
retrieving revision 1.92
diff -u -p -r1.92 ia64-tdep.c
--- ia64-tdep.c	23 Jul 2003 18:32:19 -0000	1.92
+++ ia64-tdep.c	31 Jul 2003 21:55:46 -0000
@@ -1852,6 +1852,23 @@ find_func_descr (CORE_ADDR faddr, CORE_A
   return fdesc; 
 }
 
+/* Use the following routine when printing out function pointers
+   so the user can see the function address rather than just the
+   function descriptor.  */
+static CORE_ADDR
+ia64_convert_from_func_ptr_addr (CORE_ADDR addr)
+{
+  struct obj_section *s;
+
+  s = find_pc_section (addr);
+
+  /* check if ADDR points to a function descriptor.  */
+  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
+    return read_memory_unsigned_integer (addr, 8);
+
+  return addr;
+}
+
 static CORE_ADDR
 ia64_push_dummy_call (struct gdbarch *gdbarch, CORE_ADDR func_addr,
 		      struct regcache *regcache, CORE_ADDR bp_addr,
@@ -2277,6 +2294,7 @@ ia64_gdbarch_init (struct gdbarch_info i
   set_gdbarch_deprecated_register_convertible (gdbarch, ia64_register_convertible);
   set_gdbarch_deprecated_register_convert_to_virtual (gdbarch, ia64_register_convert_to_virtual);
   set_gdbarch_deprecated_register_convert_to_raw (gdbarch, ia64_register_convert_to_raw);
+  set_gdbarch_convert_from_func_ptr_addr (gdbarch, ia64_convert_from_func_ptr_addr);
 
   set_gdbarch_use_struct_convention (gdbarch, ia64_use_struct_convention);
   set_gdbarch_deprecated_extract_return_value (gdbarch, ia64_extract_return_value);

Re: RFA: patch to display ia64 function pointers

[Kevin Buettner]

On Jul 31,  6:01pm, J. Johnston wrote:

> The following patch fixes the problem when displaying structs or function
> pointers that actually point to a function descriptor per the ia64 ABI.
> Currently, the function descriptor pointer is erroneously interpreted as
> pointing to a function and so gdb spits out some meaningless information.
> 
> With the patch, you get to see the actual function that the pointer
> is referencing via the function descriptor.
> 
> Ok to commit?
> 
> 2003-07-31  Jeff Johnston  <jjohnstn@redhat.com>
> 
> 	* ia64-tdep.c (ia64_convert_from_func_addr): New function.
> 	(ia64_gdbarch_init): Call set_gdbarch_convert_from_func_addr().

Okay.

Thanks,

Kevin

P.S. Please let me know if you're waiting on me to review any more of
your patches.

Re: RFA: patch to display ia64 function pointers

[J. Johnston]

Kevin Buettner wrote:
> On Jul 31,  6:01pm, J. Johnston wrote:
> 
> 
>>The following patch fixes the problem when displaying structs or function
>>pointers that actually point to a function descriptor per the ia64 ABI.
>>Currently, the function descriptor pointer is erroneously interpreted as
>>pointing to a function and so gdb spits out some meaningless information.
>>
>>With the patch, you get to see the actual function that the pointer
>>is referencing via the function descriptor.
>>
>>Ok to commit?
>>
>>2003-07-31  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>	* ia64-tdep.c (ia64_convert_from_func_addr): New function.
>>	(ia64_gdbarch_init): Call set_gdbarch_convert_from_func_addr().
> 
> 
> Okay.
> 
> Thanks,
> 
> Kevin
> 
> P.S. Please let me know if you're waiting on me to review any more of
> your patches.
> 

Patch checked in, thanks.  No outstanding ia64 patches at the moment.
There is a testsuite change to dump.exp to accomodate ia64 which I
have submitted.  I am also about to submit a change to prologue examination
to handle glibc's sleep() function.

-- Jeff J.

[patch] missing case in epilogue stub detection

[Jafa]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

The following patch fixes a bug where an epilogue stub isn't detected and is
erroneously treated as a subroutine.

Fixes step and next errors when at the end of a function for the ip2k. Mips
may have suffered from the same problem but the occurrence of the code would
have been rare.

2003-08-26  Nick Kelsey  <nickk@ubicom.com>

 * infrun.c (handle_inferior_event): Bug fix in subroutine detection
 code regarding dealing with an epilogue stub. The previous code correctly
 detected the stub in all cases except where the pc is equal to the stub
 entry point.

[-- Attachment #2: infrun.patch --]
[-- Type: application/octet-stream, Size: 255 bytes --]

diff -r1.10 infrun.c
2449,2450c2449
<   if (stop_pc == ecs->stop_func_start	/* Quick test */
<       || (in_prologue (stop_pc, ecs->stop_func_start) &&
---
>   if (((stop_pc == ecs->stop_func_start || in_prologue (stop_pc, ecs->stop_func_start)) &&

Re: [patch] missing case in epilogue stub detection

[Daniel Jacobowitz]

On Tue, Aug 26, 2003 at 03:36:59PM -0700, Jafa wrote:
> The following patch fixes a bug where an epilogue stub isn't detected and is
> erroneously treated as a subroutine.
> 
> Fixes step and next errors when at the end of a function for the ip2k. Mips
> may have suffered from the same problem but the occurrence of the code would
> have been rare.
> 
> 2003-08-26  Nick Kelsey  <nickk@ubicom.com>
> 
>  * infrun.c (handle_inferior_event): Bug fix in subroutine detection
>  code regarding dealing with an epilogue stub. The previous code correctly
>  detected the stub in all cases except where the pc is equal to the stub
>  entry point.


First of all, please use context or (my preference) unified diff.

Second of all, please obey the coding standards; you removed a comment
and made a line too long.

And the actual problem, please explain what the bug is and why this
change is correct.  From the patch I'll make a wild guess: stop_pc ==
ecs->stop_func_start but in_prologue returns false.  That looks like a
bug in your target's prologue code.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch] missing case in epilogue stub detection

[Jafa]

Hi Daniel,

My appologies...
(description below)

diff -u -6 -r1.10 infrun.c
--- infrun.c 20 Aug 2003 17:15:33 -0000 1.10
+++ infrun.c 26 Aug 2003 23:00:59 -0000
@@ -2443,15 +2443,15 @@

       ecs->remove_breakpoints_on_following_step = 1;
       keep_going (ecs);
       return;
     }

-  if (stop_pc == ecs->stop_func_start /* Quick test */
-      || (in_prologue (stop_pc, ecs->stop_func_start) &&
-   !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
+  if (((stop_pc == ecs->stop_func_start /* Quick test.  */
+          || in_prologue (stop_pc, ecs->stop_func_start))
+             && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
       || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
       || ecs->stop_func_name == 0)
     {
       /* It's a subroutine call.  */

       if ((step_over_calls == STEP_OVER_NONE)


The first thing the original code does is "if (stop_pc ==
ecs->stop_func_start /* Quick test */" which means if the stop_pc is equal
to the first instrucion of a function or stub then it bypasses all of the
other checks and blindly assumes it is a function. The "in_prologue" and
"IN_SOLIB_RETURN_TRAMPOLINE" functions never get called because it has
already decided that it is a function.

I believe the original code is valid in assuming that if the pc is at the
start of the func/stub then it can skip the prologue test, however it still
needs to check to see if it is in an epilogue stub.

This is critical because an epilogue stub (by definition) does not return to
the caller, thus cannot be treated as a subroutine.

BTW - Thanks for the feedback, you are probably right, the comment is still
reasonably relavant and my editor doesn't pick up lines being too long.

If there is a better way to solve this problem then I would appreciate your
advice.

Thanks

Nick

----- Original Message ----- 
From: "Daniel Jacobowitz" <drow@mvista.com>
To: "Jafa" <jafa@silicondust.com>
Cc: <gdb-patches@sources.redhat.com>
Sent: Tuesday, August 26, 2003 3:45 PM
Subject: Re: [patch] missing case in epilogue stub detection


> *This message was transferred with a trial version of CommuniGate(tm) Pro*
> On Tue, Aug 26, 2003 at 03:36:59PM -0700, Jafa wrote:
> > The following patch fixes a bug where an epilogue stub isn't detected
and is
> > erroneously treated as a subroutine.
> >
> > Fixes step and next errors when at the end of a function for the ip2k.
Mips
> > may have suffered from the same problem but the occurrence of the code
would
> > have been rare.
> >
> > 2003-08-26  Nick Kelsey  <nickk@ubicom.com>
> >
> >  * infrun.c (handle_inferior_event): Bug fix in subroutine detection
> >  code regarding dealing with an epilogue stub. The previous code
correctly
> >  detected the stub in all cases except where the pc is equal to the stub
> >  entry point.
>
>
> First of all, please use context or (my preference) unified diff.
>
> Second of all, please obey the coding standards; you removed a comment
> and made a line too long.
>
> And the actual problem, please explain what the bug is and why this
> change is correct.  From the patch I'll make a wild guess: stop_pc ==
> ecs->stop_func_start but in_prologue returns false.  That looks like a
> bug in your target's prologue code.
>
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
>

Re: [patch] missing case in epilogue stub detection

[Daniel Jacobowitz]

On Tue, Aug 26, 2003 at 04:01:52PM -0700, Jafa wrote:
> Hi Daniel,
> 
> My appologies...
> (description below)
> 
> diff -u -6 -r1.10 infrun.c
> --- infrun.c 20 Aug 2003 17:15:33 -0000 1.10
> +++ infrun.c 26 Aug 2003 23:00:59 -0000
> @@ -2443,15 +2443,15 @@
> 
>        ecs->remove_breakpoints_on_following_step = 1;
>        keep_going (ecs);
>        return;
>      }
> 
> -  if (stop_pc == ecs->stop_func_start /* Quick test */
> -      || (in_prologue (stop_pc, ecs->stop_func_start) &&
> -   !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
> +  if (((stop_pc == ecs->stop_func_start /* Quick test.  */
> +          || in_prologue (stop_pc, ecs->stop_func_start))
> +             && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
>        || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
>        || ecs->stop_func_name == 0)
>      {
>        /* It's a subroutine call.  */
> 
>        if ((step_over_calls == STEP_OVER_NONE)
> 
> 
> The first thing the original code does is "if (stop_pc ==
> ecs->stop_func_start /* Quick test */" which means if the stop_pc is equal
> to the first instrucion of a function or stub then it bypasses all of the
> other checks and blindly assumes it is a function. The "in_prologue" and
> "IN_SOLIB_RETURN_TRAMPOLINE" functions never get called because it has
> already decided that it is a function.
> 
> I believe the original code is valid in assuming that if the pc is at the
> start of the func/stub then it can skip the prologue test, however it still
> needs to check to see if it is in an epilogue stub.
> 
> This is critical because an epilogue stub (by definition) does not return to
> the caller, thus cannot be treated as a subroutine.
> 
> BTW - Thanks for the feedback, you are probably right, the comment is still
> reasonably relavant and my editor doesn't pick up lines being too long.
> 
> If there is a better way to solve this problem then I would appreciate your
> advice.

Hi Nick,

OK, thank you for the explanation.  Presumably, for whatever target
this code was originally added, the return stubs didn't show up as
functions in the symbol table and so the first check never triggered.
Your explanation makes sense, so I've committed this patch.

[I don't believe this patch requires copyright assignment, but do
you/Ubicom have one in place?  I don't remember.]

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-09-07  Daniel Jacobowitz  <drow@mvista.com>

	From Nick Kelsey <nickk@ubicom.com>:
	* infrun.c (handle_inferior_event): Check IN_SOLIB_RETURN_TRAMPOLINE
	when the stop PC is at the beginning of a function also.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.110
diff -u -p -r1.110 infrun.c
--- infrun.c	19 Jun 2003 15:04:57 -0000	1.110
+++ infrun.c	7 Sep 2003 04:34:55 -0000
@@ -2446,9 +2446,9 @@ process_event_stop_test:
       return;
     }
 
-  if (stop_pc == ecs->stop_func_start	/* Quick test */
-      || (in_prologue (stop_pc, ecs->stop_func_start) &&
-	  !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
+  if (((stop_pc == ecs->stop_func_start	/* Quick test */
+	|| in_prologue (stop_pc, ecs->stop_func_start))
+       && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
       || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
       || ecs->stop_func_name == 0)
     {

Re: [patch] missing case in epilogue stub detection

[Andrew Cagney]

> [I don't believe this patch requires copyright assignment, but do
> you/Ubicom have one in place?  I don't remember.]

As far as I can tell it's limited to ip[24]k files.  I'll check.

Andrew

Re: RFA: patch to display ia64 function pointers

[Michael Elizabeth Chastain]

jj> I am also about to submit a change to prologue examination
jj> to handle glibc's sleep() function.

Hoo hah, that sounds familiar.

Also check select() while you are at it.  On my i686-pc-linux-gnu
most of the test suite problems showed up with sleep(), but real code
is more likely to be in select().

And check out testsuite/gdb.arch/i386-prologue.exp i386-prologue.c.

This has to be done for a lot of targets.  :(  Daniel J suggested
using the s390-tdep.c 'abstract execution' engine on every target
and I think that's a good idea.

Michael C