gcc problem with switch

Log in or register to post comments
Go To Last Post
37 posts / 0 new
Author
Message
steve17
steve17's picture
Level: Raving Lunatic
Joined: Tue. Sep 7, 2004
Posts: 6436 View posts
Location: New York State
#1
Posted by steve17: Tue. Jan 6, 2015 - 10:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There seems to be a problem with switch and more than 6 cases.   The CPU will sometimes jump into the wilderness.   It's not that simple though. 

 

I am testing a program with two tasks.  One has the big switch statement and the other does not.  If I build with only the one with the switch, I get the problem.  If I add the other task, there is no problem.   Weird.

 

I had this problem with Studio 6.2.1153 so I installed SP1 and I still have the problem.  My current version of avr-gcc is 4.6.2.  GNU toolchain 3.4.0_663.

 

I can get around it easily enough.  I normally run with both tasks anyway.  I removed the other task to do some testing and got blindsided with this thing.   Another workaround of course, is to replace the switch with a series of if statements.

 

EDIT:  I should have said I'm programming an Xmega.

Tags:

Tools, Compilers and General Programming
Last Edited: Tue. Jan 6, 2015 - 10:18 PM
Top
larryvc
larryvc's picture
Level: Raving Lunatic
Joined: Thu. Dec 6, 2007
Posts: 4229 View posts
Location: Redmond, WA, USA
Posted by larryvc: Tue. Jan 6, 2015 - 10:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That's an odd problem.  Is this code something you can post or PM?

"I may make you feel but I can't make you think" - Jethro Tull - Thick As A Brick

"void transmigratus(void) {transmigratus();} // recursio infinitus" - larryvc

"It's much more practical to rely on the processing powers of the real debugger, i.e. the one between the keyboard and chair." - JW wek3

"When you arise in the morning think of what a privilege it is to be alive: to breathe, to think, to enjoy, to love." -  Marcus Aurelius

Top
keith v
keith v's picture
Level: Hangaround
Joined: Fri. Oct 24, 2014
Posts: 176 View posts
Posted by keith v: Tue. Jan 6, 2015 - 10:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's not GCC

I'd bet you have a buffer over run or invalid pointer.

Again...

It's not gcc

Keith Vasilakes

Firmware engineer

Minnesota

Top
steve17
steve17's picture
Level: Raving Lunatic
Joined: Tue. Sep 7, 2004
Posts: 6436 View posts
Location: New York State
Posted by steve17: Tue. Jan 6, 2015 - 11:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The code is not secret.  I am trying to come up with a simpler application that still fails.

 

I don't think there are any buffers involved.  The code fails during initialization of the USART.  It's more complicated than it has to be but it is pretty flexible.  It figures out the correct baud settings for the USART based on the PER clock frequency.  That's the switch code that now fails.  This code has been running fine for a couple of years.

Top
steve17
steve17's picture
Level: Raving Lunatic
Joined: Tue. Sep 7, 2004
Posts: 6436 View posts
Location: New York State
Posted by steve17: Wed. Jan 7, 2015 - 02:37 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

For starters, here are the two switch statements.  The good one has 4 cases removed with the #if 0 thing.  I don't currently use those cases.  The program runs like a well oiled watch

 

The bad one has those 4 cases restored.  When the cpu gets to the switch statement, it jumps to 0x00002e7f where there is no source.

 

I have attached the good and bad .lss files for your perusing enjoyment.  To get past the security detail I had to add a .txt on the end of the file names.  I also added a screen shot of the debugger when it goes wrong.

 

I believe avr-gcc has been known to mess up switches in the past.

 

 


// Good set_baud_rate file.  I removed 4 cases with #if 0


#ifndef SET_BAUD_RATE_H
#define SET_BAUD_RATE_H

#include "myTypes.h"
#include "usart_regs.h"
#include "system_clock.h"
#include "common_clocks.h"

class Set_baud_rate   {
public:

