AVR GCC Jump Table Bug

Log in or register to post comments
Go To Last Post
13 posts / 0 new
Author
Message
abcminiuser
abcminiuser's picture
Level: Raving Lunatic
Joined: Fri. Jan 23, 2004
Posts: 9845 View posts
Location: Melbourne, Australia
#1
Posted by abcminiuser: Tue. Aug 26, 2014 - 11:09 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi guys,

Looking for independent confirmation of this before I file a bug report for GCC.

It seems the newer GCC versions (4.8.1 tested here) the compiler will emit jump table optimizations for switch-case statements in some circumstances -- but the generated assembly is invalid in some cases. Specifically, it uses regular LPM instructions to fetch the table contents from FLASH memory, without regard to devices with larger FLASH memory spaces than the standard instruction allows. When used on a >64KB device, this causes the jump target to be pulled pseudo-randomly from FLASH.

Can anyone else see this? When compiling a bootloader for a AT90USB1287, I get:

		switch (USB_ControlRequest.bRequest)
   1efae:	30 91 84 02 	lds	r19, 0x0284
   1efb2:	83 2f       	mov	r24, r19
   1efb4:	90 e0       	ldi	r25, 0x00	; 0
   1efb6:	8a 30       	cpi	r24, 0x0A	; 10
   1efb8:	91 05       	cpc	r25, r1
   1efba:	08 f0       	brcs	.+2      	; 0x1efbe 
   1efbc:	c4 c0       	rjmp	.+392    	; 0x1f146 
   1efbe:	fc 01       	movw	r30, r24
   1efc0:	e4 5b       	subi	r30, 0xB4	; 180
   1efc2:	ff 40       	sbci	r31, 0x0F	; 15
   1efc4:	63 c2       	rjmp	.+1222   	; 0x1f48c <__tablejump2__>
		{

Which leads to:

0001f48c <__tablejump2__>:
   1f48c:	ee 0f       	add	r30, r30
   1f48e:	ff 1f       	adc	r31, r31

0001f490 <__tablejump__>:
   1f490:	05 90       	lpm	r0, Z+
   1f492:	f4 91       	lpm	r31, Z
   1f494:	e0 2d       	mov	r30, r0
   1f496:	09 94       	ijmp

Which then explodes, as the LPM in __tablejump__ doesn't work on a .text section that is relocated to 0x1E000.

I can patch this in the meantime with -fno-jump-tables but I'd like to file a proper report if someone else can confirm it so that it gets fixed.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

Tags:

Tools, Compilers and General Programming
Top
westfw
westfw's picture
Level: Raving Lunatic
Joined: Wed. Jun 19, 2002
Posts: 4783 View posts
Location: SF Bay area
Posted by westfw: Tue. Aug 26, 2014 - 03:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmm. As in "the attempts that the compile goes through to assure that the jump tables are loaded in the first 64k are meaningless if the program is relocated with its start address beyond 64k?" I'll bet that applies to a lot of the data that comes "early" in the linker maps :-(
(how would the compiler know, at compile stage!)

Top
wek
wek's picture
Level: Raving Lunatic
Joined: Fri. Dec 16, 2005
Posts: 3270 View posts
Location: Bratislava, Slovakia
Posted by wek: Tue. Aug 26, 2014 - 06:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

AVR-Libc user manual wrote:
NOTE: The tablejump instructions use the LPM assembler instruction for access to jump tables. Always use -fno-jump-tables switch, if compiling a bootloader for devices with more than 64 KB of code memory.

Once documented, it's not a bug, it's a feature... ;-)

JW

Top
abcminiuser
abcminiuser's picture
Level: Raving Lunatic
Joined: Fri. Jan 23, 2004
Posts: 9845 View posts
Location: Melbourne, Australia
Posted by abcminiuser: Wed. Aug 27, 2014 - 07:31 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dang.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

Top
abcminiuser
abcminiuser's picture
Level: Raving Lunatic
Joined: Fri. Jan 23, 2004
Posts: 9845 View posts
Location: Melbourne, Australia
Posted by abcminiuser: Sun. Aug 31, 2014 - 07:05 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry about the short reply last week Jim et. al. - long day, little free time.

