ISR_NOBLOCK bug?

Go To Last Post
3 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Folks,

Compiling this program:

#include 
#include 

unsigned char f(int a, int b, int c) {
	return a + b + c;
}

ISR(TIMER1_OVF_vect, ISR_NOBLOCK) {
	volatile int a, b, c;
	PORTC = f(a,b,c);
}

int main(void) {
	sei();
	while(1) {
		;
	}
}

seems to result in a bug as follows:

9:        ISR(TIMER1_OVF_vect, ISR_NOBLOCK) {
+00000027:   9478        SEI   
+00000028:   921F        PUSH      R1 
+00000029:   920F        PUSH      R0
+0000002A:   B60F        IN        R0,0x3F
+0000002B:   920F        PUSH      R0
+0000002C:   2411        CLR       R1
+0000002D:   932F        PUSH      R18
+0000002E:   933F        PUSH      R19
+0000002F:   934F        PUSH      R20
+00000030:   935F        PUSH      R21
+00000031:   938F        PUSH      R24
+00000032:   939F        PUSH      R25
+00000033:   93DF        PUSH      R29
+00000034:   93CF        PUSH      R28
+00000035:   D000        RCALL     PC+0x0001
+00000036:   D000        RCALL     PC+0x0001
+00000037:   D000        RCALL     PC+0x0001
+00000038:   B7CD        IN        R28,0x3D
+00000039:   B7DE        IN        R29,0x3E
11:       	PORTC = f(a,b,c);
+0000003A:   8189        LDD       R24,Y+1
+0000003B:   819A        LDD       R25,Y+2
+0000003C:   812B        LDD       R18,Y+3
+0000003D:   813C        LDD       R19,Y+4
+0000003E:   814D        LDD       R20,Y+5
+0000003F:   815E        LDD       R21,Y+6
6:        	return a + b + c;
+00000040:   0F28        ADD       R18,R24
+00000041:   0F24        ADD       R18,R20
11:       	PORTC = f(a,b,c);
+00000042:   B928        OUT       0x08,R18
12:       }
+00000043:   9626        ADIW      R28,0x06

The next two instructions perform a non-atomic
write to the stack pointer with interrupts enabled.
YIKES!

+00000044:   BFDE        OUT       0x3E,R29
+00000045:   BFCD        OUT       0x3D,R28
+00000046:   91CF        POP       R28
+00000047:   91DF        POP       R29
+00000048:   919F        POP       R25
+00000049:   918F        POP       R24 
+0000004A:   915F        POP       R21 
+0000004B:   914F        POP       R20
+0000004C:   913F        POP       R19
+0000004D:   912F        POP       R18
+0000004E:   900F        POP       R0
+0000004F:   BE0F        OUT       0x3F,R0
+00000050:   900F        POP       R0
+00000051:   901F        POP       R1
+00000052:   9518        RETI

If the function is an ordinary one (ie not an ISR) then the epilog correctly disables interrupts before modifying the stack pointer:

+00000036:   B928        OUT       0x08,R18
12:       }
+00000037:   9626        ADIW      R28,0x06
+00000038:   B60F        IN        R0,0x3F
+00000039:   94F8        CLI
+0000003A:   BFDE        OUT       0x3E,R29
+0000003B:   BE0F        OUT       0x3F,R0
+0000003C:   BFCD        OUT       0x3D,R28
+0000003D:   91CF        POP       R28
+0000003E:   91DF        POP       R29
+0000003F:   9508        RET

I'm using WinAVR 20090313, compiling for mega48.

Comments?

Christopher Hicks
==

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, a bug.
And already reported:
https://savannah.nongnu.org/bugs...

Stefan Ernst

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sternst wrote:
Yes, a bug.
Thanks Stefan. Having thought a little more, this is unlikely to cause problems in practice: to cause an error, an interrupt would have to occur between the two offending instructions, and those instructions would have to be causing the stack pointer to increment over a 256 byte boundary.

Still, it is a bug and should be fixed.

Having read the bug report, there appears to be an easy workaround, which is to abandon the macros ISR() ISR_NOBLOCK, and do the job by hand as follows:

#include 

void INT0_vect(void) __attribute__((interrupt));

void INT0_vect (void) {
	volatile int a, b, c;
	a = 1; b = 2; c = 3;
}

void INT1_vect(void) __attribute__((signal));

void INT1_vect (void) {
	volatile int a, b, c;
	a = 1; b = 2; c = 3;
}

compiles to

