loop/while/for problem

[freakdaddy]

This example code does not print 0-127 as you would expect, it only prints 0. The thread that I've linked to below hints that it may be a 5.4.0 issue.

"With IDE 1.8.4 (avr-gcc 4.9.2) it works as expected. With IDE 1.8.8 (avr-gcc 5.4.0) it fails, prints 0, once. Must be another 5.4.0 bug. Use the 4.9.2 toolchain."

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

void loop() {
  while (1) {
    for (byte i = 0 ; i < 128 ; i++) {
      Serial.println(i);
    }
  }
}

https://forum.arduino.cc/index.php?topic=621017.0

[matthijskooijman]

Interesting case, nice and small as well :-)

I wonder if it is related to signed vs unsigned. What happens if you replace 128 by 127?

[freakdaddy]

It works as expected. Apparently any number (withing the bounds of a byte) works except for 128.

[matthijskooijman]

Ah, I also notice in the thread that it also breaks without the while loop around it? Could you confirm? Always good to further reduce an example like this.

Does the compiler give any warnings (you can enable them in preferences)?

I wonder if the "byte" type and/or i is somehow signed. What happens if you use "unsigned char" or "unsigned int" rather than "byte"?

I just had a look at the https://github.com/arduino/toolchain-avr repo, but it seems there is no newer gcc version available yet to test.

[freakdaddy]

Without the while loop it is ok. So this works...

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

void loop() {
   for (byte i = 0 ; i < 128 ; i++) {
      Serial.println(i);
    }
 }

unsigned int, int work.
unsigned char, byte do not

I enabled compiler warnings and there were no warnings.

[freakdaddy]

This works...

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

byte i; 

void loop() {
  while (1) {
    for (i = 0 ; i < 128 ; i++) {
      Serial.println(i);
    }
  }
}

[matthijskooijman]

Then indeed, probably a gcc bug. Hard to look for in the gcc bugtracker, I'm afraid...

Looking at the disassembler output might also be interesting, perhaps that gives some insight in what's wrong (though I suspect that the problem is in the optimizer / type resolution somewhere entire loop will be optimized away already...).

[freakdaddy]

Can you point me to the gcc bug tracker that I should report this to?

[per1234]

The problem still occurs using avr-gcc 7.3.0-atmel3.6.1-arduino5.

[matthijskooijman]

@per1234, where did you get that? I found https://downloads.arduino.cc/packages/package_avr_3.6.0_index.json in an older toolschain-avr PR, but that only contains version "5.4.0-atmel3.6.1-arduino2" (leading me to believe that Atmel toolchain 3.6.1 contained gcc 5.4.0, but you seem to have a version with 6.3.0?

@freakdaddy, the gcc bugtracker is here: https://gcc.gnu.org/bugzilla/

For the gcc tracker, it might be useful if the example could be further reduced, perhaps even to run without the Arduino IDE and Arduino core, but just from a single .cpp file. That does complicate testing (for lack of serial printing), though assembly inspection might help there (to confirm the code is wrong without actually running it). I did a quick test just now, replacing the serial print with a volatile write, but that seemed to compile just fine.

[per1234]

The Boards Manager JSON URL for the package that provides the staging version of Arduino AVR Boards with avr-gcc 7.3.0-atmel3.6.1-arduino5 has not been advertised publicly before that I've seen, so it might be a secret. I'd need to leave it to someone like @facchinm to pass that information along.

However, there is a publicly advertised Boards Manager JSON URL that gives you Arduino AVR Boards with avr-gcc 7.3.0-atmel3.6.1-arduino4:
http://downloads.arduino.cc/packages/package_avr_7.3.0_index.json

[matthijskooijman]

I did one more test, if I compile this sketch:

void setup() { }

volatile byte x;

void loop() {
  while (1) {
    for (byte i = 0 ; i < 128 ; i++) {
      x = i;
    }
  }
}

With 5.4.0, I get the following assembly for the main function:

