www.mikrocontroller.net

Forum: Compiler & IDEs Korrupte Daten und Volatile


Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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?

Autor: holger (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
>    const char ch = UDR0;

Was soll denn der Quatsch mit dem const?

Autor: holger (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
>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;)

Autor: Johann L. (gjlayde) Benutzerseite
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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.

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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?

Autor: Nils S. (kruemeltee) Benutzerseite
Datum:

Bewertung
0 lesenswert
nicht lesenswert
>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

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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.

Autor: test (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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;

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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.

Autor: holger (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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?

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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.

Autor: ... (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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&f...

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&f...
z.B.:
http://www.avrfreaks.net/index.php?name=PNphpBB2&f...
http://www.avrfreaks.net/index.php?name=PNphpBB2&f...

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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

Autor: Käfer (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
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

Autor: ... (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Mit WinAVR kommt etwa folgendes raus:
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
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
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
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
avr-gcc -mmcu=atmega644 -Wl,-Map=t644.map main.o uart-printf.o frame-comm.o uart-comm.o    -lm  -o t644.elf
avr-objcopy -O ihex -R .eeprom -R .fuse -R .lock -R .signature  t644.elf t644.hex
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
avr-objdump -h -S t644.elf > t644.lss

AVR Memory Usage
----------------
Device: atmega644

Program:    3022 bytes (4.6% Full)
(.text + .data + .bootloader)

Data:       3387 bytes (82.7% Full)
(.data + .bss + .noinit)


Build succeeded with 0 Warnings...

...
      case ST_timer0:
  timer0 = uart_getc();
 2ba:  0e 94 ba 02   call  0x574  ; 0x574 <uart_getc>
 2be:  08 2f         mov  r16, r24
  next_state = ST_timer1;
  break;
      case ST_timer1:
  timer1 = uart_getc();
 2c0:  0e 94 ba 02   call  0x574  ; 0x574 <uart_getc>
 2c4:  18 2f         mov  r17, r24
  next_state = ST_checksum;
  break;
      case ST_checksum:
  if (uart_checksum_recv()) { /* checksum successful */
 2c6:  0e 94 e1 02   call  0x5c2  ; 0x5c2 <uart_checksum_recv>
 2ca:  88 23         and  r24, r24
 2cc:  79 f1         breq  .+94       ; 0x32c <main+0x122>
    /* set up timer with the value we just got */
    timer_count = orig_timer_count = (((uint16_t)timer1)<<8) | timer0;
 2ce:  91 2f         mov  r25, r17
 2d0:  80 e0         ldi  r24, 0x00  ; 0
 2d2:  20 2f         mov  r18, r16
 2d4:  30 e0         ldi  r19, 0x00  ; 0
 2d6:  82 2b         or  r24, r18
 2d8:  93 2b         or  r25, r19
 2da:  90 93 37 02   sts  0x0237, r25
 2de:  80 93 36 02   sts  0x0236, r24
 2e2:  80 91 36 02   lds  r24, 0x0236
 2e6:  90 91 37 02   lds  r25, 0x0237
 2ea:  90 93 3a 0e   sts  0x0E3A, r25
 2ee:  80 93 39 0e   sts  0x0E39, r24

...

char uart_checksum_recv(void)
{
 5c2:  1f 93         push  r17
  const uint8_t v = checksum_accu & 0xff;
 5c4:  10 91 34 02   lds  r17, 0x0234
  const uint8_t c = uart_getc();
 5c8:  0e 94 ba 02   call  0x574  ; 0x574 <uart_getc>
 5cc:  90 e0         ldi  r25, 0x00  ; 0
 5ce:  18 17         cp  r17, r24
 5d0:  09 f4         brne  .+2        ; 0x5d4 <uart_checksum_recv+0x12>
 5d2:  91 e0         ldi  r25, 0x01  ; 1
  return (v == c);
}
 5d4:  89 2f         mov  r24, r25
 5d6:  1f 91         pop  r17
 5d8:  08 95         ret

...

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [avrasm]AVR-Assembler-Code[/avrasm]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.
Hinweis: der ursprüngliche Beitrag ist mehr als 6 Monate alt.
Bitte hier nur auf die ursprüngliche Frage antworten,
für neue Fragen einen neuen Beitrag erstellen.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.