[avr,pr19401] Update PC after break

  avr-gdb doesn't seem to be rewinding the PC after break. If a breakpoint
is at four byte instruction, it resumes from the middle of the
instruction. This caused the target to reject the next (half) instruction
as illegal. In case of breakpoint at two byte instruction, it resumes from
the the next instruction. Instruction at breakpoint location was skipped
as the PC was rewinded after break.

Test case in PR19401 is the example for the first situation. Below
example is for second situation.

volatile int a = 12;

int main ()
{
  a = 23;
  while (1)
    a++;

  return 0;
}

command line: avr-gcc test.c -mmcu=atmega2560 -g -o test.elf
avr-gdb test.elf
(gdb) target sim
(gdb) load
(gdb) b main
(gdb) run
0x00000124 in main () at sim1.c:5
5         a = 0x0123;
(gdb) disassemble
Dump of assembler code for function main:
   0x0000011a <+0>:     push    r28
   0x0000011c <+2>:     push    r29
   0x0000011e <+4>:     in      r28, 0x3d       ; 61
   0x00000120 <+6>:     in      r29, 0x3e       ; 62
   0x00000122 <+8>:     ldi     r24, 0x23       ; 35
=> 0x00000124 <+10>:    ldi     r25, 0x01       ; 1
   0x00000126 <+12>:    sts     0x0201, r25
   0x0000012a <+16>:    sts     0x0200, r24
   0x0000012e <+20>:    lds     r24, 0x0200
   0x00000132 <+24>:    lds     r25, 0x0201
   0x00000136 <+28>:    adiw    r24, 0x01       ; 1
   0x00000138 <+30>:    sts     0x0201, r25
   0x0000013c <+34>:    sts     0x0200, r24
   0x00000140 <+38>:    rjmp    .-20            ;  0x12e <main+20>
End of assembler dump.
(gdb) si
0x00000126      5         a = 0x0123;
(gdb) p $r24
$1 = 0
(gdb) p $r25
$2 = 1
--
Register r24 should have value 0x23.

Attached patch decrements the PC after break so that target can execute the
instruction at breakpoint location.

No regressions found when tested with the simulator.

If ok, could someone commit please? I do not have commit access.

Regards,
Pitchumani

gdb/ChangeLog

2016-03-23  Pitchumani Sivanupandi  <pitchumani.s@atmel.com>

    * avr-tdep.c (avr_break_insn): Define.
    (avr_breakpoint_from_pc): Remove avr_break_insn define.
    (avr_gdbarch_init): Set decr_pc_after_break hook with the size of
    the break instruction.
static CORE_ADDR
@@ -916,7 +918,6 @@ static const unsigned char *
 avr_breakpoint_from_pc (struct gdbarch *gdbarch,
                        CORE_ADDR *pcptr, int *lenptr)
 {
-    static const unsigned char avr_break_insn [] = { 0x98, 0x95 };
     *lenptr = sizeof (avr_break_insn);
     return avr_break_insn;
 }
@@ -1518,6 +1519,7 @@ avr_gdbarch_init (struct gdbarch_info info, struct
gdbarch_list *arches)
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);

   set_gdbarch_breakpoint_from_pc (gdbarch, avr_breakpoint_from_pc);
+  set_gdbarch_decr_pc_after_break (gdbarch, sizeof (avr_break_insn));

   frame_unwind_append_unwinder (gdbarch, &avr_frame_unwind);
   frame_base_set_default (gdbarch, &avr_frame_base);
  

On 03/23/2016 02:14 PM, Pitchumani Sivanupandi wrote:
> avr-gdb doesn't seem to be rewinding the PC after break. If a breakpoint
> is at four byte instruction, it resumes from the middle of the
> instruction. This caused the target to reject the next (half) instruction
> as illegal. In case of breakpoint at two byte instruction, it resumes from
> the the next instruction. Instruction at breakpoint location was skipped
> as the PC was rewinded after break.
> 
> Test case in PR19401 is the example for the first situation. Below
> example is for second situation.

How was this not noticed before?  

> command line: avr-gcc test.c -mmcu=atmega2560 -g -o test.elf
> avr-gdb test.elf
> (gdb) target sim
> (gdb) load
> (gdb) b main
> (gdb) run
> 0x00000124 in main () at sim1.c:5

I would expect to see mention of a SIGTRAP here, and for all
breakpoints, as gdb won't be able to map the current PC address
with any GDB breakpoint. Doesn't that happen?

Is the simulator behaving differently from real hardware?  Are existing
stubs rewinding the PC themselves, or using a different breakpoint
instruction (by implementing the z/Z packets)?

Thanks,
Pedro Alves
  

