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

Submitter:  Marcus Holland-Moritz <mhx>
Submitted:  Sat 12 Sep 2015 12:48:42 PM UTC
   
 
Category:  Simulation Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  zfrdh
Open/Closed:  Closed Component Version:  * simulavrxx
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group administrator
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 Files:
   
   
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
  • -email is unavailable- added by zfrdh (Updated the item)
  • -email is unavailable- added by mhx (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-09-14 zfrdh StatusNeed Info Fixed
        Open/ClosedOpen Closed
    2015-09-13 zfrdh StatusNone Need Info
        Assigned toNone zfrdh
    2015-09-12 mhx Attached File- Added clibug.c, #34862
        Attached File- Added fix-cli.patch, #34863

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code