Forum: Compiler & IDEs Compilerfehler in avr-gcc 4.8.2?


von Uhu U. (uhu)


Lesenswert?

Der Quellcode:
1
#define SWAPB(x)    (x)
2
3
void initWatchdog(uint8_t timeout) {
4
    uint8_t sreg = SREG;
5
    cli();
6
7
    if (mcusr & (1 << PORF))                  // on power on reset
8
        WdtTrapAddress = 0;
9
10
    // scan wdt isr for return address offset
11
    const uint16_t __flash *pushPtr = (const uint16_t __flash *)
12
       (*((const uint16_t __flash *) 0x1a) << 1) + 5;
13
    uint16_t code;
14
15
    isrStackOffset = 3;
16
    code = SWAPB(*pushPtr);
17
    while (0x0f92u == (code & 0x0ffeu)) {
18
        pushPtr++;
19
        code = SWAPB(*pushPtr);
20
        isrStackOffset++;
21
    }
22
23
    wdt_enable(timeout);
24
    WDTCSR |= (1 << WDIE);              // Enable WDT interrupt
25
26
    SREG = sreg;
27
}

Der daraus erzeugte asm-Code:
1
void initWatchdog(uint8_t timeout) {
2
    uint8_t sreg = SREG;
3
    176e:  6f b7         in  r22, 0x3f  ; 63
4
    cli();
5
    1770:  f8 94         cli
6
7
    if (mcusr & (1 << PORF))                             // on power on reset
8
    1772:  90 91 f5 01   lds  r25, 0x01F5
9
    1776:  90 ff         sbrs  r25, 0
10
    1778:  04 c0         rjmp  .+8        ; 0x1782 <initWatchdog+0x14>
11
        WdtTrapAddress = 0;
12
    177a:  10 92 f7 01   sts  0x01F7, r1
13
    177e:  10 92 f6 01   sts  0x01F6, r1
14
15
    // scan wdt isr for return address offset
16
    const uint16_t __flash *pushPtr = (const uint16_t __flash *) 
17
        (*((const uint16_t __flash *) 0x1a) << 1) + 5;
18
    1782:  ea e1         ldi  r30, 0x1A  ; 26
19
    1784:  f0 e0         ldi  r31, 0x00  ; 0
20
    1786:  25 91         lpm  r18, Z+
21
    1788:  35 91         lpm  r19, Z+
22
    178a:  22 0f         add  r18, r18
23
    178c:  33 1f         adc  r19, r19
24
    uint16_t code;
25
26
    isrStackOffset = 3;
27
    code = SWAPB(*pushPtr);
28
    178e:  f9 01         movw  r30, r18
29
    1790:  3a 96         adiw  r30, 0x0a  ; 10
30
#1  1792:  45 91         lpm  r20, Z+
31
#1  1794:  55 91         lpm  r21, Z+
32
    1796:  54 27         eor  r21, r20
33
    1798:  45 27         eor  r20, r21
34
    179a:  54 27         eor  r21, r20
35
    179c:  f9 01         movw  r30, r18
36
    179e:  3c 96         adiw  r30, 0x0c  ; 12
37
    while (0x0f92u == (code & 0x0ffeu)) {
38
    17a0:  93 e0         ldi  r25, 0x03  ; 3
39
    17a2:  21 e0         ldi  r18, 0x01  ; 1
40
    17a4:  29 0f         add  r18, r25
41
    17a6:  4e 7f         andi  r20, 0xFE  ; 254
42
    17a8:  5f 70         andi  r21, 0x0F  ; 15
43
    17aa:  42 39         cpi  r20, 0x92  ; 146
44
    17ac:  5f 40         sbci  r21, 0x0F  ; 15
45
    17ae:  39 f4         brne  .+14       ; 0x17be <initWatchdog+0x50>
46
        pushPtr++;
47
        code = SWAPB(*pushPtr);
48
#2  17b0:  41 91         ld  r20, Z+
49
#2  17b2:  51 91         ld  r21, Z+
50
    17b4:  54 27         eor  r21, r20
51
    17b6:  45 27         eor  r20, r21
52
    17b8:  54 27         eor  r21, r20
53
    17ba:  92 2f         mov  r25, r18
54
    17bc:  f2 cf         rjmp  .-28       ; 0x17a2 <initWatchdog+0x34>
55
    17be:  90 93 f4 01   sts  0x01F4, r25
56
        isrStackOffset++;
57
    }
58
59
    wdt_enable(timeout);
60
    17c2:  83 ff         sbrs  r24, 3
61
    17c4:  02 c0         rjmp  .+4        ; 0x17ca <initWatchdog+0x5c>
62
    17c6:  98 e2         ldi  r25, 0x28  ; 40
63
    17c8:  01 c0         rjmp  .+2        ; 0x17cc <initWatchdog+0x5e>
64
    17ca:  98 e0         ldi  r25, 0x08  ; 8
65
    17cc:  87 70         andi  r24, 0x07  ; 7
66
    17ce:  89 2b         or  r24, r25
67
    17d0:  28 e1         ldi  r18, 0x18  ; 24
68
    17d2:  30 e0         ldi  r19, 0x00  ; 0
69
    17d4:  0f b6         in  r0, 0x3f  ; 63
70
    17d6:  f8 94         cli
71
    17d8:  a8 95         wdr
72
    17da:  20 93 60 00   sts  0x0060, r18
73
    17de:  0f be         out  0x3f, r0  ; 63
74
    17e0:  80 93 60 00   sts  0x0060, r24
75
    WDTCSR |= (1 << WDIE);                              // Enable WDT interrupt
76
    17e4:  80 91 60 00   lds  r24, 0x0060
77
    17e8:  80 64         ori  r24, 0x40  ; 64
78
    17ea:  80 93 60 00   sts  0x0060, r24
79
80
    SREG = sreg;
81
    17ee:  6f bf         out  0x3f, r22  ; 63
82
    17f0:  08 95         ret
83
}

