AVR gcc: Funktionsparameter werden überschrieben

[G T M. (gt_m)]

Hallo in die Runde,

ich habe hier gerade ein Problem mit einem Atmega88, das ich mir nicht 
erklären kann.

Ich übergebe einer Funktion einen Funktionspointer, der eigentlich 
ausgewertet werden soll. Stattdessen wird aber sein Wert innerhalb der 
Funktion verändert.

Der Code (aufs relevante reduziert):

...
uint8_t te = timerexist(led_on);
if (!te) ...
...
uint8_t timerexist(const void (*f)(void*)) {
  uint8_t current = t_first;
  while(current != T_LAST) {
    if(&(tasks[current].func.f) == &f)
      return 1;
...

Beim Blick in den Assembler ist denn auch schnell klar, wo es kaputt 
geht:

  uint8_t te = timerexist(led_on);
00000A36  LDI R24,0x95    Load immediate 
00000A37  LDI R25,0x07    Load immediate 
00000A38  RCALL PC-0x0122  Relative call subroutine 
     if(!te){
00000A39  TST R24    Test for Zero or Minus 
...
    uint8_t timerexist(const void (*f)(void*)) {
00000916  PUSH R28       Push register on stack 
00000917  PUSH R29       Push register on stack 
00000918  RCALL PC+0x0001  Relative call subroutine 
00000919  IN R28,0x3D    In from I/O location 
0000091A  IN R29,0x3E    In from I/O location 
    uint8_t current = t_first;
0000091B  LDS R24,0x0165  Load direct from data space 
    while(current != T_LAST) {
0000091D  CPI R24,0xFF    Compare with immediate 
0000091E  BREQ PC+0x2D    Branch if equal 
    if(&(tasks[current].func.f) == &f)
0000091F  LDI R25,0x00    Load immediate 
00000920  MOVW R20,R24    Copy register pair

Die Adresse von led_on ist in R25 R24 0x0795. In timerexist wird dann 
erst R24 mit t_first überschrieben und R25 auf 0x00 gesetzt.

Aber warum? Sollte der Compiler nicht wissen, dass in R24 und R25 noch 
Daten sind, die benötigt werden?
Im Debugger wird f weiterhin auf function (word address) 
*{registers}@R25 R24 abgebildet, und man kann beim Durchsteppen 
mitverfolgen, wie der Wert kaputt geht. (von 0x0795 über 0x07ab zu 
0x00ab)

Kompiliert wurde mit

avr-gcc -funsigned-char -funsigned-bitfields -O3 -fpack-struct -fshort-enums -g2 -DDEBUG -Wall -c -std=gnu99 -MD -MP -MF "$(@:%.o=%.d)" -MT"$(@:%.o=%.d)" -MT"$(@:%.o=%.o)"  -mmcu=atmega88

Ich bin gerade echt ein bisschen ratlos, wie man das Problem beheben 
könnte. Ideen?

[Johann L. (gjlayde)]

Das Problem liegt ziemlich sicher in den Pünktchen oder in = &f anstatt 
in =f.  Du willst ziemlich sicher nicht die Adresse der lokalen 
Variablen f zuweisen.

Und beachte Warnungen!

[Rene H. (Gast)]

@Johann
Wo siehst du eine Zuweisung? Ich sehe bloss einen Vergleich.

@G T M
Hast du schon mal ohne -O3 kompiliert?

Grüsse,
R.

[Wusel D. (stefanfrings_de)]

Da in deinem Code keine Zuweisung auf f stattfindet, vermute ich den 
Fehler ganz woanders. Möglicherweise hast Du einen Stack überlauf. Stack 
überlauf ist beinahe immer die Ursache für unerwartete (vor allem nicht 
programmierte) Schreibzugriffe auf Variablen.

Dass der Compiler R25 überschreibt, muss kein Fehler sein. Wenn f später 
nochmal gebraucht wird, kann er ja erneut einen LDI R25,0x07 ausführen.

Deaktiviere mal den Optimizer mit -O0, dann erhälst Du einen weniger 
veränderten Code der eher dem C Quelltext entspricht. Ich wette, dass 
dein Problem dann immer noch besteht, und dann wird eventuell 
offensichtlicher, das die Fehlerursache ein Stack Überlauf ist (oder 
auch nicht).

Ich habe mir angewöhnt, Code stets mit der Option -O0 zu debuggen. Es 
sei denn, das Problem tritt nur bei einer bestimmten Optimierungs-Stufe 
auf.

[Johann L. (gjlayde)]

Ok, ist ein Vergleich.  Für den gilt aber das gleiche in grün.

[foobar (Gast)]

Johann L. schrieb:
> Ok, ist ein Vergleich.  Für den gilt aber das gleiche in grün.

Sehe ich auch so. f ist schon ein Pointer. Davon noch einmal die Adresse 
zu nehmen macht an dieser Stelle so keinen Sinn.  Kann aber auch daran 
liegen, dass zu wenig Kontext vorhanden ist.

[Johann L. (gjlayde)]

Soll wohl das gemeint sein:

if (tasks[current].func.f == f)

[Rene H. (Gast)]

Ziemlich sicher sogar. Stellt sich nur die Frage ob das den Fehler 
behebt.

Ohne mehr Kontext kann man da nicht helfen.

Grüsse,
René

[Andreas B. (andreas_b77)]

G T M. schrieb:
> Aber warum? Sollte der Compiler nicht wissen, dass in R24 und R25 noch
> Daten sind, die benötigt werden?

Wie schon bemerkt wurde, verwendest du die Daten im Parameter nicht, 
sondern nimmst nur die Adresse (zumindest angenommen, den Rest der 
Funktion sehen wir ja nicht). Der Compiler weiß also, dass die Daten 
tatsächlich nicht benötigt werden.

Außerdem ist es etwas sinnlos, einen void Rückgabewert const zu machen. 
Der Funktionspointer Parameter sollte vermutlich "void (*const 
f)(void*)" heißen.

[G T M. (gt_m)]

Erstmal Danke für die zahlreichen Antworten!

Johann L. schrieb:
> Und beachte Warnungen!

Keine Warnungen vorhanden.

Johann L. schrieb:
> Soll wohl das gemeint sein:
> if (tasks[current].func.f == f)
Rene H. schrieb:
> Ziemlich sicher sogar. Stellt sich nur die Frage ob das den Fehler
> behebt.

Hmm.. problematisch, weil der Compiler dann die Adresse hardcodiert und 
gleichzeitig wegoptimiert?

Wenn ich das & weglasse, bekommt der Debugger jedenfalls mit, was los 
ist:

Rene H. schrieb:
> @G T M
> Hast du schon mal ohne -O3 kompiliert?

Der sagt dann nämlich bei -O>0 zu f:
"Optimized away"

Andreas B. schrieb:
> Wie schon bemerkt wurde, verwendest du die Daten im Parameter nicht,
> sondern nimmst nur die Adresse (zumindest angenommen, den Rest der
> Funktion sehen wir ja nicht). Der Compiler weiß also, dass die Daten
> tatsächlich nicht benötigt werden.

Ja, Dein Post hat sich gerade mit meinem Tippen und ausprobieren 
überschnitten.

Aber auch wenn ich nicht die Adresse verwende, sondern die Pointer 
direkt vergleiche, wird das Ganze wegoptimiert, obwohl es für den 
Vergleich benötigt wird.

> Außerdem ist es etwas sinnlos, einen void Rückgabewert const zu machen.
> Der Funktionspointer Parameter sollte vermutlich "void (*const
> f)(void*)" heißen.
Stimmt natürlich. Muss bei der Fehlersuche verrutscht sein. Ändert aber 
nichts am Ergebnis.

Die gesamte funktion sieht (jetzt) so aus:

uint8_t timerexist(void (*const f)(void*)) {
  uint8_t current = t_first;
  while(current != T_LAST) {  
    if(tasks[current].func.f == f)
      return 1;
    current = tasks[current].next;
  }
  return 0;
}

und f wird außer bei -O0 wegoptimiert.
Wenn current != T_LAST ist, läuft die Schleife auch. Zum Vergleich (der 
nicht wegoptimiert wird) herangezogen werden die Register, in denen f 
vor dem Sprung in timerexist zu finden war.

Viele Grüße!

[Johann L. (gjlayde)]

Hast du einen Testfall, so daß man das nachvollziehen kann?

Mit deinem Code (zuzüglich stint.h) kommt:

foo.c: In function 'timerexist':
foo.c:7:21: error: 't_first' undeclared (first use in this function)
   uint8_t current = t_first;
                     ^
foo.c:7:21: note: each undeclared identifier is reported only once for each function it appears in
foo.c:8:20: error: 'T_LAST' undeclared (first use in this function)
   while(current != T_LAST) {  
                    ^
foo.c:9:8: error: 'tasks' undeclared (first use in this function)
     if(tasks[current].func.f == f)

[Johann L. (gjlayde)]

Folgender Testfall wird richtig übersetzt:

#include <stdint.h>

extern struct
{
    struct { void (*f)(void*); } func;
    uint8_t next;
} tasks[];

extern uint8_t t_first;

#define T_LAST 10

uint8_t timerexist (void (*f)(void*))
{
  uint8_t current = t_first;
  while (current != T_LAST)
  {  
    if (tasks[current].func.f == f)
      return 1;
    current = tasks[current].next;
  }
  return 0;
}

ergibt mit avr-gcc 4.8 und -Os

timerexist:
  lds r18,t_first   ;  8  movqi_insn/4  [length = 2]
.L2:
  cpi r18,lo8(10)   ;  34  *cmpqi/3  [length = 1]
  breq .L7   ;  35  branch  [length = 1]
  ldi r19,0   ;  63  movqi_insn/1  [length = 1]
  movw r30,r18   ;  61  *movhi/1  [length = 1]
  lsl r30   ;  69  *ashlhi3_const/2  [length = 2]
  rol r31
  add r30,r18   ;  17  *addhi3/1  [length = 2]
  adc r31,r19
  subi r30,lo8(-(tasks))   ;  18  *addhi3/2  [length = 2]
  sbci r31,hi8(-(tasks))
  ld r18,Z   ;  19  *movhi/3  [length = 2]
  ldd r19,Z+1
  cp r18,r24   ;  20  *cmphi/3  [length = 2]
  cpc r19,r25
  breq .L5   ;  21  branch  [length = 1]
  ldd r18,Z+2   ;  30  movqi_insn/4  [length = 1]
  rjmp .L2   ;  73  jump  [length = 1]
.L7:
  ldi r24,0   ;  5  movqi_insn/1  [length = 1]
  ret   ;  66  return  [length = 1]
.L5:
  ldi r24,lo8(1)   ;  4  movqi_insn/2  [length = 1]
  ret   ;  68  return  [length = 1]