$ avr-objdump --demangle -d sketch_jun13a.ino.elf
(other code removed)
00000124 <main>:
 124:   78 94           sei
 126:   84 b5           in      r24, 0x24       ; 36
 128:   82 60           ori     r24, 0x02       ; 2
 12a:   84 bd           out     0x24, r24       ; 36
 12c:   84 b5           in      r24, 0x24       ; 36
 12e:   81 60           ori     r24, 0x01       ; 1
 130:   84 bd           out     0x24, r24       ; 36
 132:   85 b5           in      r24, 0x25       ; 37
 134:   82 60           ori     r24, 0x02       ; 2
 136:   85 bd           out     0x25, r24       ; 37
 138:   85 b5           in      r24, 0x25       ; 37
 13a:   81 60           ori     r24, 0x01       ; 1
 13c:   85 bd           out     0x25, r24       ; 37
 13e:   ee e6           ldi     r30, 0x6E       ; 110
 140:   f0 e0           ldi     r31, 0x00       ; 0
 142:   80 81           ld      r24, Z
 144:   81 60           ori     r24, 0x01       ; 1
 146:   80 83           st      Z, r24
 148:   e1 e8           ldi     r30, 0x81       ; 129
 14a:   f0 e0           ldi     r31, 0x00       ; 0
 14c:   10 82           st      Z, r1
 14e:   80 81           ld      r24, Z
 150:   82 60           ori     r24, 0x02       ; 2
 152:   80 83           st      Z, r24
 154:   80 81           ld      r24, Z
 156:   81 60           ori     r24, 0x01       ; 1
 158:   80 83           st      Z, r24
 15a:   e0 e8           ldi     r30, 0x80       ; 128
 15c:   f0 e0           ldi     r31, 0x00       ; 0
 15e:   80 81           ld      r24, Z
 160:   81 60           ori     r24, 0x01       ; 1
 162:   80 83           st      Z, r24
 164:   e1 eb           ldi     r30, 0xB1       ; 177
 166:   f0 e0           ldi     r31, 0x00       ; 0
 168:   80 81           ld      r24, Z
 16a:   84 60           ori     r24, 0x04       ; 4
 16c:   80 83           st      Z, r24
 16e:   e0 eb           ldi     r30, 0xB0       ; 176
 170:   f0 e0           ldi     r31, 0x00       ; 0
 172:   80 81           ld      r24, Z
 174:   81 60           ori     r24, 0x01       ; 1
 176:   80 83           st      Z, r24
 178:   ea e7           ldi     r30, 0x7A       ; 122
 17a:   f0 e0           ldi     r31, 0x00       ; 0
 17c:   80 81           ld      r24, Z
 17e:   84 60           ori     r24, 0x04       ; 4
 180:   80 83           st      Z, r24
 182:   80 81           ld      r24, Z
 184:   82 60           ori     r24, 0x02       ; 2
 186:   80 83           st      Z, r24
 188:   80 81           ld      r24, Z
 18a:   8e 7f           andi    r24, 0xFE       ; 254
 18c:   80 83           st      Z, r24
 18e:   80 81           ld      r24, Z
 190:   80 68           ori     r24, 0x80       ; 128
 192:   80 83           st      Z, r24
 194:   10 92 c1 00     sts     0x00C1, r1      ; 0x8000c1 <__TEXT_REGION_LENGTH__+0x7e00c1>
 198:   10 92 00 01     sts     0x0100, r1      ; 0x800100 <_edata>

0000019c <_exit>:
 19c:   f8 94           cli

0000019e <__stop_program>:
 19e:   ff cf           rjmp    .-2             ; 0x19e <__stop_program>

