[RFA/commit/powerpc] breakpoint inserted past function end

[RFA/commit/powerpc] breakpoint inserted past function end

[Joel Brobecker]

On powerpc, the prologue scanner reads instruction after instruction,
and just skips instructions that do not affect a frame.  This means
that it does not stop if if finds and unexpected instruction (which
could possibly happen with optimization, I presume). To avoid scanning
too many instructions, it tries to establish an upper limit.

The upper limit is first computed using the debugging (line) info,
but if that fails, it falls back on an arbitrary 100 bytes (or 25
instructions).  The problem is that, if the function is shorter than
those 25 instructions, we run the risk of skipping the entire function
and returning a PC that's outside our function.

In the event where we can find a symbol for a given PC (and therefore
can determine function start and end addresses), but cannot find an
upper limit using skip_prologue_using_sal, then we can at least limit
make sure that the 25 instructions do not put us beyour our function.
If it does, then further reduce the upper-limit to the end of the function.

gdb/ChangeLog:

        * rs6000-tdep.c (rs6000_skip_prologue): Make sure that the prologue
        upper limit address is not greater than the function end address
        when the upper limit could not be computed using the debugging
        info.

This seems fairly straightforward, but I couldn't run the testsuite
(only the AdaCore testsuite) because I don't have access to a powerpc
machine running an OS that we can run the testsuite on (I ran the AdaCore
testsuite on VxWorks).

I'll commit in a few days, pending comments.

---
 gdb/rs6000-tdep.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index c16e933..9832b5b 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -2090,12 +2090,12 @@ static CORE_ADDR
 rs6000_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
   struct rs6000_framedata frame;
-  CORE_ADDR limit_pc, func_addr;
+  CORE_ADDR limit_pc, func_addr, func_end_addr = 0;
 
   /* See if we can determine the end of the prologue via the symbol table.
      If so, then return either PC, or the PC after the prologue, whichever
      is greater.  */
-  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+  if (find_pc_partial_function (pc, NULL, &func_addr, &func_end_addr))
     {
       CORE_ADDR post_prologue_pc
 	= skip_prologue_using_sal (gdbarch, func_addr);
@@ -2113,6 +2113,11 @@ rs6000_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   if (limit_pc == 0)
     limit_pc = pc + 100;          /* Magic.  */
 
+  /* Do not allow limit_pc to be past the function end, if we know
+     where that end is...  */
+  if (func_end_addr && limit_pc > func_end_addr)
+    limit_pc = func_end_addr;
+
   pc = skip_prologue (gdbarch, pc, limit_pc, &frame);
   return pc;
 }
-- 
1.7.1

Re: [RFA/commit/powerpc] breakpoint inserted past function end

[Thiago Jung Bauermann]

Hi Joel,

On Thu, 2011-01-13 at 18:04 -0500, Joel Brobecker wrote:
> This seems fairly straightforward, but I couldn't run the testsuite
> (only the AdaCore testsuite) because I don't have access to a powerpc
> machine running an OS that we can run the testsuite on (I ran the AdaCore
> testsuite on VxWorks).

Does it help if I run the testsuite on ppc-linux and ppc64-linux?

> I'll commit in a few days, pending comments.

Can't comment... I never needed to dive into prologue analysis.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFA/commit/powerpc] breakpoint inserted past function end

[Michael Snyder]

This seems reasonable to me.

Joel Brobecker wrote:
> On powerpc, the prologue scanner reads instruction after instruction,
> and just skips instructions that do not affect a frame.  This means
> that it does not stop if if finds and unexpected instruction (which
> could possibly happen with optimization, I presume). To avoid scanning
> too many instructions, it tries to establish an upper limit.
> 
> The upper limit is first computed using the debugging (line) info,
> but if that fails, it falls back on an arbitrary 100 bytes (or 25
> instructions).  The problem is that, if the function is shorter than
> those 25 instructions, we run the risk of skipping the entire function
> and returning a PC that's outside our function.
> 
> In the event where we can find a symbol for a given PC (and therefore
> can determine function start and end addresses), but cannot find an
> upper limit using skip_prologue_using_sal, then we can at least limit
> make sure that the 25 instructions do not put us beyour our function.
> If it does, then further reduce the upper-limit to the end of the function.
> 
> gdb/ChangeLog:
> 
>         * rs6000-tdep.c (rs6000_skip_prologue): Make sure that the prologue
>         upper limit address is not greater than the function end address
>         when the upper limit could not be computed using the debugging
>         info.
> 
> This seems fairly straightforward, but I couldn't run the testsuite
> (only the AdaCore testsuite) because I don't have access to a powerpc
> machine running an OS that we can run the testsuite on (I ran the AdaCore
> testsuite on VxWorks).
> 
> I'll commit in a few days, pending comments.
> 
> ---
>  gdb/rs6000-tdep.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index c16e933..9832b5b 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -2090,12 +2090,12 @@ static CORE_ADDR
>  rs6000_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
>  {
>    struct rs6000_framedata frame;
> -  CORE_ADDR limit_pc, func_addr;
> +  CORE_ADDR limit_pc, func_addr, func_end_addr = 0;
>  
>    /* See if we can determine the end of the prologue via the symbol table.
>       If so, then return either PC, or the PC after the prologue, whichever
>       is greater.  */
> -  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
> +  if (find_pc_partial_function (pc, NULL, &func_addr, &func_end_addr))
>      {
>        CORE_ADDR post_prologue_pc
>  	= skip_prologue_using_sal (gdbarch, func_addr);
> @@ -2113,6 +2113,11 @@ rs6000_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
>    if (limit_pc == 0)
>      limit_pc = pc + 100;          /* Magic.  */
>  
> +  /* Do not allow limit_pc to be past the function end, if we know
> +     where that end is...  */
> +  if (func_end_addr && limit_pc > func_end_addr)
> +    limit_pc = func_end_addr;
> +
>    pc = skip_prologue (gdbarch, pc, limit_pc, &frame);
>    return pc;
>  }

Re: [RFA/commit/powerpc] breakpoint inserted past function end

[Thiago Jung Bauermann]

On Mon, 2011-01-17 at 16:40 -0200, Thiago Jung Bauermann wrote:
> Does it help if I run the testsuite on ppc-linux and ppc64-linux?

I tested anyway and there were no regressions.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFA/commit/powerpc] breakpoint inserted past function end

[Joel Brobecker]

> > Does it help if I run the testsuite on ppc-linux and ppc64-linux?
> 
> I tested anyway and there were no regressions.

Hey, Thiago. I just can't say how grateful I am. Thank you so much
for doing this!

-- 
Joel

Re: [RFA/commit/powerpc] breakpoint inserted past function end

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * rs6000-tdep.c (rs6000_skip_prologue): Make sure that the prologue
>         upper limit address is not greater than the function end address
>         when the upper limit could not be computed using the debugging
>         info.

Checked in.  Thanks again to Thiago for the testing...

-- 
Joel