   static Set_baud_rate* set_baud_rate_p;


   Set_baud_rate(Common_clocks::MyBaud baud_rate, Usart_regs* usart_regs_p)   {
      set_baud_rate_p = this;
      this->baud_rate = baud_rate;
      this->usart_regs_p = usart_regs_p;

      Calculate_and_set();
      }


   void Set_baud(Common_clocks::MyBaud baud_rate)   {
      this->baud_rate = baud_rate;
      Calculate_and_set();
      }


   void Calculate_and_set()   {
      Common_clocks::Per_clock_rate per_clock_rate = System_clock::system_clock_p->Fetch_per_clock_frequency();
      switch(baud_rate)   {
         case Common_clocks::_57600:
            Do_57600(per_clock_rate);
            break;

         case Common_clocks::_115200:
            Do_115200(per_clock_rate);
            break;

         default:
            break;
         }

      usart_regs_p->Set_UBBR_and_scale_factor((u_int8)UBBR, scale_factor);
      }


   Common_clocks::MyBaud Fetch_baud()   {
      return baud_rate;
      }


   ~Set_baud_rate()   {
      }


private:
   void Do_57600(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
         case Common_clocks::Per_clock_48000000: // 0.16
            Set_UBBR_and_scale_factor(51, 0);
            break;

         case Common_clocks::Per_clock_24000000:
            Set_UBBR_and_scale_factor(25, 0);
            break;

         case Common_clocks::Per_clock_12000000:
            Set_UBBR_and_scale_factor(192, -4);  // 0.16      // could be (0, 12)?
            break;

#if 0                                                        /// *** removing these cases makes it work
         case Common_clocks::Per_clock_6000000:
            Set_UBBR_and_scale_factor(11, -1);   // 0.16
            break;

         case Common_clocks::Per_clock_3000000:
            Set_UBBR_and_scale_factor(9, -2);    // 0.16
            break;

         case Common_clocks::Per_clock_2000000:
            Set_UBBR_and_scale_factor(75, -6);
            break;

         case Common_clocks::Per_clock_4000000:
            Set_UBBR_and_scale_factor(107, -5);  // 0.08
            break;
#endif

         case Common_clocks::Per_clock_8000000:
            Set_UBBR_and_scale_factor(123, -4);  // 0.08
            break;

         case Common_clocks::Per_clock_16000000:
            Set_UBBR_and_scale_factor(131, -3);      // 0.08 % error
            break;

         default:
            break;
         }
      }


   void Do_115200(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
         case Common_clocks::Per_clock_2000000:
            Set_UBBR_and_scale_factor(11, -7);
            break;

         case Common_clocks::Per_clock_3000000:      // 0.16 % error
            Set_UBBR_and_scale_factor(5, -3);
            break;

         case Common_clocks::Per_clock_4000000:
            Set_UBBR_and_scale_factor(75, -6);
            break;

         case Common_clocks::Per_clock_6000000:      // 0.16 % error
            Set_UBBR_and_scale_factor(9, -2);
            break;

         case Common_clocks::Per_clock_8000000:
            Set_UBBR_and_scale_factor(214, -6);      // 0.08 % error
            break;

         case Common_clocks::Per_clock_12000000:
            Set_UBBR_and_scale_factor(176, -5);      // 0.16 % error
            break;

         case Common_clocks::Per_clock_16000000:
            Set_UBBR_and_scale_factor(123, -4);      // 0.08 % error
            break;

         case Common_clocks::Per_clock_24000000:
            Set_UBBR_and_scale_factor(  96, -3);     // 0.16 % error    // could be 12, 0 ?
            break;

         case Common_clocks::Per_clock_32000000:
            Set_UBBR_and_scale_factor( 131, -3);     // 0.08 % error
            break;

         case Common_clocks::Per_clock_48000000:
            Set_UBBR_and_scale_factor(  12, +1);     // 0.16 % error
            break;

         default:
            break;
         }
      }


   void Set_UBBR_and_scale_factor(u_int16 UBBR, char scale_factor)   {
      this->UBBR = UBBR;
      this->scale_factor = scale_factor;
      }

   Common_clocks::MyBaud baud_rate;

   u_int16 UBBR;
   char scale_factor;
   Usart_regs* usart_regs_p;

   };

