ArduinoCore-avr
ArduinoCore-avr copied to clipboard
arduino
loop/while/for problem
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
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?
It works as expected. Apparently any number (withing the bounds of a byte) works except for 128.
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.
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.
This works...
void setup() {
Serial.begin(115200);
}
byte i;
void loop() {
while (1) {
for (i = 0 ; i < 128 ; i++) {
Serial.println(i);
}
}
}
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...).
@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.
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
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.
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.
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.
@per1234 @matthijskooijman avr-gcc-7.3.0-atmel3.6.1-arduino5 has been released with megaAVR core 1.8.1, links are here :wink:
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" :smile: ). So I think reporting to mainline and waiting for a fix could be the only thing to do.
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.
Really didn't feel qualified to report the gcc bug but I did and provided a link back to here. Thanks for everyone's work on this!
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90886
Thanks for reporting. Am I seeing correctly that you can also reproduce this on x86/amd64 rather than just on AVR?
okay, have built in linux had to get a clean built avr-gcc 4.8.5 ( which is same as my normal windows enviroment for arduino building.)
gcc-4.8.5 gives this...
.file "test.c"
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__SREG__ = 0x3f
__tmp_reg__ = 0
__zero_reg__ = 1
.section .text.startup,"ax",@progbits
.global main
.type main, @function
main:
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
ldi r24,0
.L2:
sts x,r24
subi r24,lo8(-(1))
cpi r24,lo8(-128)
brne .L2
ldi r24,0
ldi r25,0
ret
.size main, .-main
.comm x,1,1
.ident "GCC: (GNU) 4.8.5"
.global __do_clear_bss
and then under avr-gcc-9.1.0 gives this....
.file "test.c"
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__SREG__ = 0x3f
__tmp_reg__ = 0
__zero_reg__ = 1
.text
.section .text.startup,"ax",@progbits
.global main
.type main, @function
main:
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
ldi r24,0
.L2:
sts x,r24
subi r24,lo8(-(1))
cpi r24,lo8(-128)
brne .L2
ldi r25,0
ldi r24,0
/* epilogue start */
ret
.size main, .-main
.comm x,1,1
.ident "GCC: (GNU) 9.1.0"
so that would imply a avr-gcc-9.1 for windows might work
As a work-around for affected versions, -fwrapv seems to work. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90886#c11