I'd argue this is a dangerous optimization then; if the target architecture could conceivably contain a larger address space than the standard fetch instructions would allow (i.e. LPM and ELPM) the compiler should default to pessimistically generating the extended instructions. Only if an explicit "assume output is not relocated flag" is set should it generate potentially "broken", but slightly smaller, ASM code.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

Top
SprinterSB
SprinterSB's picture
Level: Raving Lunatic
Joined: Thu. Dec 21, 2006
Posts: 2108 View posts
Location: Saar-Lor-Lux
Posted by SprinterSB: Sun. Aug 31, 2014 - 04:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-gcc puts this stuff into section .progmem.gcc_sw_table and asserts it is located in the first 64k of flash.

The default ld scripts puts .progmem.gcc* right after .vectors and thus near the start of the flash.

You'll need really many switch/case to make .progmem.gcc* overflow.

avrfreaks does not support Opera. Profile inactive.

Top
abcminiuser
abcminiuser's picture
Level: Raving Lunatic
Joined: Fri. Jan 23, 2004
Posts: 9845 View posts
Location: Melbourne, Australia
Posted by abcminiuser: Mon. Sep 1, 2014 - 08:40 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Indeed, or a relocated base offset (the reason behind the failure here). I really do think the compiler should generate ELPM by default to ensure it works and have a new flag for the micro-optimization, as bootloaders are common and the reason for the compiled code to fail is not.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

Top
SprinterSB
SprinterSB's picture
Level: Raving Lunatic
Joined: Thu. Dec 21, 2006
Posts: 2108 View posts
Location: Saar-Lor-Lux
Posted by SprinterSB: Mon. Sep 1, 2014 - 09:48 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So you are suggesting to extend avr-gcc so that it can produce relocatable code, i.e. no CALL, no JMP, etc.?

avrfreaks does not support Opera. Profile inactive.

Top
abcminiuser
abcminiuser's picture
Level: Raving Lunatic
Joined: Fri. Jan 23, 2004
Posts: 9845 View posts
Location: Melbourne, Australia
Posted by abcminiuser: Mon. Sep 1, 2014 - 09:54 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No (but of course PIC would be nice!) - I just want a common use case of the compiler to work without the gotchas.

That said, I've no interest in contributing to another GCC development debate; I truly appreciate the amazing work that goes into the compiler from all the wonderful contributors, but every discussion in the past has devolved into the compiler authors telling users exactly why they shouldn't want the thing that they want.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

Top
SprinterSB
SprinterSB's picture
Level: Raving Lunatic
Joined: Thu. Dec 21, 2006
Posts: 2108 View posts
Location: Saar-Lor-Lux
Posted by SprinterSB: Mon. Sep 1, 2014 - 02:02 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

PIC??? You need dynamic linking on AVR??

avrfreaks does not support Opera. Profile inactive.

Top
Bronze Marktheusch
theusch's picture
Level: 10k+ Postman
Joined: Mon. Feb 19, 2001
Posts: 40013 View posts
Location: Wisconsin USA
Posted by theusch: Mon. Sep 1, 2014 - 05:40 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

PIC??? You need dynamic linking on AVR??

Oh, yeah--Position Independent Code.

For a moment, I thought Dean had visited the Dark Side and wanted a new GCC target. (If it doesn't have it already.)

No PIC needed then? DLL and load-and-go and such not appropriate for AVR8?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Top
SprinterSB
SprinterSB's picture
Level: Raving Lunatic
Joined: Thu. Dec 21, 2006
Posts: 2108 View posts
Location: Saar-Lor-Lux
Posted by SprinterSB: Sun. Sep 7, 2014 - 04:45 PM (Reply to #11)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
  •  
  1.  
  •  

avrfreaks does not support Opera. Profile inactive.

Top
SprinterSB
SprinterSB's picture
Level: Raving Lunatic
Joined: Thu. Dec 21, 2006
Posts: 2108 View posts
Location: Saar-Lor-Lux
Posted by SprinterSB: Thu. Sep 11, 2014 - 12:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I added that few instructions to gcc 4.9.2; anyone who needs that feature with an older version can use the attached patch. Even with that enhancement the generated code will neither be relocatable nor position independent.

Attachment(s): 

Plain text icon patch.diff_.txt

avrfreaks does not support Opera. Profile inactive.

Top