bug: transmits 08F5 which is not content of buffer

[deanqx]

#include <util/delay.h>

int main(void) {
  uart0_init(BAUD_CALC(9600UL));
  while (1) {
    uart0_putint(1023);
    uart0_puts("\r\n");
    _delay_ms(1000);
  }
}

Using -Os -std=gnu99 to compile.

cat /dev/ttyUSB0 | xxd
00000000: 3130 3233 08f5 0a0a 3130 3233 08f5 0a0a  1023....1023....
00000010: 3130 3233 08f5 0a0a 3130 3233 08f5 0a0a  1023....1023....
00000020: 3130 3233 08f5 0a0a 3130 3233 08f5 0a0a  1023....1023....

Original uart0_putint function from the library (comment added):

void uart0_putint(int16_t data)
{
#ifndef USART_NO_LOCAL_BUFFERS
	char u_tmp_buff[7]; // heading, 5 digit bytes, NULL
#endif

	itoa(data, u_tmp_buff, 10);
	uart0_putstr(u_tmp_buff);
	// asm volatile("nop");
}

Adding the nop command fixes this bug.

asm volatile("nop");

$ cat /dev/ttyUSB0 | xxd
00000000: 3130 3233 0a0a 3130 3233 0a0a 3130 3233  1023..1023..1023
00000010: 0a0a 3130 3233 0a0a 3130 3233 0a0a 3130  ..1023..1023..10

I don't think anyone will review this. In case someone does: I can provide concrete compile commands and more details.

[jnk0le]

If you define USART_NO_ABI_BREAKING_PREMATURES in config file. Does it solve the issue?

[deanqx]

I have tried that but to be sure I tried it once now again. I don't get a serial message at all for some reason if define USART_NO_ABI_BREAKING_PREMATURES. I could send you my project files with a main function prepared to just test this issue but it works with the fix so it is no major issue for me.

[jnk0le]

If you turn that workaround into asm volatile(""); or atomic_signal_fence() (from c11 standard), does the issue return?

Could you also send the assembly output of the offending binary? It was usually in .lss files IIRC

[deanqx]

How to reproduce

main.c

#include "usart.h"
#include "util/delay.h"

int main(void) {
  uart0_init(BAUD_CALC(9600UL));

  while (1) {
    uart0_putuint(1023);
    uart0_puts("\r\n");

    _delay_ms(1000);
  }
}

Makefile

build:
	avr-gcc -DF_CPU=16000000UL -mmcu=atmega328p -Os --std=gnu99 usart.c main.c -o a.elf

build_asm:
	avr-gcc -DF_CPU=16000000UL -mmcu=atmega328p -Os --std=gnu99 usart.c main.c -S

upload:
	avr-objcopy -O ihex a.elf a.hex
	avrdude -p atmega328p -P /dev/ttyUSB0 -c arduino -U flash:w:a.hex

Using this simplified Makefile to build now. Upload to Arduino with:

make build upload

Receiving serial data with both:

minicom -D /dev/ttyUSB0 -b 9600
cat /dev/ttyUSB0 | xxd

Workaround

asm volatile("");

Adding this works too. I use C99.

Assembly

Compiled with:

make build_asm

The following assembly uses the workaround asm volatile(""); placed in the code as shown in the first message:

main.s

	.file	"main.c"
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__SREG__ = 0x3f
__tmp_reg__ = 0
__zero_reg__ = 1
	.text
	.section	.rodata.str1.1,"aMS",@progbits,1
.LC0:
	.string	"\r\n"
	.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,lo8(103)
	sts 196,r24
	sts 197,__zero_reg__
	ldi r24,lo8(-104)
	sts 193,r24
.L2:
	ldi r24,lo8(-1)
	ldi r25,lo8(3)
	call uart0_putuint
	ldi r24,lo8(.LC0)
	ldi r25,hi8(.LC0)
	call uart0_putstr
	ldi r18,lo8(3199999)
	ldi r24,hi8(3199999)
	ldi r25,hlo8(3199999)
1:	subi r18,1
	sbci r24,0
	sbci r25,0
	brne 1b
	rjmp .
	nop
	rjmp .L2
	.size	main, .-main
	.ident	"GCC: (GNU) 15.1.0"
.global __do_copy_data

Original and not working
snippet of usart.s

uart0_putuint:
	push r28
	push r29
	rcall .
	rcall .
	rcall .
	push __tmp_reg__
	in r28,__SP_L__
	in r29,__SP_H__
/* prologue: function */
/* frame size = 7 */
/* stack size = 9 */
.L__stack_usage = 9
	ldi r20,lo8(10)
	movw r22,r28
	subi r22,-1
	sbci r23,-1
	call __utoa_ncheck
	movw r24,r28
	adiw r24,1
/* epilogue start */
	adiw r28,7
	in __tmp_reg__,__SREG__
	cli
	out __SP_H__,r29
	out __SREG__,__tmp_reg__
	out __SP_L__,r28
	pop r29
	pop r28
	jmp uart0_putstr
	.size	uart0_putuint, .-uart0_putuint

With workaround
snippet of usart.s

uart0_putuint:
	push r28
	push r29
	rcall .
	rcall .
	rcall .
	push __tmp_reg__
	in r28,__SP_L__
	in r29,__SP_H__
/* prologue: function */
/* frame size = 7 */
/* stack size = 9 */
.L__stack_usage = 9
	ldi r20,lo8(10)
	movw r22,r28
	subi r22,-1
	sbci r23,-1
	call __utoa_ncheck
	movw r24,r28
	adiw r24,1
	call uart0_putstr
/* epilogue start */
	adiw r28,7
	in __tmp_reg__,__SREG__
	cli
	out __SP_H__,r29
	out __SREG__,__tmp_reg__
	out __SP_L__,r28
	pop r29
	pop r28
	ret
	.size	uart0_putuint, .-uart0_putuint

[jnk0le]

So, the gcc decided to tail call into putstr after deallocating stackframe, which holds the temporary buffer. What could possibly go wrong.
GCC on godbold, of course, doesn't want to reproduce. But it looks like a compiler bug. You can try a different version of gcc (newer or older)

Calling into utoa_ncheck instead of itoa_ncheck is sus btw. Does it print negative numbers as it should?

[deanqx]

But it looks like a compiler bug

Maybe AVR-GCCs optimizer is confused by the assembly code in C. Or AVR introduced a bug in GCC when they ported it because I don't think GCC has a bug. It's definitely weird.

Calling into utoa_ncheck instead of itoa_ncheck is sus btw. Does it print negative numbers as it should?

it is the uint function so that's correct

[jnk0le]

inline asm does consume pointer through input operands so it shouldn't get confused.
With USART_NO_ABI_BREAKING_PREMATURES there should be no assembly anyway (except interrupt handlers)

Calling into utoa_ncheck instead of itoa_ncheck is sus btw. Does it print negative numbers as it should?

it is the uint function so that's correct

ok, didn't spot it uses different function than first post.

[deanqx]

USART_NO_ABI_BREAKING_PREMATURES did not work for me when I opened this issue. I won't look into it for now.