On Wed, Mar 23, 2016 at 02:40:03PM +0000, Pedro Alves wrote:
> On 03/23/2016 02:14 PM, Pitchumani Sivanupandi wrote:
> > avr-gdb doesn't seem to be rewinding the PC after break. If a breakpoint
> > is at four byte instruction, it resumes from the middle of the
> > instruction. This caused the target to reject the next (half) instruction
> > as illegal. In case of breakpoint at two byte instruction, it resumes from
> > the the next instruction. Instruction at breakpoint location was skipped
> > as the PC was rewinded after break.
> > 
> > Test case in PR19401 is the example for the first situation. Below
> > example is for second situation.
> 
> How was this not noticed before?  

This test case was working till gdb-7.9. May be a regression from 7.10.

> > command line: avr-gcc test.c -mmcu=atmega2560 -g -o test.elf
> > avr-gdb test.elf
> > (gdb) target sim
> > (gdb) load
> > (gdb) b main
> > (gdb) run
> > 0x00000124 in main () at sim1.c:5
> 
> I would expect to see mention of a SIGTRAP here, and for all
> breakpoints, as gdb won't be able to map the current PC address
> with any GDB breakpoint. Doesn't that happen?

copy-paster error. SIGTRAP occurs for this case.

Update test case and gdb log:

volatile int a = 12;

int main ()
{
  a = 0x0123;
  while (1)
    a++;

  return 0;
}

avr-gcc sim1.c -mmcu=atmega2560 -g -o sim1.elf

<snip>
Reading symbols from sim1.elf...done.
(gdb) target sim
Connected to the simulator.
(gdb) load
Loading section .data, size 0x2 lma 0x146
Loading section .text, size 0x146 lma 0x0
Start address 0x0
Transfer rate: 2624 bits in <1 sec.
(gdb) b main
Breakpoint 1 at 0x122: file sim1.c, line 5.
(gdb) run
Starting program: /home/zero/build/avr-trunk/sim1.elf

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00000124 in main () at sim1.c:5
5         a = 0x0123;
(gdb) disassemble
Dump of assembler code for function main:
   0x0000011a <+0>:     push    r28
   0x0000011c <+2>:     push    r29
   0x0000011e <+4>:     in      r28, 0x3d       ; 61
   0x00000120 <+6>:     in      r29, 0x3e       ; 62
   0x00000122 <+8>:     ldi     r24, 0x23       ; 35
=> 0x00000124 <+10>:    ldi     r25, 0x01       ; 1
   0x00000126 <+12>:    sts     0x0201, r25
   0x0000012a <+16>:    sts     0x0200, r24
   0x0000012e <+20>:    lds     r24, 0x0200
   0x00000132 <+24>:    lds     r25, 0x0201
   0x00000136 <+28>:    adiw    r24, 0x01       ; 1
   0x00000138 <+30>:    sts     0x0201, r25
   0x0000013c <+34>:    sts     0x0200, r24
   0x00000140 <+38>:    rjmp    .-20            ;  0x12e <main+20>
End of assembler dump.
(gdb) si
0x00000126      5         a = 0x0123;
(gdb) p $r24
$1 = 0
(gdb) p $r25
$2 = 1
(gdb)
<snip>
 
> Is the simulator behaving differently from real hardware?  Are existing

Yes, it seems. I checked atmega2560 device with avarice, avr-dragon and
avr-gdb. PC is 0x122 when target hits breakpoint, as expected.

> stubs rewinding the PC themselves, or using a different breakpoint
> instruction (by implementing the z/Z packets)?

Sorry, I don't understand what you mean by existing stubs.

Regards,
Pitchumani
  

On 03/24/2016 01:40 PM, Pitchumani Sivanupandi wrote:
> On Wed, Mar 23, 2016 at 02:40:03PM +0000, Pedro Alves wrote:
>> On 03/23/2016 02:14 PM, Pitchumani Sivanupandi wrote:
>>> avr-gdb doesn't seem to be rewinding the PC after break. If a breakpoint
>>> is at four byte instruction, it resumes from the middle of the
>>> instruction. This caused the target to reject the next (half) instruction
>>> as illegal. In case of breakpoint at two byte instruction, it resumes from
>>> the the next instruction. Instruction at breakpoint location was skipped
>>> as the PC was rewinded after break.
>>>
>>> Test case in PR19401 is the example for the first situation. Below
>>> example is for second situation.
>>
>> How was this not noticed before?
>
> This test case was working till gdb-7.9. May be a regression from 7.10.

Did you do a git bisect to find out why this changed?

>
>>> command line: avr-gcc test.c -mmcu=atmega2560 -g -o test.elf
>>> avr-gdb test.elf
>>> (gdb) target sim
>>> (gdb) load
>>> (gdb) b main
>>> (gdb) run
>>> 0x00000124 in main () at sim1.c:5
>>
>> I would expect to see mention of a SIGTRAP here, and for all
>> breakpoints, as gdb won't be able to map the current PC address
>> with any GDB breakpoint. Doesn't that happen?
>
> copy-paster error. SIGTRAP occurs for this case.