#endif

 

 


// Bad set_baud_rate.h    I restored the 4 cases with #if 1


#ifndef SET_BAUD_RATE_H
#define SET_BAUD_RATE_H

#include "myTypes.h"
#include "usart_regs.h"
#include "system_clock.h"
#include "common_clocks.h"

class Set_baud_rate   {
public:

   static Set_baud_rate* set_baud_rate_p;


   Set_baud_rate(Common_clocks::MyBaud baud_rate, Usart_regs* usart_regs_p)   {
      set_baud_rate_p = this;
      this->baud_rate = baud_rate;
      this->usart_regs_p = usart_regs_p;

      Calculate_and_set();
      }


   void Set_baud(Common_clocks::MyBaud baud_rate)   {
      this->baud_rate = baud_rate;
      Calculate_and_set();
      }


   void Calculate_and_set()   {
      Common_clocks::Per_clock_rate per_clock_rate = System_clock::system_clock_p->Fetch_per_clock_frequency();
      switch(baud_rate)   {
         case Common_clocks::_57600:
            Do_57600(per_clock_rate);
            break;

         case Common_clocks::_115200:
            Do_115200(per_clock_rate);
            break;

         default:
            break;
         }

      usart_regs_p->Set_UBBR_and_scale_factor((u_int8)UBBR, scale_factor);
      }


   Common_clocks::MyBaud Fetch_baud()   {
      return baud_rate;
      }


   ~Set_baud_rate()   {
      }


private:
   void Do_57600(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
         case Common_clocks::Per_clock_48000000: // 0.16
            Set_UBBR_and_scale_factor(51, 0);
            break;

         case Common_clocks::Per_clock_24000000:
            Set_UBBR_and_scale_factor(25, 0);
            break;

         case Common_clocks::Per_clock_12000000:
            Set_UBBR_and_scale_factor(192, -4);  // 0.16      // could be (0, 12)?
            break;

#if 1                                                        /// *** restoring these cases makes it fail
         case Common_clocks::Per_clock_6000000:
            Set_UBBR_and_scale_factor(11, -1);   // 0.16
            break;

         case Common_clocks::Per_clock_3000000:
            Set_UBBR_and_scale_factor(9, -2);    // 0.16
            break;

         case Common_clocks::Per_clock_2000000:
            Set_UBBR_and_scale_factor(75, -6);
            break;

         case Common_clocks::Per_clock_4000000:
            Set_UBBR_and_scale_factor(107, -5);  // 0.08
            break;
#endif

         case Common_clocks::Per_clock_8000000:
            Set_UBBR_and_scale_factor(123, -4);  // 0.08
            break;

         case Common_clocks::Per_clock_16000000:
            Set_UBBR_and_scale_factor(131, -3);      // 0.08 % error
            break;

         default:
            break;
         }
      }


   void Do_115200(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
         case Common_clocks::Per_clock_2000000:
            Set_UBBR_and_scale_factor(11, -7);
            break;

         case Common_clocks::Per_clock_3000000:      // 0.16 % error
            Set_UBBR_and_scale_factor(5, -3);
            break;

         case Common_clocks::Per_clock_4000000:
            Set_UBBR_and_scale_factor(75, -6);
            break;

         case Common_clocks::Per_clock_6000000:      // 0.16 % error
            Set_UBBR_and_scale_factor(9, -2);
            break;

         case Common_clocks::Per_clock_8000000:
            Set_UBBR_and_scale_factor(214, -6);      // 0.08 % error
            break;

         case Common_clocks::Per_clock_12000000:
            Set_UBBR_and_scale_factor(176, -5);      // 0.16 % error
            break;

         case Common_clocks::Per_clock_16000000:
            Set_UBBR_and_scale_factor(123, -4);      // 0.08 % error
            break;

         case Common_clocks::Per_clock_24000000:
            Set_UBBR_and_scale_factor(  96, -3);     // 0.16 % error    // could be 12, 0 ?
            break;

         case Common_clocks::Per_clock_32000000:
            Set_UBBR_and_scale_factor( 131, -3);     // 0.08 % error
            break;

         case Common_clocks::Per_clock_48000000:
            Set_UBBR_and_scale_factor(  12, +1);     // 0.16 % error
            break;

         default:
            break;
         }
      }


   void Set_UBBR_and_scale_factor(u_int16 UBBR, char scale_factor)   {
      this->UBBR = UBBR;
      this->scale_factor = scale_factor;
      }

   Common_clocks::MyBaud baud_rate;

   u_int16 UBBR;
   char scale_factor;
   Usart_regs* usart_regs_p;

   };