The main function does not contain any jumps, so the infinite loop has somehow disappeared. If I put just loop into a separate .cpp file (and rename to main()), the generated code looks ok, so I suspect that there is something in the nested function calls, nested infinite loops (Arduino's main() also has one), archive-based compilation process, separation in compilation units, LTO, or a combination of these that triggers this problem.

[matthijskooijman]

One more test shows it's not the Arduino main function that's involved, so probably the way things are compiled. If I compile the following code:

volatile byte x;

int main() {
  while (1) {
    for (byte i = 0 ; i < 128 ; i++) {
      x = i;
    }
  }
}

I get the following main function:

00000090 <main>:
  90:   10 92 00 01     sts     0x0100, r1      ; 0x800100 <_edata>

00000094 <_exit>:
  94:   f8 94           cli

00000096 <__stop_program>:
  96:   ff cf           rjmp    .-2             ; 0x96 <__stop_program>

So this is exactly your problem: the loop only runs once with a value of 0 (which is in r=1 and stored to the global variable x at adress 0x0100).

Note that IIRC the Arduino main() function is declared weak, so adding it to your sketch replaces Arduino's version with your own.

[matthijskooijman]

I managed to reduce this even further and reproduce it without involving the Arduino IDE or code, using avr-gcc from Ubuntu Disco:

$ cat main.cpp
volatile unsigned char x;

int main() {
  while (1) {
    for (unsigned char i = 0 ; i < 128 ; i++) {
      x = i;
    }
  }
}
$ avr-g++ -Os -mmcu=atmega328p main.cpp -o main.elf
$ avr-objdump --demangle -d main.elf
(other code omitted)
00000090 <main>:
  90:   10 92 00 01     sts     0x0100, r1      ; 0x800100 <_edata>

00000094 <_exit>:
  94:   f8 94           cli

00000096 <__stop_program>:
  96:   ff cf           rjmp    .-2             ; 0x96 <__stop_program>
$ avr-g++ --version
avr-g++ (GCC) 5.4.0

I could not reduce the compiler commandline further than this (though I could swap out -Os for -O2, atmega328p for atmega2560 and avr-g++ for avr-gcc with the same result).

And as before, replacing 128 by 127 fixes the loop. I also saw that using 129 fixes things. From other tests I noticed that a < 128 test is compiled into an and r24 r24 instruction, which happens to set the sign bit for all values >= 128, so perhaps some optimization pass that misinterprets this check later?

I'm pretty sure this is a compiler bug, since even if there would be some kind of signed vs unsigned or integer promototion thing we're missing here, that should really not cause the outer infinite loop to disappear (unless our program invokes undefined behaviour, but then the compiler might need to be kinder and tell us :-p).

I think this would be more than concise enough to report a gcc bug with. @freakdaddy, would you file that report? I already tried searching for existing reports a bit, but perhaps you can dig a bit deeper (though it's hard to search for something like this, of course). Be sure to also look at closed bugs, since it might be fixed already.

Also, it would be good to confirm on the newer gcc version that @per1234 linked above.

[facchinm]

@per1234 @matthijskooijman avr-gcc-7.3.0-atmel3.6.1-arduino5 has been released with megaAVR core 1.8.1, links are here 😉

http://downloads.arduino.cc/tools/avr-gcc-7.3.0-atmel3.6.1-arduino5-arm-linux-gnueabihf.tar.bz2
http://downloads.arduino.cc/tools/avr-gcc-7.3.0-atmel3.6.1-arduino5-aarch64-pc-linux-gnu.tar.bz2
http://downloads.arduino.cc/tools/avr-gcc-7.3.0-atmel3.6.1-arduino5-x86_64-apple-darwin14.tar.bz2
http://downloads.arduino.cc/tools/avr-gcc-7.3.0-atmel3.6.1-arduino5-i686-pc-linux-gnu.tar.bz2
http://downloads.arduino.cc/tools/avr-gcc-7.3.0-atmel3.6.1-arduino5-x86_64-pc-linux-gnu.tar.bz2
http://downloads.arduino.cc/tools/avr-gcc-7.3.0-atmel3.6.1-arduino5-i686-w64-mingw32.zip

Hope it helps but this really looks like an optimization problem on GCC side that we can't avoid with any flag combination (maybe just -O1 but it would hurt flash occupation "a bit" 😄 ). So I think reporting to mainline and waiting for a fix could be the only thing to do.

[darrylp1per]

Using my own compiled GCC 4.8.5, everything works ( code compilation wise ).

snippet of source used....

volatile byte x;

void loop() {
  // put your main code here, to run repeatedly: 
  for (byte i = 0 ; i < 128 ; i++) {
      x = i;
    }
}

produces this asm code.

000000f2 <loop>:
  f2:	80 e0       	ldi	r24, 0x00	; 0
  f4:	87 fd       	sbrc	r24, 7
  f6:	04 c0       	rjmp	.+8      	; 0x100 <main+0x40>
  f8:	80 93 00 01 	sts	0x0100, r24	; 0x800100 <_edata>
  fc:	8f 5f       	subi	r24, 0xFF	; 255
  fe:	fa cf       	rjmp	.-12     	; 0xf4 <main+0x34>
 100:	80 e0       	ldi	r24, 0x00	; 0
 102:	fa cf       	rjmp	.-12     	; 0xf8 <main+0x38>

00000104 <_exit>:
 104:	f8 94       	cli

00000106 <__stop_program>:
 106:	ff cf       	rjmp	.-2      	; 0x106 <__stop_program>

the jump back ( 102 ) is the main loop repeating. the test and exit is done at f6.