Compiler bug - loops that end with 65535 (0xFFFF) do not compile correctly

Development Suggestions for the Arduino Project
1

Hello all!

I have stumbled upon a code construct that the Arduino compiler mis-compiles.

Loops that use an unsigned integer as the counter variable do not compile correctly if the loop's exit condition is "<=65535U" or "<=0xFFFFU". The object code / assembly language that is produced is wrong - it never checks for the ending condition of the loop, so the loop ends up being an infinite loop. Here are two examples of loops that cause this bug to manifest...

unsigned int ui = 16384U;
do { whatever } while (ui <= 65535U);

unsigned int ui;
for (ui = 0x6000U; ui <= 0xFFFFU; ui++) { whatever }

The object code for the 'for' loop above that is produced by the Arduino software is as follows...

126: e0 e0 ldi r30, 0x00 ; 0
128: f0 e6 ldi r31, 0x60 ; 96
12c: whatever
12e: fe cf rjmp .-4 ; 0x12c <setup+0x6>

Note that the counter variable (in registers 30 & 31) is never compared to anything, and there is no conditional branch that operates on the result of the comparison. Instead, there is a unconditional jump back to the top of the loop, causing it to run forever.

The attached sketch demonstrates this. The "Serial.println()" before the loop gets invoked, but the "Serial.println()" after the loop never does. A quick look at the assembly code produced by the compiler shows why - the loop never terminates, and the counter variable "ui" keeps incrementing and then wrapping forever.

I have tested this on a Mega 2560 and on a Duemilanove 328, using the 1.0.1 version and the 1.0.3 version of the Arduino software, on two different computers.

Note that if you change the loop's ending condition to "<=65534", or "<65535", or "<= 60000", etc, then the compiler produces the correct object code and the program runs as expected. There is something special about taking the counter variable all the way up to the biggest number that fits in a 16-bit unsigned int (65535).

This situation seems so odd to me that I thought I would post it and see if I could get some independent confirmation before I submitted a bug report.

Thanks!

kc-science

CompilerBug.ino (278 Bytes)

2

Loops that use an unsigned integer as the counter variable do not compile correctly if the loop's exit condition is "<=65535U" or "<=0xFFFFU". The object code / assembly language that is produced is wrong - it never checks for the ending condition of the loop, so the loop ends up being an infinite loop.

That's correct. It' an optimization. An unsigned int is ALWAYS less than or equal to 65535, so there is no reason to put the check in the code.

3

westfw is right. Since the condition cannot be met, the bug is in your code.

It would be like comparing an unsigned int to a negative number. The compiler could well assume that condition would never be met as well.

In fact I'm right, strangely enough.

This sketch:

void setup ()
  {
  Serial.begin (115200);

  for (unsigned int i = 0; i >= 0; i--)
    Serial.println (42);
  }  // end of setup

void loop () {}

Generates this (after doing the Serial.begin):

  d0:	88 e9       	ldi	r24, 0x98	; 152
  d2:	91 e0       	ldi	r25, 0x01	; 1
  d4:	6a e2       	ldi	r22, 0x2A	; 42
  d6:	70 e0       	ldi	r23, 0x00	; 0
  d8:	4a e0       	ldi	r20, 0x0A	; 10
  da:	50 e0       	ldi	r21, 0x00	; 0
  dc:	0e 94 9b 03 	call	0x736	; 0x736 <_ZN5Print7printlnEii>
  e0:	f7 cf       	rjmp	.-18     	; 0xd0 <setup+0x10>

Notice how the test has been optimized away, and it just does a Serial.println of 42, and then loops back and does it again?

So it's not at all a compiler bug. The compiler is doing what you told it to do, as efficiently as it can.

4

I stand corrected.

Thank you Nick and westfw for your excellent replies. Unsigned 16-bit integers greater than 0xFFFF are indeed extremely rare and elusive creatures. I wrote a "while(true)" loop without realizing it!

The reason I wrote that ridiculous loop in the first place is to test an SRAM chip I have hooked to my Arduino. My original loop was as follows. I figured I would have the ending condition watch for the wrap to zero - but THAT compiled to an infinite loop...

unsigned char *ptr;
for (ptr = (unsigned char *)0x6000U; ptr != (unsigned char *)0U; ptr++)
*ptr = (unsigned char) 0xFF;

Why would this not terminate? (Actually, in practice, it DOES terminate, rather spectacularly, when the pointer wraps and the register file gets overwritten with 0xFF, causing the Atmel to reboot - a method of breaking out of a loop which did NOT make it into the ANSI Standard.)

Just for the heck of it, I just compiled the following, and this does NOT compile into an infinite loop, even though I would expect this to produce the same object code as the loop above...

unsigned int ui;
for (ui = 0x6000U; ui != 0U; ui++)
*((unsigned char *)ui) = 0xFF;

Am I missing something else here?

Thanks again!

kc-science

5 
for (ui = 0x6000U; ui != 0U; ui++)

You don't need to qualify zero. Zero is zero, that looks like you are saying "ow"!

In fact you never need to qualify literals, unless you want to promote them in an expression.

eg.

long a = 1000 * 1000;

That would assume 1000 (both of them) are int, and produce unexpected results in the multiplication. So in this case you need:

long a = 1000L * 1000;

However:

long a = 1000000;

... is perfectly valid. The compiler gets what you want, you don't need to thump it over the head.

Proof:

void setup ()
  {
  Serial.begin (115200);

  long a = 1000 * 1000;
  long b = 1000L * 1000;
  long c = 1000000;
  
  Serial.println (a);
  Serial.println (b);
  Serial.println (c);
  }  // end of setup

void loop () { }

Output:

16960
1000000
1000000

unsigned int ui;
for (ui = 0x6000U; ui != 0U; ui++)
  *((unsigned char *)ui) = 0xFF;

That will terminate. ui will wrap, and reach zero.

6

Dear....

Is there a simple soltion for this to put someting in a srcipt??
Where do i need to search in to the code??

// These two functions are used to read and write long variables from and to eeprom
//

long readlong(int address)
{
    // use long address (groups of 4)
    address+=20;
    address *= 4;
    return eeprom_read_dword((uint32_t*) address);
}

void writelong(int address, long value)
{
    address+=20;
    address *= 4;
    eeprom_write_dword((uint32_t*) address, value);

I found this....

Or where do i have to search for..

Thanks

And sorry bricklayer here