[RFC] fix for avr_skip_prologue()

[RFC] fix for avr_skip_prologue()

[Theodore Roth]

Hi,

Before I commit this, I wanted to know if this is the correct fix or just
an evil hack side-stepping the some other problem.

The problem shows up when I set a break point on a simple function as
such:

  (gdb) b foo_simple

If foo_simple() has no prologue, the break point is set at the _end_ of
the function (effectively the return insn).

If this fix is acceptable, is it too late to make it into the 5.2 branch?

Ted Roth



2002-07-19  Theodore A. Roth  <troth@verinet.com>

	* gdb/avr-tdep.c(avr_skip_prologue): Fix to return the correct pc.

Index: gdb/avr-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/avr-tdep.c,v
retrieving revision 1.4
diff -u -p -r1.4 avr-tdep.c
--- gdb/avr-tdep.c	17 Jun 2002 23:32:27 -0000	1.4
+++ gdb/avr-tdep.c	19 Jul 2002 18:33:30 -0000
@@ -995,7 +995,12 @@ avr_skip_prologue (CORE_ADDR pc)
     {
       sal = find_pc_line (func_addr, 0);

-      if (sal.line != 0 && sal.end < func_end)
+      /* troth/2002-70-19: For some very simple functions, gcc doesn't
+         generate a prologue and the sal.end ends up being the insn (2 bytes)
+         before func_end (the address of the next func). By adjusting
+         func_end, we can catch these functions and return the correct pc. */
+
+      if (sal.line != 0 && sal.end < (func_end-2))
 	return sal.end;
     }

Re: [RFC] fix for avr_skip_prologue()

[Andrew Cagney]

> Hi,
> 
> Before I commit this, I wanted to know if this is the correct fix or just
> an evil hack side-stepping the some other problem.
> 
> The problem shows up when I set a break point on a simple function as
> such:
> 
>   (gdb) b foo_simple
> 
> If foo_simple() has no prologue, the break point is set at the _end_ of
> the function (effectively the return insn).
> 
> If this fix is acceptable, is it too late to make it into the 5.2 branch?
> 
> Ted Roth

I think so :-(  By the time someone has figured out if it is right or 
wrong, I'll have finished rolling 5.2.1.

Andrew

Re: [RFC] fix for avr_skip_prologue()

[Andrew Cagney]

> Hi,
> 
> Before I commit this, I wanted to know if this is the correct fix or just
> an evil hack side-stepping the some other problem.
> 
> The problem shows up when I set a break point on a simple function as
> such:
> 
>   (gdb) b foo_simple
> 
> If foo_simple() has no prologue, the break point is set at the _end_ of
> the function (effectively the return insn).
> 
> If this fix is acceptable, is it too late to make it into the 5.2 branch?

I don't think your change is any worse than many other *_skip_prologue() 
tweaks.  A typical skip_prologue() function is an accumulation of 
heuristics and a lot of comments.

> 2002-07-19  Theodore A. Roth  <troth@verinet.com>
> 
> 	* gdb/avr-tdep.c(avr_skip_prologue): Fix to return the correct pc.
> 
> Index: gdb/avr-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/avr-tdep.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 avr-tdep.c
> --- gdb/avr-tdep.c	17 Jun 2002 23:32:27 -0000	1.4
> +++ gdb/avr-tdep.c	19 Jul 2002 18:33:30 -0000
> @@ -995,7 +995,12 @@ avr_skip_prologue (CORE_ADDR pc)
>      {
>        sal = find_pc_line (func_addr, 0);
> 
> -      if (sal.line != 0 && sal.end < func_end)
> +      /* troth/2002-70-19: For some very simple functions, gcc doesn't
> +         generate a prologue and the sal.end ends up being the insn (2 bytes)
> +         before func_end (the address of the next func). By adjusting
> +         func_end, we can catch these functions and return the correct pc. */

I'd just also mention that the instruction in question is ``return'' and 
is two bytes long.

> +      if (sal.line != 0 && sal.end < (func_end-2))
>  	return sal.end;
>      }

enjoy,
Andrew

> 

Re: [RFC] fix for avr_skip_prologue()

[Theodore A. Roth]

On Sun, 4 Aug 2002, Andrew Cagney wrote:

>I don't think your change is any worse than many other *_skip_prologue() 
>tweaks.  A typical skip_prologue() function is an accumulation of 
>heuristics and a lot of comments.
>

<snip>

>> -      if (sal.line != 0 && sal.end < func_end)
>> +      /* troth/2002-70-19: For some very simple functions, gcc doesn't
>> +         generate a prologue and the sal.end ends up being the insn (2 bytes)
>> +         before func_end (the address of the next func). By adjusting
>> +         func_end, we can catch these functions and return the correct pc. */
>
>I'd just also mention that the instruction in question is ``return'' and 
>is two bytes long.

Let's see if I understand what you are saying here. It looks like sal.end 
becomes the ``return'' instruction which is 1 insn (2 bytes) before 
func_end. If so, it might make more sense for the patch to be 

+      if (sal.line != 0 && (sal.end+2) < func_end)

Thus if sal.end is the ``return'' insn for the simple function, we just
return the current pc instead of sal.end.

If that makes any sense to you, I'll fix the comment and commit it.

Ted Roth

Re: [RFC] fix for avr_skip_prologue()

[Andrew Cagney]

> On Sun, 4 Aug 2002, Andrew Cagney wrote:
> 
> 
>>I don't think your change is any worse than many other *_skip_prologue() 
>>tweaks.  A typical skip_prologue() function is an accumulation of 
>>heuristics and a lot of comments.
>>
> 
> 
> <snip>
> 
>>> -      if (sal.line != 0 && sal.end < func_end)
>>> +      /* troth/2002-70-19: For some very simple functions, gcc doesn't
>>> +         generate a prologue and the sal.end ends up being the insn (2 bytes)
>>> +         before func_end (the address of the next func). By adjusting
>>> +         func_end, we can catch these functions and return the correct pc. */
> 
>>
>>I'd just also mention that the instruction in question is ``return'' and 
>>is two bytes long.
> 
> 
> Let's see if I understand what you are saying here. It looks like sal.end 
> becomes the ``return'' instruction which is 1 insn (2 bytes) before 
> func_end. If so, it might make more sense for the patch to be 

Yes.  that's what I think is happening based on your comments.

> +      if (sal.line != 0 && (sal.end+2) < func_end)
> 
> Thus if sal.end is the ``return'' insn for the simple function, we just
> return the current pc instead of sal.end.
> 
> If that makes any sense to you, I'll fix the comment and commit it.

Ok.

Andrew