bugSimulavr: an AVR simulator - Bugs: bug #45941, [PATCH] Bug in interrupt handling...

 
 

bug #45941: [PATCH] Bug in interrupt handling with instructions that clear interrupt flag

Submitted by:  Marcus Holland-Moritz <mhx>
Submitted on:  Sat 12 Sep 2015 12:48:42 PM UTC  
 
Category: SimulationSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Klaus Rudolph <zfrdh>
Open/Closed: ClosedComponent Version: simulavrxx

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Sun 13 Sep 2015 08:43:30 PM UTC, comment #2:

Hi Klaus!

I can confirm that your fix works with my code. \o/

Thanks,
Marcus

Marcus Holland-Moritz <mhx>
Sun 13 Sep 2015 07:58:01 PM UTC, comment #1:

@markus:

Thanks for your finding!

Can you please check my changes which I did on the master today? Did your program now run as expected?

My fix looks a bit hot because the AvrDevice::Step method is quite ugly implemented... for me there are to many nested checks.

So please have a look for my changes if the hot fix works. Please add this as a remark here. If so, I will close the bug. If not, I will look into it :-)

Klaus Rudolph <zfrdh>
Project AdministratorIn charge of this item.
Sat 12 Sep 2015 12:48:42 PM UTC, original submission:

Hi!

I believe I've found a bug in the way simulavr handles interrupts
in the presence of instructions that disable global interrupts.

In a nutshell, when an interrupt is detected and queued to be
executed after the next statement and the next statement is
either CLI or any other instruction that clears the global
interrupt flag (OUT SREG, ...), the interrupt handler will still
be called /after/ global interrupts have been disabled. This
basically means that code that was meant to be uninterruptible
(protected by CLI / SEI) can now be interrupted as the RETI
will jump back to the first instruction after CLI and will even
leave interrupts enabled.

The documentation for CLI clearly states:

"The interrupts will be immediately disabled. No interrupt
will be executed after the CLI instruction, even if it
occurs simultaneously with the CLI instruction."

It isn't quite as clear for when OUT SREG clears the interrupt
flag, but I verified that the behaviour is the same with an
actual ATmega32u4.

I noticed this behaviour when the scheduler for an RTOS I had
written years ago was running amok if left running for long
enough under simulavr. The code had been running for years
without any issues, but failed on simulavr after ~6 seconds
of simulation time.

I've attached some example code to reproduce the problem. From
the trace, you can see that __vector_6 is called after the
execution of CLI (removed wait states for readability):

clibug.elf 0x009c: main+0xd SEI SREG=[I-----Z-]
clibug.elf 0x009e: main+0xe clibug.elf interrupt on index 6cleared
IRQ prepared for addr c
CLI SREG=[------Z-]
clibug.elf 0x00a0: main+0xf IRQ DETECTED: VectorAddr: 12clibug.elf IrqSystem: IrqHandlerStarted Vec: 6
SP=0x45c 0x50 SP=0x45b 0x0
clibug.elf 0x0018: ,__vector_default+0xc JMP 6c __vector_6
clibug.elf 0x006c: __vector_6 PUSH R1 SP=0x45a 0x0
clibug.elf 0x006e: __vector_6+0x1 PUSH R0 SP=0x459 0xaa
clibug.elf 0x0070: __vector_6+0x2 IN R0, 0x3f
clibug.elf 0x0072: __vector_6+0x3 PUSH R0 SP=0x458 0x2
clibug.elf 0x0074: __vector_6+0x4 EOR R1, R1 SREG=[------Z-]
clibug.elf 0x0076: __vector_6+0x5 NOP
clibug.elf 0x0078: __vector_6+0x6 POP R0 SP=0x459 0x2
clibug.elf 0x007a: __vector_6+0x7 OUT 0x3f, R0
clibug.elf 0x007c: __vector_6+0x8 POP R0 SP=0x45a 0xaa
clibug.elf 0x007e: __vector_6+0x9 POP R1 SP=0x45b 0x0
clibug.elf 0x0080: __vector_6+0xa RETI SP=0x45c 0x0 SP=0x45d 0x50 clibug.elf IrqSystem: IrqHandler Finished Vec: 6
clibug.elf 0x00a0: main+0xf NOP

I've also attached a patch that aims to fix the problem by
checking the current instruction before preparing interrupts.
If the instructions will clear the interrupt flag, interrupt
preparation will be skipped. I'm not sure this is the most
elegant way to solve the problem, in particular as there is
now some redundancy with the instruction decoder, but I
couldn't think of a better way while at the same time keeping
the code simple. I'm totally open for suggestions on how to
do things differently :-)

Thanks,
Marcus

PS: The example code was written for atmega16 and I've used the
following commands to test it:

# avr-gcc -mmcu=atmega16 -gstabs -O2 -std=gnu99 -DF_OSC=1000000 clibug.c -o clibug.elf
# simulavr -d atmega16 -t clibug.trace -F 1000000 -m 10000000 -f clibug.elf

Marcus Holland-Moritz <mhx>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #34862:  clibug.c added by mhx (380B - text/x-csrc)
file #34863:  fix-cli.patch added by mhx (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by zfrdh (Updated the item)
  • -unavailable- added by mhx (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 14 Sep 2015 07:49:06 AM UTCzfrdhStatusNeed Info=>Fixed
      Open/ClosedOpen=>Closed
    Sun 13 Sep 2015 07:54:05 PM UTCzfrdhStatusNone=>Need Info
      Assigned toNone=>zfrdh
    Sat 12 Sep 2015 12:48:42 PM UTCmhxAttached File-=>Added clibug.c, #34862
      Attached File-=>Added fix-cli.patch, #34863

    Back to the top


    Powered by Savane 3.1-cleanup1