Bei Adresse 1792 (Markierung #1) wird *pushPtr korrekt per lpm geladen.

In der While-Schleife auf Adresse 17b0 (Markierung #2) wird der nächste 
code geladen - diesmal nicht per lpm sondern mit ld. Der Compiler hat 
also den Modifier __flash am pushPtr verloren und liest jetzt aus dem 
RAM.


Wenn ich die Deklaration für pushPtr ändere in
1
const __flash uint16_t *pushPtr = 

verschwinden die 3 seltsamen eor ab Adressen 1796 und 17b4. Am 
Ladefehler in der while-Schleife ändert sich nichts.

: Bearbeitet durch User
Beitrag #5707726 wurde vom Autor gelöscht.
von (prx) A. K. (prx)


Lesenswert?

Ändere testweise die Initialisierung so, dass dem Compiler nichts über 
die Flash-Adresse bekannt ist, auch kein Wertebereich, und kein Cast 
darin vorkommt. Beispielsweise per Parameter von ebendiesem Typ. Es geht 
dabei nur um den erzeugten Code, das Programm muss nicht funktionieren.

Die 3 EORs sind ein Byte-Swap. Du bist ganz sicher, dass der 
präsentierte Quellcode und der Asm-Code zusammen gehören?

: Bearbeitet durch User
von Uhu U. (uhu)


Lesenswert?

Meinst du so:
1
const __flash uint16_t *pushPtr = (*((const uint16_t *) 0x1a) << 1) + 5;

Das gibt eine Warnung, aber an dem vermurksten Code ändert sich nichts. 
(Außer die eor sind weg - aber das hatte ich oben schon angemerkt.)

von (prx) A. K. (prx)


Lesenswert?

typedef ... ptr_t;
void initWatchdog(uint8_t timeout, ptr_t init_p) {
    ...
    ptr_t pushPtr = init_p;
    ...

Aber bitte so, dass der Compiler den Aufruf der Funktion nicht sieht.

Schau jedoch erst einmal, ob das SWAPB genau das ist oder war, was du 
oben reingeschrieben hast. Notfalls im Preprocessor-Output. Ich tippe 
drauf, dass die EORs etwas mit SWAPB zu tun hatten.

: Bearbeitet durch User
von Uhu U. (uhu)


Lesenswert?

A. K. schrieb:
> Aber schau erst einmal, ob das SWAPB genau das ist, was du oben
> reingeschrieben hast. Notfalls im Preprocessor-Output.

Das SWAPB habe ich wegdefiniert:
1
//#define SWAPB(x)    (uint16_t) (0xffffu & ((((x) & 0xff00u) >> 8) | (((x) & 0xffu) << 8)))
2
#define SWAPB(x)    (x)
3
4
void initWatchdog(uint8_t timeout) {
5
    uint8_t sreg = SREG;
6
    cli();
7
8
    if (mcusr & (1 << PORF))                             // on power on reset
9
        WdtTrapAddress = 0;
10
11
    // scan wdt isr for return address offset
12
    //const __flash uint16_t *pushPtr = (const __flash uint16_t *) (*((const __flash uint16_t *) 0x1a) << 1) + 5;
13
    const __flash uint16_t *pushPtr = 0x1a;
14
    uint16_t code;
15
16
    isrStackOffset = 3;
17
    code = SWAPB(*pushPtr);
18
    while (0x0f92u == (code & 0x0ffeu)) {
19
        pushPtr++;
20
        code = SWAPB(*pushPtr);
21
        isrStackOffset++;
22
    }
23
24
    wdt_enable(timeout);
25
    WDTCSR |= (1 << WDIE);                              // Enable WDT interrupt
26
27
    SREG = sreg;
28
}

Die Initialisierung für pushPtr mit 0x1a ändert auch nichts.

A. K. schrieb:
> Die 3 EORs sind ein Byte-Swap. Du bist ganz sicher, dass der
> präsentierte Quellcode und der Asm-Code zusammen gehören?

Die eor kommen vom SWAPB in der ursprünglichen Definition - etwas 
umständlich, wenn man einfach nur die Register beim Laden vertauschen 
müsste…

: Bearbeitet durch User
von (prx) A. K. (prx)


Lesenswert?

A. K. schrieb:
> Ändere testweise die Initialisierung so, dass dem Compiler nichts über
> die Flash-Adresse bekannt ist,

Uhu U. schrieb:
> const __flash uint16_t *pushPtr = 0x1a;

Hier kennt der Compiler die Adresse.

von (prx) A. K. (prx)


Lesenswert?

Uhu U. schrieb:
> Die eor kommen vom SWAPB in der ursprünglichen Definition - etwas
> umständlich, wenn man einfach nur die Register beim Laden vertauschen
> müsste…

Die beiden Ladebefehle bilden eine Einheit bei der Umsetzung der 16-Bit 
Operation auf die 8-Bit Maschine.

von Uhu U. (uhu)


Lesenswert?

A. K. schrieb:
> typedef ... ptr_t;
> void initWatchdog(uint8_t timeout, ptr_t init_p) {
>     ...
>     ptr_t pushPtr = init_p;
>     ...
>
> Aber bitte so, dass der Compiler den Aufruf der Funktion nicht sieht.

Der Aufruf steht in einem anderen Quellfile.

Wenn man es so macht, wie du vorgeschlagen hast, ist der Fehler weg (mit 
wegdefiniertem SWAPB):
1
    code = SWAPB(*pushPtr);
2
    1782:  fb 01         movw  r30, r22
3
    1784:  25 91         lpm  r18, Z+
4
    1786:  35 91         lpm  r19, Z+
5
    while (0x0f92u == (code & 0x0ffeu)) {
6
    1788:  93 e0         ldi  r25, 0x03  ; 3
7
    178a:  51 e0         ldi  r21, 0x01  ; 1
8
    178c:  59 0f         add  r21, r25
9
    178e:  2e 7f         andi  r18, 0xFE  ; 254
10
    1790:  3f 70         andi  r19, 0x0F  ; 15
11
    1792:  22 39         cpi  r18, 0x92  ; 146
12
    1794:  3f 40         sbci  r19, 0x0F  ; 15
13
    1796:  21 f4         brne  .+8        ; 0x17a0 <initWatchdog+0x32>
14
        pushPtr++;
15
        code = SWAPB(*pushPtr);
16
    1798:  25 91         lpm  r18, Z+
17
    179a:  35 91         lpm  r19, Z+
18
    179c:  95 2f         mov  r25, r21
19
    179e:  f5 cf         rjmp  .-22       ; 0x178a <initWatchdog+0x1c>
20
    17a0:  90 93 f4 01   sts  0x01F4, r25
21
        isrStackOffset++;
22
    }

: Bearbeitet durch User
von Johann L. (gjlayde) Benutzerseite


Lesenswert?

Uhu U. schrieb:
> Der Quellcode:

Wozu soll dieser Code denn gut sein?

Wofür auch immer, mit neueren Compilerversionen wird er nicht mehr 
funktionieren.

Falls in der ISR kein Frame gebraucht wird, dann kann die Anzahl der 
PUSHes im Prolog ganz einfach und unabhängig von der Compilerversion 
erhalten werden, zum Beispiel:
1
#include <avr/interrupt.h>
2
3
uint8_t isrStackOffset;
4
5
ISR (__vector_1)
6
{
7
    __asm ("ldi %0, .L__stack_usage" : "=d" (isrStackOffset));
8
}

Falls der Wert in einer anderen Funktion als der ISR benötigt wird, dann 
ist es etwas komplizierter und hässlicher, aber immer noch portabel 
zwischen unterschiedlichen Versionen von avr-gcc machbar:
1
// !!!
2
// !!! Muss mit -fno-toplevel-reorder compiliert werden
3
// !!! und unmittelbar auf die ISR folgen!
4
// !!!
5
6
extern const uint8_t n_pushed;
7
8
__asm (".global n_pushed"                           "\n\t"
9
       ".section .rodata.n_pushed,\"a\",@progbits"  "\n"
10
"n_pushed:"                                         "\n\t"
11
       ".byte .L__stack_usage");

Auch die return-Adresse einer Funktion kann man bekommen, und zwar mit 
__builtin_return_address(0).

http://gcc.gnu.org/onlinedocs/gcc/Return-Address.html

An die Speicheradresse der return-Adresse auf dem Stack kommt man aber 
leider nicht per Built-in heran.

Was v4.8 angeht, die ist ja schon was älter — ob es da ein Problem mit 
Address-Spaces gab weiß ich jetzt nicht mehr.  Auf die Schnelle find ich 
zumindets kein passenden PR.  Ne neuere Version wäre jedenfalls kein 
Luxus.

: Bearbeitet durch User
Bitte 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
Noch kein Account? Hier anmelden.