[avr,sim] PR19401 - Update PC when simulate break instruction

Message ID 57769A40.504@atmel.com
State New, archived
Headers

Commit Message

Sivanupandi, Pitchumani July 1, 2016, 4:28 p.m. UTC
  When there is a software breakpoint for avr target, gdb asks remote to 
write break instruction (0x9598) in the breakpoint location. When target 
hits breakpoint location the it asks remote to restore the content. 
Remote will resume from the PC when GDB asks.

After executing break instruction AVR MCU's PC will point to next 
location. It is the remote (gdb-server/ sim) who sets PC to next 
instruction to be executed (after hitting software breakpoint).

So, AVR simulator's PC is expected to point breakpoint location when it 
hits break insn. Simulator was doing that earlier. It is changed after 
the commit 9943d3185, which introduces sim_engine_halt to handle break. 
Since this function jumps to sim_resume using longjmp to sim_resume, 
code that sets PC (cpu->pc = ipc) is not executed.

Following patch will restore the behavior of setting PC on break.


        case OP_bld:

If OK, could someone commit please? I don't have commit access.

Regards,
Pitchumani

sim/ChangeLog

2016-07-01  Pitchumani Sivanupandi  <pitchumani.s@atmel.com>

     PR target/ 19401
     * avr/interp.c (step_once): Pass break instruction address to
     sim_engine_halt function which writes that to PC. Remove code that
     follows that function call as it is unreachable.
  

Comments

Sivanupandi, Pitchumani July 8, 2016, 5:30 a.m. UTC | #1
Ping!

On Friday 01 July 2016 09:58 PM, Pitchumani Sivanupandi wrote:
> When there is a software breakpoint for avr target, gdb asks remote to
> write break instruction (0x9598) in the breakpoint location. When target
> hits breakpoint location the it asks remote to restore the content.
> Remote will resume from the PC when GDB asks.
>
> After executing break instruction AVR MCU's PC will point to next
> location. It is the remote (gdb-server/ sim) who sets PC to next
> instruction to be executed (after hitting software breakpoint).
>
> So, AVR simulator's PC is expected to point breakpoint location when it
> hits break insn. Simulator was doing that earlier. It is changed after
> the commit 9943d3185, which introduces sim_engine_halt to handle break.
> Since this function jumps to sim_resume using longjmp to sim_resume,
> code that sets PC (cpu->pc = ipc) is not executed.
>
> Following patch will restore the behavior of setting PC on break.
>
> diff --git a/sim/avr/interp.c b/sim/avrinterp.c
> index 31a9940..fcce9b2 100644
> --- a/sim/avr/interp.c
> +++ b/sim/avr/interp.c
> @@ -911,8 +911,7 @@ step_once (SIM_CPU *cpu)
>
>         case OP_break:
>      /* Stop on this address.  */
> -   sim_engine_halt (CPU_STATE (cpu), cpu, NULL, cpu->pc, sim_stopped,
> SIM_SIGTRAP);
> -   cpu->pc = ipc;
> +   sim_engine_halt (CPU_STATE (cpu), cpu, NULL, ipc, sim_stopped,
> SIM_SIGTRAP);
>      break;
>
>         case OP_bld:
>
> If OK, could someone commit please? I don't have commit access.
>
> Regards,
> Pitchumani
>
> sim/ChangeLog
>
> 2016-07-01  Pitchumani Sivanupandi  <pitchumani.s@atmel.com>
>
>      PR target/ 19401
>      * avr/interp.c (step_once): Pass break instruction address to
>      sim_engine_halt function which writes that to PC. Remove code that
>      follows that function call as it is unreachable.
>
  
Denis Chertykov July 19, 2016, 6:49 a.m. UTC | #2
Committed.

2016-07-08 8:30 GMT+03:00 Pitchumani Sivanupandi
<pitchumani.sivanupandi@atmel.com>:
> Ping!
>
>
> On Friday 01 July 2016 09:58 PM, Pitchumani Sivanupandi wrote:
>>
>> When there is a software breakpoint for avr target, gdb asks remote to
>> write break instruction (0x9598) in the breakpoint location. When target
>> hits breakpoint location the it asks remote to restore the content.
>> Remote will resume from the PC when GDB asks.
>>
>> After executing break instruction AVR MCU's PC will point to next
>> location. It is the remote (gdb-server/ sim) who sets PC to next
>> instruction to be executed (after hitting software breakpoint).
>>
>> So, AVR simulator's PC is expected to point breakpoint location when it
>> hits break insn. Simulator was doing that earlier. It is changed after
>> the commit 9943d3185, which introduces sim_engine_halt to handle break.
>> Since this function jumps to sim_resume using longjmp to sim_resume,
>> code that sets PC (cpu->pc = ipc) is not executed.
>>
>> Following patch will restore the behavior of setting PC on break.
>>
>> diff --git a/sim/avr/interp.c b/sim/avrinterp.c
>> index 31a9940..fcce9b2 100644
>> --- a/sim/avr/interp.c
>> +++ b/sim/avr/interp.c
>> @@ -911,8 +911,7 @@ step_once (SIM_CPU *cpu)
>>
>>         case OP_break:
>>      /* Stop on this address.  */
>> -   sim_engine_halt (CPU_STATE (cpu), cpu, NULL, cpu->pc, sim_stopped,
>> SIM_SIGTRAP);
>> -   cpu->pc = ipc;
>> +   sim_engine_halt (CPU_STATE (cpu), cpu, NULL, ipc, sim_stopped,
>> SIM_SIGTRAP);
>>      break;
>>
>>         case OP_bld:
>>
>> If OK, could someone commit please? I don't have commit access.
>>
>> Regards,
>> Pitchumani
>>
>> sim/ChangeLog
>>
>> 2016-07-01  Pitchumani Sivanupandi  <pitchumani.s@atmel.com>
>>
>>      PR target/ 19401
>>      * avr/interp.c (step_once): Pass break instruction address to
>>      sim_engine_halt function which writes that to PC. Remove code that
>>      follows that function call as it is unreachable.
>>
>
  

Patch

diff --git a/sim/avr/interp.c b/sim/avrinterp.c
index 31a9940..fcce9b2 100644
--- a/sim/avr/interp.c
+++ b/sim/avr/interp.c
@@ -911,8 +911,7 @@  step_once (SIM_CPU *cpu)

        case OP_break:
     /* Stop on this address.  */
-   sim_engine_halt (CPU_STATE (cpu), cpu, NULL, cpu->pc, sim_stopped, 
SIM_SIGTRAP);
-   cpu->pc = ipc;
+   sim_engine_halt (CPU_STATE (cpu), cpu, NULL, ipc, sim_stopped, 
SIM_SIGTRAP);
     break;