Ack.

>> Is the simulator behaving differently from real hardware?  Are existing
>
> Yes, it seems. I checked atmega2560 device with avarice, avr-dragon and
> avr-gdb. PC is 0x122 when target hits breakpoint, as expected.
>
>> stubs rewinding the PC themselves, or using a different breakpoint
>> instruction (by implementing the z/Z packets)?
>
> Sorry, I don't understand what you mean by existing stubs.

The server side of the GDB remote connection when you do live
debugging with those devices.

So the question remains -- what does the AVR do when it
executes that breakpoint instruction?

- Does the PC point to the breakpoint instruction and thus
   the SIM is wrong.

- Or does the PC point after the breakpoint instruction, which
   suggests that stubs are decrementing the PC themselves before
   presenting the stop to gdb, otherwise they'd have tripped
   on this.

This matters because getting the decrement wrong when the target
does the decrement itself is also bad.  If you have two
consecutive 2-byte instructions, and you set breakpoints on both,
and the code flow jumps to the second instruction, GDB will decrement
the PC incorrectly:

   ADDR1   INSN1    << breakpoint 1
   ADDR2   INSN1    << breakpoint 2
   ...     ...
   ADDRn   jmp ADDR2

So the stub reports "stop at ADDR2", and then GDB misunderstands that
as meaning "stop at ADDR1 + 2", and incorrectly decrements the PC to
"ADDR1".

Thanks,
Pedro Alves
  

On Tuesday 29 March 2016 10:47 PM, Pedro Alves wrote:
> On 03/24/2016 01:40 PM, Pitchumani Sivanupandi wrote:
>> On Wed, Mar 23, 2016 at 02:40:03PM +0000, Pedro Alves wrote:
>>> On 03/23/2016 02:14 PM, Pitchumani Sivanupandi wrote:
>>>> avr-gdb doesn't seem to be rewinding the PC after break. If a 
>>>> breakpoint
>>>> is at four byte instruction, it resumes from the middle of the
>>>> instruction. This caused the target to reject the next (half) 
>>>> instruction
>>>> as illegal. In case of breakpoint at two byte instruction, it 
>>>> resumes from
>>>> the the next instruction. Instruction at breakpoint location was 
>>>> skipped
>>>> as the PC was rewinded after break.
>>>>
>>>> Test case in PR19401 is the example for the first situation. Below
>>>> example is for second situation.
>>>
>>> How was this not noticed before?
>>
>> This test case was working till gdb-7.9. May be a regression from 7.10.
>
> Did you do a git bisect to find out why this changed?
>
>>
>>>> command line: avr-gcc test.c -mmcu=atmega2560 -g -o test.elf
>>>> avr-gdb test.elf
>>>> (gdb) target sim
>>>> (gdb) load
>>>> (gdb) b main
>>>> (gdb) run
>>>> 0x00000124 in main () at sim1.c:5
>>>
>>> I would expect to see mention of a SIGTRAP here, and for all
>>> breakpoints, as gdb won't be able to map the current PC address
>>> with any GDB breakpoint. Doesn't that happen?
>>
>> copy-paster error. SIGTRAP occurs for this case.
>
> Ack.
>
>>> Is the simulator behaving differently from real hardware?  Are existing
>>
>> Yes, it seems. I checked atmega2560 device with avarice, avr-dragon and
>> avr-gdb. PC is 0x122 when target hits breakpoint, as expected.
>>
>>> stubs rewinding the PC themselves, or using a different breakpoint
>>> instruction (by implementing the z/Z packets)?
>>
>> Sorry, I don't understand what you mean by existing stubs.
>
> The server side of the GDB remote connection when you do live
> debugging with those devices.
>
> So the question remains -- what does the AVR do when it
> executes that breakpoint instruction?
>
> - Does the PC point to the breakpoint instruction and thus
>   the SIM is wrong.
>
> - Or does the PC point after the breakpoint instruction, which
>   suggests that stubs are decrementing the PC themselves before
>   presenting the stop to gdb, otherwise they'd have tripped
>   on this.
>
Hi Pedro,

Thanks for the comments. Sorry for the delay in responding.

AVR target stops when executes break insn. It is the gdb-server that
rewrites PC if target executes break insn. Checked atmel backend
and avarice; it restores the content in the breakpoint location (i.e.
replaces break insn with original content).

Simulator for avr target doesn't update PC when break insn is executed.
It used to do that, but it was broken after commit 9943d3185.
Commented in bugzilla.
https://sourceware.org/bugzilla/show_bug.cgi?id=19401

IMHO, it is the simulator needs fix, not the gdb. I'll send out patch 
for simulator.

Thanks.

Regards,
Pitchumani