#endif

 

 

Attachment(s): 

Plain text icon Good_Test_switch_crash.lss_.txt
Plain text icon Bad_Test_switch_crash.lss_.txt
Top
joeymorin
joeymorin's picture
Level: 10k+ Postman
Joined: Tue. Jul 17, 2012
Posts: 11771 View posts
Location: Location, Location
Posted by joeymorin: Wed. Jan 7, 2015 - 05:40 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
When the cpu gets to the switch statement, it jumps to 0x00002e7f where there is no source.
That's the word address.  The .lss shows code using byte addresses.  2E7F is 5CFE as a byte address:

private:
   void Do_57600(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
    10f2:	50 e0       	ldi	r21, 0x00	; 0
    10f4:	4a 30       	cpi	r20, 0x0A	; 10
    10f6:	51 05       	cpc	r21, r1
    10f8:	08 f0       	brcs	.+2      	; 0x10fc <_ZN13Set_baud_rate17Calculate_and_setEv+0x2a>
    10fa:	91 c0       	rjmp	.+290    	; 0x121e <_ZN13Set_baud_rate17Calculate_and_setEv+0x14c>
    10fc:	42 50       	subi	r20, 0x02	; 2
    10fe:	5f 4f       	sbci	r21, 0xFF	; 255
    1100:	fa 01       	movw	r30, r20
    1102:	0c 94 7f 2e 	jmp	0x5cfe	; 0x5cfe <__tablejump2__>
   void Do_115200(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
    1180:	50 e0       	ldi	r21, 0x00	; 0
    1182:	4a 30       	cpi	r20, 0x0A	; 10
    1184:	51 05       	cpc	r21, r1
    1186:	08 f0       	brcs	.+2      	; 0x118a <_ZN13Set_baud_rate17Calculate_and_setEv+0xb8>
    1188:	4a c0       	rjmp	.+148    	; 0x121e <_ZN13Set_baud_rate17Calculate_and_setEv+0x14c>
    118a:	48 5f       	subi	r20, 0xF8	; 248
    118c:	5e 4f       	sbci	r21, 0xFE	; 254
    118e:	fa 01       	movw	r30, r20
    1190:	0c 94 7f 2e 	jmp	0x5cfe	; 0x5cfe <__tablejump2__>
private:
   void Do_57600(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
    1c9a:	17 96       	adiw	r26, 0x07	; 7
    1c9c:	4c 91       	ld	r20, X
    1c9e:	50 e0       	ldi	r21, 0x00	; 0
    1ca0:	4a 30       	cpi	r20, 0x0A	; 10
    1ca2:	51 05       	cpc	r21, r1
    1ca4:	08 f0       	brcs	.+2      	; 0x1ca8 <_ZN8Usart_d0C1Ev+0xa2>
    1ca6:	4a c0       	rjmp	.+148    	; 0x1d3c <_ZN8Usart_d0C1Ev+0x136>
    1ca8:	4e 5e       	subi	r20, 0xEE	; 238
    1caa:	5e 4f       	sbci	r21, 0xFE	; 254
    1cac:	fa 01       	movw	r30, r20
    1cae:	0c 94 7f 2e 	jmp	0x5cfe	; 0x5cfe <__tablejump2__>
private:
   void Do_57600(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
    48ac:	90 e0       	ldi	r25, 0x00	; 0
    48ae:	8a 30       	cpi	r24, 0x0A	; 10
    48b0:	91 05       	cpc	r25, r1
    48b2:	08 f0       	brcs	.+2      	; 0x48b6 <_ZN9Usart_mon12Make_changesEv+0x76>
    48b4:	91 c0       	rjmp	.+290    	; 0x49d8 <_ZN9Usart_mon12Make_changesEv+0x198>
    48b6:	84 5e       	subi	r24, 0xE4	; 228
    48b8:	9e 4f       	sbci	r25, 0xFE	; 254
    48ba:	fc 01       	movw	r30, r24
    48bc:	0c 94 7f 2e 	jmp	0x5cfe	; 0x5cfe <__tablejump2__>
   void Do_115200(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
    493a:	90 e0       	ldi	r25, 0x00	; 0
    493c:	8a 30       	cpi	r24, 0x0A	; 10
    493e:	91 05       	cpc	r25, r1
    4940:	08 f0       	brcs	.+2      	; 0x4944 <_ZN9Usart_mon12Make_changesEv+0x104>
    4942:	4a c0       	rjmp	.+148    	; 0x49d8 <_ZN9Usart_mon12Make_changesEv+0x198>
    4944:	8a 5d       	subi	r24, 0xDA	; 218
    4946:	9e 4f       	sbci	r25, 0xFE	; 254
    4948:	fc 01       	movw	r30, r24
    494a:	0c 94 7f 2e 	jmp	0x5cfe	; 0x5cfe <__tablejump2__>
00005cfe <__tablejump2__>:
    5cfe:	ee 0f       	add	r30, r30
    5d00:	ff 1f       	adc	r31, r31

00005d02 <__tablejump__>:
    5d02:	05 90       	lpm	r0, Z+
    5d04:	f4 91       	lpm	r31, Z
    5d06:	e0 2d       	mov	r30, r0
    5d08:	19 94       	eijmp

This is an example of a jump table.  AVR GCC will use them for switch statements above a certain size.  That threshold is different for different targets.

https://www.avrfreaks.net/forum/avr-objdump-and-jump-tables

 

The eijmp suggests that you're using a device with > 128kB flash, and the EIND register.  The address of that register is the same for all devices: 0x3C (I/O space).  The CRT initialises it:

     230:	00 e0       	ldi	r16, 0x00	; 0
     232:	0c bf       	out	0x3c, r16	; 60

Nothing else touches it, so all eijmp instructions will jump somewhere in the first 128kB of flash.

 

Note the 'code' which immediately follows the interrupt vector table:

     1fc:	a4 08       	sbc	r10, r4
     1fe:	9d 08       	sbc	r9, r13
     200:	ab 08       	sbc	r10, r11
     202:	96 08       	sbc	r9, r6
     204:	b2 08       	sbc	r11, r2
     206:	8f 08       	sbc	r8, r15
     208:	b9 08       	sbc	r11, r9
     20a:	89 08       	sbc	r8, r9
     20c:	0f 09       	sbc	r16, r15
     20e:	83 08       	sbc	r8, r3
     210:	ca 08       	sbc	r12, r10
     212:	d1 08       	sbc	r13, r1
     214:	d8 08       	sbc	r13, r8
     216:	df 08       	sbc	r13, r15
     218:	e6 08       	sbc	r14, r6
     21a:	ed 08       	sbc	r14, r13
     21c:	f4 08       	sbc	r15, r4
     21e:	fb 08       	sbc	r15, r11
     220:	02 09       	sbc	r16, r2
     222:	09 09       	sbc	r16, r9
     224:	7f 0e       	add	r7, r31
     226:	77 0e       	add	r7, r23
     228:	87 0e       	add	r8, r23
     22a:	6f 0e       	add	r6, r31
     22c:	8f 0e       	add	r8, r31
     22e:	67 0e       	add	r6, r23
     230:	97 0e       	add	r9, r23
     232:	60 0e       	add	r6, r16
     234:	9e 0e       	add	r9, r30
     236:	59 0e       	add	r5, r25
     238:	81 24       	eor	r8, r1
     23a:	7a 24       	eor	r7, r10
     23c:	88 24       	eor	r8, r8
     23e:	73 24       	eor	r7, r3
     240:	8f 24       	eor	r8, r15
     242:	6c 24       	eor	r6, r12
     244:	96 24       	eor	r9, r6
     246:	66 24       	eor	r6, r6
     248:	ec 24       	eor	r14, r12
     24a:	60 24       	eor	r6, r0
     24c:	a7 24       	eor	r10, r7
     24e:	ae 24       	eor	r10, r14
     250:	b5 24       	eor	r11, r5
     252:	bc 24       	eor	r11, r12
     254:	c3 24       	eor	r12, r3
     256:	ca 24       	eor	r12, r10
     258:	d1 24       	eor	r13, r1
     25a:	d8 24       	eor	r13, r8
     25c:	df 24       	eor	r13, r15
     25e:	e6 24       	eor	r14, r6

While avr-objdump blindly disassembles those flash locations as instructions, they are in fact word addresses, and represent the addresses of the code for each case statement in all of the switch statements (and other code) for which jump tables are in use.  So the first address is (little-endian) 08A4, which is byte address 1148:

   void Set_UBBR_and_scale_factor(u_int16 UBBR, char scale_factor)   {
      this->UBBR = UBBR;
    1148:	8b e4       	ldi	r24, 0x4B	; 75
    114a:	90 e0       	ldi	r25, 0x00	; 0
    114c:	89 83       	std	Y+1, r24	; 0x01
    114e:	9a 83       	std	Y+2, r25	; 0x02
      this->scale_factor = scale_factor;
    1150:	8a ef       	ldi	r24, 0xFA	; 250
    1152:	8b 83       	std	Y+3, r24	; 0x03
    1154:	64 c0       	rjmp	.+200    	; 0x121e <_ZN13Set_baud_rate17Calculate_and_setEv+0x14c>

Note that this is an inlined copy of the function you call in each of the case clauses for the switch statement in Do_57600(), with registers loaded with the arguments for the appropriate case clause, in this case for (75, -6), so the case clause in question is:

private:
   void Do_57600(Common_clocks::Per_clock_rate per_clock_rate)   {
      switch(per_clock_rate)   {
         case Common_clocks::Per_clock_48000000: // 0.16
            Set_UBBR_and_scale_factor(51, 0);
            break;

         case Common_clocks::Per_clock_24000000:
            Set_UBBR_and_scale_factor(25, 0);
            break;

         case Common_clocks::Per_clock_12000000:
            Set_UBBR_and_scale_factor(192, -4);  // 0.16      // could be (0, 12)?
            break;

#if 1                                                        /// *** restoring these cases makes it fail
         case Common_clocks::Per_clock_6000000:
            Set_UBBR_and_scale_factor(11, -1);   // 0.16
            break;

         case Common_clocks::Per_clock_3000000:
            Set_UBBR_and_scale_factor(9, -2);    // 0.16
            break;

         case Common_clocks::Per_clock_2000000:
            Set_UBBR_and_scale_factor(75, -6);
            break;

         case Common_clocks::Per_clock_4000000:
            Set_UBBR_and_scale_factor(107, -5);  // 0.08
            break;
#endif

         case Common_clocks::Per_clock_8000000:
            Set_UBBR_and_scale_factor(123, -4);  // 0.08
            break;

         case Common_clocks::Per_clock_16000000:
            Set_UBBR_and_scale_factor(131, -3);      // 0.08 % error
            break;

         default:
            break;
         }
      }

 

Then it jumps to:

      usart_regs_p->Set_UBBR_and_scale_factor((u_int8)UBBR, scale_factor);
    121e:	9b 81       	ldd	r25, Y+3	; 0x03
    1220:	89 81       	ldd	r24, Y+1	; 0x01
    1222:	ec 81       	ldd	r30, Y+4	; 0x04
    1224:	fd 81       	ldd	r31, Y+5	; 0x05

Which is the code following the case clause in Calculate_and_set() which called Do_57600() in the first place, which would be the natural result of inlining that code as well.

 

This all seems quite right to me.  I expect if you were to trace all of the jump table vectors you'd find nothing untoward.

 

The mystery is why the presence of jump tables results in a 'crash'.  I think they are exposing an unrelated elsewhere bug in your code.  I'd still be looking somewhere else.  Stack overflow, buffer overflow, non-atomic operation, uninitialised automatic, etc.

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Wed. Jan 7, 2015 - 06:24 AM
Top
steve17
steve17's picture
Level: Raving Lunatic
Joined: Tue. Sep 7, 2004
Posts: 6436 View posts
Location: New York State
Posted by steve17: Wed. Jan 7, 2015 - 02:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I do have EIND.  This is a 128a4u but don't let that 128 fool you.  That's app flash.  The bootloader is above that.

 

I'll look some more.  I can probably rule out a few things.  This is startup code with interrupts disabled.  Also the Programmable Interrupt Controller has not been set (no interrupt priorities enabled) so I doubt there are any interrupts.  That should mean "atomic" is not a factor.

 

No stack overflow.  I have oodles of RAM.  In main() I write a pattern in RAM from heap top to stack bottom and the pattern is still there when it goes belly up.  I get the crashes when I don't write the pattern too, in case I am screwing that up.

 

I don't see where uninitialized automatics are a problem.  Actually in this C++ code there are few of those.  In this Set_baud_rate class there are 2 data members that are not initialized in the constructor.  Those are UBBR and scale_factor.  This code is trying to set those.

 

I just confirmed something that I thought I had observed yesterday.  A few minutes ago I stepped through the code, and it worked. The program ran okay.  I haven't gotten it to do that again.  What the hell does that mean?  

Top
keith v
keith v's picture
Level: Hangaround
Joined: Fri. Oct 24, 2014
Posts: 176 View posts
Posted by keith v: Wed. Jan 7, 2015 - 03:35 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

why... WHY,  WHY! are you doing all this in the header???

are you masochistic or is there some reason?

 

Put the code in a C++ file, you're not being clever doing it all in the header.

I'd make you life miserable if you gave me code like that at work.

 

you are running a task switcher;

How solid is it's code?

is it doing something bad when there is only 1 task?
 

Keith Vasilakes

Firmware engineer

Minnesota

Top
clawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99332 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Wed. Jan 7, 2015 - 03:57 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

why... WHY,  WHY! are you doing all this in the header???

are you masochistic or is there some reason?

But this is common in C++. You put the body of the small functions in the class definition. That is in the .h file.

 

(I do agree it makes debugging/following code more difficult but apparently C++ masochists like it that way ;-)

Top
keith v
keith v's picture
Level: Hangaround
Joined: Fri. Oct 24, 2014
Posts: 176 View posts
Posted by keith v: Wed. Jan 7, 2015 - 05:05 PM (Reply to #9)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

why... WHY,  WHY! are you doing all this in the header???

are you masochistic or is there some reason?

But this is common in C++. You put the body of the small functions in the class definition. That is in the .h file.

 

(I do agree it makes debugging/following code more difficult but apparently C++ masochists like it that way ;-)

 

No No No No NO.

This is NOT common in C++, not anywhere I have been.

It's a neophyte technique that subverts so many important design rules I cannot begin to iterate them.

 

Except for trivial getters and setters putting code in headers is a huge antipattern.

There is no good reason to do it, and a lot of good reasons not to do it.

 

You do that on my team and you will be hung by your toes from the rafters, after you fix it of course.

Keith Vasilakes

Firmware engineer

Minnesota

Top
Koshchi
Koshchi's picture
Level: 10k+ Postman
Joined: Wed. Nov 17, 2004
Posts: 15674 View posts
Location: Vancouver, BC
Posted by Koshchi: Wed. Jan 7, 2015 - 08:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But this is common in C++. You put the body of the small functions in the class definition.

None of those functions would I consider "small". For me, if it can't be easily put into one line of code, it doesn't belong in the header.

Regards,
Steve A.

The Board helps those that help themselves.

Top
joeymorin
joeymorin's picture
Level: 10k+ Postman
Joined: Tue. Jul 17, 2012
Posts: 11771 View posts
Location: Location, Location
Posted by joeymorin: Wed. Jan 7, 2015 - 11:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I just confirmed something that I thought I
had observed yesterday. A few minutes ago I
stepped through the code, and it worked. The
program ran okay. I haven't gotten it to do
that again. What the hell does that mean?
To me it still reeks of an uninitialised automatic (and/or a corrupted [not overflowed] stack). Works right after programming but before cycling power? Doesn't work after a power down or maybe also after an EXTRF or WDRF? Almost certainly an uninitialised automatic.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Top
steve17
steve17's picture
Level: Raving Lunatic
Joined: Tue. Sep 7, 2004
Posts: 6436 View posts
Location: New York State
Posted by steve17: Wed. Jan 7, 2015 - 11:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Putting everything in the header makes it easier to write and to read.  Most of my classes don't have a .cpp file.  One file is easier than two no matter how you slice it.  With two, you get a bit of duplication because each function in the .cpp needs a prototype in the .h.  Also each function in the .cpp needs to be scoped.  How that could be better beats me.

 

It makes setting up projects easier too.  Basically only the .cpp files need to be specified and the fewer the better.

 

The only disadvantage that I can see is lots more headers get included in compilation units.  That could slow down builds but mine go pretty quickly.

 

The only time I use a .cpp file is if I need a place to put an ISR or a static class member.  

 

In fact I do have a .cpp for this class because there is another class that might want to call it.

 


// here is the .cpp for Set_baud_rate.  It ticks me off that I need it, but it seems to be the easiest way to give other classes access.

#include "set_baud_rate.h"

Set_baud_rate* Set_baud_rate::set_baud_rate_p;

 

 

Top
steve17
steve17's picture
Level: Raving Lunatic
Joined: Tue. Sep 7, 2004
Posts: 6436 View posts
Location: New York State
Posted by steve17: Thu. Jan 8, 2015 - 12:37 AM (Reply to #12)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:

To me it still reeks of an uninitialised automatic (and/or a corrupted [not overflowed] stack). Works right after programming but before cycling power? Doesn't work after a power down or maybe also after an EXTRF or WDRF? Almost certainly an uninitialised automatic.

I agree it looks like something is not being initialized.  Sometimes it works and sometimes it doesn't.  It doesn't seem to depend on anything except maybe the phase of the moon.  I'll repeatedly try with the debugger and it seems to work about one time out of four.

 

I have a couple of screen shots showing the good and the bad.  When I get to the switch, I use the disassembly thing.  It goes to 00000462 immediately then jumps to 00002c78.  There it loads up the Z register and does a EIJMP.  If 046f is in the Z register, it works.  If not, it jumps to erased memory.  

 

If the Z register is wrong, I can enter the correct value and it takes off running well when it does the EIJMP.