Hallo Kollegen,
ich habe in der main zwei variablen:
uint8_t timer0 = 0;
uint8_t timer1 = 0;
dort beschreibe ich in einer state machine diese variablen mit
uart-charactern und baue mir daraus einen uint16:
case ST_timer0:
timer0 = uart_getc();
next_state = ST_timer1;
break;
case ST_timer1:
timer1 = uart_getc();
next_state = ST_checksum;
break;
case ST_checksum:
if (uart_checksum_recv()) { /* checksum successful */
timer_count = orig_timer_count = (((uint16_t)timer1)<<8) | timer0;
timer_init();
sei();
leider geht der timer1 bei der shift operation verloren (timer1<<8
ergibt 1). wenn ich den source neu übersetze ohne die shift operation
kommt in timer1 und in timer0 der richtige wert an. so. wenn ich vor die
variablen volatile schreibe dann tut es. aber hier steht dass man
volatile nur braucht wenn ich per ISR und main gleichzeitig auf einer
resource operiere. das mache ich aber nicht. den uart bediene ich durch
pollen.
die funktion uart_getc() sieht so aus:
char uart_getc()
{
loop_until_bit_is_set( UCSR0A, RXC0 );
const char ch = UDR0;
uart_checksum_update(ch);
return ch;
}
der ganze source liegt unter:
http://github.com/ndim/freemcan/tree/master/firmware/
ich übersetze mit:
avr-gcc -c -mmcu=atmega644 -I. -fshort-enums -gstabs -I../include -Os
-Wall -Wextra -Wstrict-prototypes -std=gnu99 main.c -o main.o
hat jemand eine idee???
Viele Grüße & Danke im Voraus
Käfer schrieb: > hat jemand eine idee??? Nope. Keine Erklärung Hast du dir schon mal den Assembler Code rund um diese Stelle angesehen? Hast du versuchsweise die beiden Variablen aus dem main() raus zu globalen Vairablen gemacht? Wenns dann geht, wäre das ein Hinweis darauf, dass unter Umständen irgendwo ein Bufferoverflow die beiden Variablen auf dem Stack niedergebügelt hat. Wie stellst du fest, dass da was verloren geht?
>Hast du versuchsweise die beiden Variablen aus dem main() raus zu >globalen Vairablen gemacht? Wenns dann geht, wäre das ein Hinweis >darauf, dass unter Umständen irgendwo ein Bufferoverflow die beiden >Variablen auf dem Stack niedergebügelt hat. Ich würde einfach mal die Optimierung ausschalten. Wahrscheinlich jagd da wieder jemand Geistern mit dem Debugger hinterher;)
holger schrieb: >> const char ch = UDR0; > > Was soll denn der Quatsch mit dem const? Wieso Quatsch? ch wird im weiteren nicht verändert, also ist nix dagegen einzuwenden.
als globale static funktionierts auch. den stackbedarf haben wir schon einmal gemessen und überprüfen dies während dem make CHECK: MAX_BSS_END=0x1100 >= BSS_END=0xd3e CHECK: STACK_START=0x10ff >= 0xd6e = (BSS_END=0xd3e + MIN_STACK_SIZE=0x30) sollte eigentlich i.O. sein. ramschmierer - hmm. compiler optimierung ausschalten hilft aber auch. das blöde ist ich habe keinen debugger. d.h. ich muss mich darauf verlassen ob die anwendung das tut was sie soll. ob im timer_count das richtige drinsteht oder nicht merkt man sofort bei der anwenung
siehe oben. optimierung ausschalten löst auch das problem. was mach ich denn nun? ist der avr-gcc denn bekannt dafür dass er nicht so gut mit optimierungen umgehen kann?
>siehe oben. optimierung ausschalten löst auch das problem. >was mach ich denn nun? ist der avr-gcc denn bekannt dafür dass er nicht >so gut mit optimierungen umgehen kann? Welche Version hast du? Gibt da gerade einen Bug http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45052
avr-gcc-4.4.2-2.fc12.x86_64. der bug sollte nicht das problem sein da mein gcc das volatile ja berücksichtigt. das blöde ist halt dass es egal ist was ich tue, sobald nur irgend was sich im assemblerfile ändert geht es plötzlich (ob es jetzt -fshort-enums oder -Os ist). das schliesst einen ramschmierer halt nicht aus. die idee das assembler file zu interpretieren ist nicht so doof, nur bin ich zu doof dafür ;-) muss mal drüber schlafen.
Käfer schrieb: > leider geht der timer1 bei der shift operation verloren Wie stellst du das fest? Stimmt das Ergebnis in timer_count und orig_timer_count nicht oder ändert sich timer1? Außerdem, mein Bauchgefühl sagt mir grad, dass solche Doppelzuweisungen nicht eindeutig definiert sind (sicher bin ich mir da aber nicht): timer_count = orig_timer_count = (((uint16_t)timer1)<<8) | timer0; trenn das mal irgendwie so: orig_timer_count = (((uint16_t)timer1)<<8) | timer0; timer_count = orig_timer_count;
das auftrennen der doppelzuweisung auf zwei zeilen habe ich schon versucht. geht auch nicht. orig_timer_count = (((uint16_t)timer1)<<8) | timer0; timer_count = orig_timer_count; -> die ANWENDUNG VERHÄLT SICH SO als ob immer 000000001 bin im oberen byte von timer_count stehen würde (egal was übers uart kommt), das untere byte entspricht timer0 muss mal gerade überlegen: /*snip*/ timer_count = orig_timer_count = timer1; -> es steht das gesendete byte von timer1 in timer_count (über uart muss timer1 also richtig kommen) /*snip*/ timer_count = orig_timer_count = timer0; -> es steht das gesendete byte von timer0 in timer_count /*snip*/ timer_count = orig_timer_count = (((uint16_t)timer1)<<8) -> ich glaube das hat auch nicht getan -> shift operation? /*snip*/ was aber komischer weise funktioniert ist anstatt mit dem uart_getch harte zahlen auf timer1 und 0 im code zu schreiben. -> das wiederlegt die these von der kaputten shiftoperation (du hast recht, ich habe mich missverständlich ausgedrückt ;-). deswegen bin ich auch auf das volatile gekommen. /*snip*/ timer1 und 0 als 16 bit (anstatt auf 8 bit variablen) und dafür beim shiften nicht mehr auf 16 bit zu casten geht auch nicht. /*snip*/ so muss mal überlegen - achja ein typecast auf uint8_t beim schreiben auf timer1 und 0 mit getchar hab ich auch versucht. das ändert auch nichts (die assembler files sind sogar identisch). harrr
Käfer schrieb: > das auftrennen der doppelzuweisung auf zwei zeilen habe ich schon > versucht. geht auch nicht. Ist egal. Das ist eindeutig und sauber definiert. Deine Analysen sind interessant. Aber im Grunde gilt: Was du da an dieser Stelle im Code hast ist alles sauber.
Ich glaub du hast ganz andere Probleme. Atomarer Zugriff zum Beispiel.
timer_count wird in einem Interrupt runtergezählt
timer_count--;
if (timer_count == 0) {
Nur so nebenbei: Ist ein Unterlauf von timer_count erlaubt?
timer_count = orig_timer_count;
Kann ohne atomaren Zugriff in die Hose gehen!
Was ist wenn dabei der Timer Interrupt zuschlägt?
atomarer zugriff ist ein gutes stichwort aber auch da haben wir sehr viel gehirnschmalz reingesteckt: der rechner kommt aus einem kalt oder warmstart. holt sich timer0, dann timer1 geht dann in in den zustand ST_checksum und dort steht das statement timer_count = ... usw. erst dann wird der timer initialisert und erst dann kommt der sei(). d.h. zu diesem zeitpunkt sind alle interrupts noch gesperrt. der zustand kann erst wieder durch softreset oder kaltstart angesprungen werden. wenn der timer counter überläuft ist das egal da das timer_flag im interrupt service selber geschrieben wird. das ist die einzige steuervariable des systems. ist der timer abgelaufen (das flag gesetzt) ist alles vorbei und man kommt nur noch durch einen reset heraus. das timer_flag ist ein uint8 - die entsprechenden if - statements müssten alle atomar sein. im code gibt es nur eine einzige stelle die nicht atomar ist (das ist aber absicht) aber die wird nirgends angesprungen, ausser wenn nicht ganz ausdrücklich vom user angefordert und auch da werden nur dann daten ans uart verschickt und nicht an den systemvariablen rumgeschraubt. also ich bin relativ ratlos gerade. wenn ich den code zurückbaue verschwindet der fehler auch.
Lass Dir den tatsächliche Wert von timer_count doch einfach mal über die UART ausgeben. Ansonsten, "avr-gcc-4.4.2-2.fc12.x86_64" sieht verdammt nach einer Default-Toolchain von Fedora aus. Wenn das tatsächlich so ist --> gaaaaaaanz schlechte Idee! Dem Teil fehlen mit ziemlicher Sicherheit etliche Patches. Da hilft nur selberbauen, siehe: http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=42631&start=all Ist afaik die einzige Quelle wie man zu einer brauchbaren Toolchain unter Linux kommt. Siehe auch diverse weitere Threads hier: http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewforum&f=2 z.B.: http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=94344&highlight=linux http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=95328&start=all&postdays=0&postorder=asc
habe vor dem sei() mal folgendes eingefügt: loop_until_bit_is_set(UCSR0A, UDRE0); UDR0 = (char)(timer_count>>8); loop_until_bit_is_set(UCSR0A, UDRE0); UDR0 = (char)(timer0); loop_until_bit_is_set(UCSR0A, UDRE0); UDR0 = (char)(timer_count&0xff); loop_until_bit_is_set(UCSR0A, UDRE0); UDR0 = (char)(timer1); Sending 'm' command to device (param=10=0x000a) Received 0x0006=6 bytes of layer 1 data 0000 01 0a 0a 00 46 4d ....FM ->01 oberes byte timer_count (falsch) ->0a timer0 (richtig) ->0a unteres byte timer_count (richtig) ->00 timer1 (richtig) bestätigt damit obige vermutungen die punkte mit dem avrgcc selberbau werde ich mal im hinterkopf behalten. vorerst mache ich ohne optimierungen mal weiter. das löst temporär ja auch das problem. ein kleines bisschen schlechter beigeschmack bleibt halt. in diesem sinne - danke nochmal für das zahlreiche feedback
Hallo,
sieht nach compilerbug aus (sowohl gcc 4.5.0 als auch 4.4.2). Ich habe
meinen spezialisten-kollegen gefragt, der hat sich mal den assembler
angeschaut.
Anbei der codesnippet:
case ST_timer0:
timer0 = uart_getc();
3be: 0e 94 c6 02 call 0x58c ; 0x58c <uart_getc>
3c2: f8 2e mov r15, r24
---> ergebnis der uart_getc() ist in r24
next_state = ST_timer1;
break;
case ST_timer1:
timer1 = uart_getc();
3c4: 0e 94 c6 02 call 0x58c ; 0x58c <uart_getc>
---> ergebnis der uart_getc() ist in r24 wird aber nicht gesichert!
next_state = ST_checksum;
break;
case ST_checksum:
if (uart_checksum_recv()) { /* checksum successful */
3c8: 0e 94 d3 02 call 0x5a6 ; 0x5a6 <uart_checksum_recv>
---> aufruf von uart_checksum_recv
3cc: 88 23 and r24, r24
3ce: 61 f1 breq .+88 ; 0x428 <main+0x14a>
/* set up timer with the value we just got */
timer_count = orig_timer_count = (((uint16_t)timer1)*256) | timer0;
3d0: 80 e0 ldi r24, 0x00 ; 0
3d2: ec 01 movw r28, r24
3d4: 2f 2d mov r18, r15
3d6: 30 e0 ldi r19, 0x00 ; 0
3d8: 2c 2b or r18, r28
3da: 3d 2b or r19, r29
3dc: 30 93 3a 01 sts 0x013A, r19
3e0: 20 93 39 01 sts 0x0139, r18
3e4: 20 91 39 01 lds r18, 0x0139
3e8: 30 91 3a 01 lds r19, 0x013A
3ec: 30 93 3d 0d sts 0x0D3D, r19
3f0: 20 93 3c 0d sts 0x0D3C, r18
char uart_checksum_recv(void)
{
5a6: 1f 93 push r17
const uint8_t v = checksum_accu & 0xff;
5a8: 10 91 37 01 lds r17, 0x0137
const uint8_t c = uart_getc();
---> uart_getc überschreibt r24 an dieser stelle
5ac: 0e 94 c6 02 call 0x58c ; 0x58c <uart_getc>
return (v == c);
5b0: 91 e0 ldi r25, 0x01 ; 1
5b2: 18 13 cpse r17, r24
5b4: 90 e0 ldi r25, 0x00 ; 0
}
5b6: 89 2f mov r24, r25
5b8: 1f 91 pop r17
5ba: 08 95 ret
Mit WinAVR kommt etwa folgendes raus:
1 | avr-gcc -mmcu=atmega644 -Wall -gdwarf-2 -std=gnu99 -DHAVE_UPRINTF_IMPLEMENTATION -DF_CPU=16000000UL -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -MD -MP -MT main.o -MF dep/main.o.d -c ../main.c |
2 | avr-gcc -mmcu=atmega644 -Wall -gdwarf-2 -std=gnu99 -DHAVE_UPRINTF_IMPLEMENTATION -DF_CPU=16000000UL -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -MD -MP -MT uart-printf.o -MF dep/uart-printf.o.d -c ../uart-printf.c |
3 | avr-gcc -mmcu=atmega644 -Wall -gdwarf-2 -std=gnu99 -DHAVE_UPRINTF_IMPLEMENTATION -DF_CPU=16000000UL -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -MD -MP -MT frame-comm.o -MF dep/frame-comm.o.d -c ../frame-comm.c |
4 | avr-gcc -mmcu=atmega644 -Wall -gdwarf-2 -std=gnu99 -DHAVE_UPRINTF_IMPLEMENTATION -DF_CPU=16000000UL -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -MD -MP -MT uart-comm.o -MF dep/uart-comm.o.d -c ../uart-comm.c |
5 | avr-gcc -mmcu=atmega644 -Wl,-Map=t644.map main.o uart-printf.o frame-comm.o uart-comm.o -lm -o t644.elf |
6 | avr-objcopy -O ihex -R .eeprom -R .fuse -R .lock -R .signature t644.elf t644.hex |
7 | avr-objcopy -j .eeprom --set-section-flags=.eeprom="alloc,load" --change-section-lma .eeprom=0 --no-change-warnings -O ihex t644.elf t644.eep || exit 0 |
8 | avr-objdump -h -S t644.elf > t644.lss |
9 | |
10 | AVR Memory Usage |
11 | ---------------- |
12 | Device: atmega644 |
13 | |
14 | Program: 3022 bytes (4.6% Full) |
15 | (.text + .data + .bootloader) |
16 | |
17 | Data: 3387 bytes (82.7% Full) |
18 | (.data + .bss + .noinit) |
19 | |
20 | |
21 | Build succeeded with 0 Warnings... |
1 | ... |
2 | case ST_timer0: |
3 | timer0 = uart_getc(); |
4 | 2ba: 0e 94 ba 02 call 0x574 ; 0x574 <uart_getc> |
5 | 2be: 08 2f mov r16, r24 |
6 | next_state = ST_timer1; |
7 | break; |
8 | case ST_timer1: |
9 | timer1 = uart_getc(); |
10 | 2c0: 0e 94 ba 02 call 0x574 ; 0x574 <uart_getc> |
11 | 2c4: 18 2f mov r17, r24 |
12 | next_state = ST_checksum; |
13 | break; |
14 | case ST_checksum: |
15 | if (uart_checksum_recv()) { /* checksum successful */
|
16 | 2c6: 0e 94 e1 02 call 0x5c2 ; 0x5c2 <uart_checksum_recv> |
17 | 2ca: 88 23 and r24, r24 |
18 | 2cc: 79 f1 breq .+94 ; 0x32c <main+0x122> |
19 | /* set up timer with the value we just got */ |
20 | timer_count = orig_timer_count = (((uint16_t)timer1)<<8) | timer0; |
21 | 2ce: 91 2f mov r25, r17 |
22 | 2d0: 80 e0 ldi r24, 0x00 ; 0 |
23 | 2d2: 20 2f mov r18, r16 |
24 | 2d4: 30 e0 ldi r19, 0x00 ; 0 |
25 | 2d6: 82 2b or r24, r18 |
26 | 2d8: 93 2b or r25, r19 |
27 | 2da: 90 93 37 02 sts 0x0237, r25 |
28 | 2de: 80 93 36 02 sts 0x0236, r24 |
29 | 2e2: 80 91 36 02 lds r24, 0x0236 |
30 | 2e6: 90 91 37 02 lds r25, 0x0237 |
31 | 2ea: 90 93 3a 0e sts 0x0E3A, r25 |
32 | 2ee: 80 93 39 0e sts 0x0E39, r24 |
33 | |
34 | ... |
35 | |
36 | char uart_checksum_recv(void) |
37 | {
|
38 | 5c2: 1f 93 push r17 |
39 | const uint8_t v = checksum_accu & 0xff; |
40 | 5c4: 10 91 34 02 lds r17, 0x0234 |
41 | const uint8_t c = uart_getc(); |
42 | 5c8: 0e 94 ba 02 call 0x574 ; 0x574 <uart_getc> |
43 | 5cc: 90 e0 ldi r25, 0x00 ; 0 |
44 | 5ce: 18 17 cp r17, r24 |
45 | 5d0: 09 f4 brne .+2 ; 0x5d4 <uart_checksum_recv+0x12> |
46 | 5d2: 91 e0 ldi r25, 0x01 ; 1 |
47 | return (v == c); |
48 | } |
49 | 5d4: 89 2f mov r24, r25 |
50 | 5d6: 1f 91 pop r17 |
51 | 5d8: 08 95 ret |
52 | |
53 | ... |
Thread beobachten
Seitenaufteilung abschaltenBitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.