5:        void INT0_vect (void) {
+00000036:   9478        SEI                      
+00000037:   921F        PUSH      R1             
+00000038:   920F        PUSH      R0             
+00000039:   B60F        IN        R0,0x3F        
+0000003A:   920F        PUSH      R0             
+0000003B:   2411        CLR       R1             
+0000003C:   938F        PUSH      R24            
+0000003D:   939F        PUSH      R25            
+0000003E:   93DF        PUSH      R29            
+0000003F:   93CF        PUSH      R28            
+00000040:   D000        RCALL     PC+0x0001      
+00000041:   D000        RCALL     PC+0x0001      
+00000042:   D000        RCALL     PC+0x0001      
+00000043:   B7CD        IN        R28,0x3D       
+00000044:   B7DE        IN        R29,0x3E       
7:        	a = 1; b = 2; c = 3;
+00000045:   E081        LDI       R24,0x01       
+00000046:   E090        LDI       R25,0x00       
+00000047:   839A        STD       Y+2,R25        
+00000048:   8389        STD       Y+1,R24        
+00000049:   E082        LDI       R24,0x02       
+0000004A:   E090        LDI       R25,0x00       
+0000004B:   839C        STD       Y+4,R25        
+0000004C:   838B        STD       Y+3,R24        
+0000004D:   E083        LDI       R24,0x03       
+0000004E:   E090        LDI       R25,0x00       
+0000004F:   839E        STD       Y+6,R25        
+00000050:   838D        STD       Y+5,R24        
8:        }
+00000051:   9626        ADIW      R28,0x06       
+00000052:   94F8        CLI                      
+00000053:   BFDE        OUT       0x3E,R29       
+00000054:   9478        SEI                      
+00000055:   BFCD        OUT       0x3D,R28       
+00000056:   91CF        POP       R28            
+00000057:   91DF        POP       R29            
+00000058:   919F        POP       R25            
+00000059:   918F        POP       R24            
+0000005A:   900F        POP       R0             
+0000005B:   BE0F        OUT       0x3F,R0        
+0000005C:   900F        POP       R0             
+0000005D:   901F        POP       R1             
+0000005E:   9518        RETI                     
@0000005F: __vector_2
12:       void INT1_vect (void) {
+0000005F:   921F        PUSH      R1             
+00000060:   920F        PUSH      R0             
+00000061:   B60F        IN        R0,0x3F        
+00000062:   920F        PUSH      R0             
+00000063:   2411        CLR       R1             
+00000064:   938F        PUSH      R24            
+00000065:   939F        PUSH      R25            
+00000066:   93DF        PUSH      R29            
+00000067:   93CF        PUSH      R28            
+00000068:   D000        RCALL     PC+0x0001      
+00000069:   D000        RCALL     PC+0x0001      
+0000006A:   D000        RCALL     PC+0x0001      
+0000006B:   B7CD        IN        R28,0x3D       
+0000006C:   B7DE        IN        R29,0x3E       
14:       	a = 1; b = 2; c = 3;
+0000006D:   E081        LDI       R24,0x01       
+0000006E:   E090        LDI       R25,0x00       
+0000006F:   839A        STD       Y+2,R25        
+00000070:   8389        STD       Y+1,R24        
+00000071:   E082        LDI       R24,0x02       
+00000072:   E090        LDI       R25,0x00       
+00000073:   839C        STD       Y+4,R25        
+00000074:   838B        STD       Y+3,R24        
+00000075:   E083        LDI       R24,0x03       
+00000076:   E090        LDI       R25,0x00       
+00000077:   839E        STD       Y+6,R25        
+00000078:   838D        STD       Y+5,R24        
15:       }
+00000079:   9626        ADIW      R28,0x06       
+0000007A:   BFDE        OUT       0x3E,R29       
+0000007B:   BFCD        OUT       0x3D,R28       
+0000007C:   91CF        POP       R28            
+0000007D:   91DF        POP       R29            
+0000007E:   919F        POP       R25            
+0000007F:   918F        POP       R24            
+00000080:   900F        POP       R0             
+00000081:   BE0F        OUT       0x3F,R0        
+00000082:   900F        POP       R0             
+00000083:   901F        POP       R1             
+00000084:   9518        RETI

INT0_vect is non-blocking (and correctly guards the stack pointer modify in the epilog), but INT1_vect blocks.

Christopher Hicks
==

PS I rather like the use of "RCALL PC+0x0001" to do an atomic decrement of the stack pointer by two. Crafty!