Problem mit SDCC

[Thomas Z. (usbman)]

Ich benutze in der Regel Keil c51, habe mir aber angewöhnt meinen Code 
auch für SDCC kompatibel zu machen. Die Unterschiede löse ich mit ein 
paar macros.
Bisher hatte ich v3.9 installiert nun aber ein Update auf v4.4 gemacht. 
Da die Jungs inzwischen wieder mal am Parser gespielt haben lassen sich 
alte Programme nicht mehr übersetzen, das sind aber Kleinigkeiten die 
sich einfach lösen lassen. Gut ist dass sich das CBYTE macro aus Keils 
<absacc.h> nun auch im SDCC fehlerfrei benutzen lässt. ( #define CBYTE 
((uint8_t volatile CODE  *) 0) )

Dieses reduzierte Programm wirft eine warning:
warning 196: pointer target lost const qualifier in Zeile 14.

#include <stdint.h>

__code uint8_t DevDesc[18] = { 0x12,0x01,0x10,0x01,0xFF,0x00,0x00,0x08,
                               0xC0,0x16,0xDC,0x05,0x00,0x10,0x01,0x02,
                               0x03,0x10 };

__xdata __at (0x0000) uint8_t  EP0_Buffer[0x08 + 2];

uint8_t *ptr;

void main (void)
{
   uint8_t i;
   ptr = &DevDesc[0];
   for (i= 0;i<8;i++) 
      EP0_Buffer[i] = *ptr++;

   while(1) {}   
}

Die warning bekommt man relativ einfach weg wenn man const uint8_t *ptr 
schreibt. Soweit so gut. Ich habe mich aber ganz bewusst gegen const auf 
den generic Pointer entschieden weil ich auch schreibend zugreifen muss. 
Das geht natürlich nicht mit const.

Der generierte code ist ok und funktioniert, ich mag allerdings keine 
Warnings da diese sehr oft eben doch auf ein Problem hinweisen. Ich 
halte die warning für einen Bug. Die kommt übrigens auch bei v3.9. Keil 
übersetzt das ohne zu meckern.

Was meint Ihr? Mach ich da einen Fehler oder ist das ein Bug im SDCC?

[Niklas G. (erlkoenig)]

Du konvertierst ein "const uint8_t*" nach "uint8_t*", das gibt in jedem 
Compiler für jede Plattform eine Warning (zu Recht).

Thomas Z. schrieb:
> Ich habe mich aber ganz bewusst gegen const auf
> den generic Pointer entschieden weil ich auch schreibend zugreifen muss.

Du überschreibst den Device-Deskriptor im Flash? Über eben diesen 
Pointer?

[Thomas Z. (usbman)]

Niklas G. schrieb:
> Du konvertierst ein "const uint8_t*" nach "uint8_t*", das gibt in jedem
> Compiler für jede Plattform eine Warning (zu Recht).

ich könnte das verstehen wenn da stehen würde

const __code uint8_t DevDesc[18]

Der Compiler scheint aus __code automatisch const __code zu machen. 
Trotzdem ist es doch legal einem generic Pointer ein Array aus dem Flash 
zuzuweisen.

> Du überschreibst den Device-Deskriptor im Flash? Über eben diesen
> Pointer?
natürlich nicht. Wie sollte das auch gehen? Ich benutze den Pointer halt 
an anderer Stelle auch schreibend.

[Niklas G. (erlkoenig)]

Thomas Z. schrieb:
> Der Compiler scheint aus __code automatisch const __code zu machen.

Das macht auch Sinn denke ich, damit man nicht versehentlich versucht da 
drauf zu schreiben.

Thomas Z. schrieb:
> Trotzdem ist es doch legal einem generic Pointer ein Array aus dem Flash
> zuzuweisen.

Wenn das einfach so ginge könntest du sehr einfach versehentlich in den 
Flash "schreiben" (weiß nicht was der 8051 dann macht, Absturz?).

Thomas Z. schrieb:
> Ich benutze den Pointer halt
> an anderer Stelle auch schreibend.

Benutz halt einen anderen Pointer? Holzhammermethode: Eine "union" mit 
"const" und "nicht-const" Pointer. Oder halt manuell auf "uint8_t*" 
casten.

[Oliver S. (oliverso)]

Niklas G. schrieb:
> Oder halt manuell auf "uint8_t*"
> casten.

Das hilft zwar, deutet aber halt zusammen mit

Thomas Z. schrieb:
> Ich benutze den Pointer halt
> an anderer Stelle auch schreibend.

auf ein vermeidbares „Ich schiesse mir von der Brust durchs Knie ins 
Auge“-Problem hin.

Sauber gelöst, würde das alles nicht nötig sein.

Oliver

[Klaus S. (kseege)]

Thomas Z. schrieb:
> Was meint Ihr? Mach ich da einen Fehler oder ist das ein Bug im SDCC?

Als langjähriger ASM-Programmierer der 8051-Familie würde ich mir 
natürlich erstmal den generierten Code ansehen, dann wüßte man mehr.
Meine Vermutung ist, daß es sich um eine Warnung vor einer eventuellen 
Fehlermöglichkeit handelt. Der deklarierte Pointer steht im internen RAM 
und kann benutzt werden, um auf internes und externes RAM sowie auf ROM 
zuzugreifen. Auf ROM kann man eben (mit movc) nur lesend zugreifen und 
nicht schreibend, das ist m.E. die potentielle Fehlermöglichkeit, die 
später als error aufschlagen kann.

Schreibt man das Ganze ohne Pointer, solte es ohne Warnung durchgehen.
EP0_Buffer[i] = DevDesc[i];

Gruß Klaus

[Klaus S. (kseege)]

Ansonsten sollte eigentlich (nach meinen unperfekten C-Kentnissen) ein 
umcasten helfen:
EP0_Buffer[i] = (_xdata uint8_t)(*ptr++);

Gruß Klaus

[Klaus S. (kseege)]

Thomas Z. schrieb:
> ptr = &DevDesc[0];

Bei mir scheint heute der Euro nur cent-weise zu fallen. Natürlich 
gehört da korrekterweise ebenfalls ein cast hin, bin aber nich ganz 
sattelfest, wie der aussehen muß. Der 8051 hat nun mal 3 
unterschiedliche Adreßräume und der korrekte Typ muß für einen Pointer 
per cast angegeben werden, wenn es nicht der definierte (in diesem Fall 
auf inernes RAM) ist.

Gruß Klaus

[Thomas Z. (usbman)]

Klaus S. schrieb:
> Als langjähriger ASM-Programmierer der 8051-Familie würde ich mir
> natürlich erstmal den generierten Code ansehen
Hab ich das passt schon. Ist ja auch nur eine warning. Der code wird 
korrekt erzeugt mit MOVC.

Wie gesagt Keil c51 übersetzt das ohne Murren. Tatsächlich ist die ptr 
Variable nicht global sondern in einer Interrupt Funktion lokal 
definiert

static uint8_t *ptr;

Das spielt für Warnung aber keine Rolle. Üblicherweise arbeite ich nur 
mit Keil und teste manchmal mit SDCC wenn der code veröffentlicht werden 
soll.

[Peter D. (peda)]

Thomas Z. schrieb:
> Ich benutze den Pointer halt
> an anderer Stelle auch schreibend.

Ich würde local verwendete Pointer auch local definieren. Dann könnte 
der Compiler sie in Register optimieren (kürzere Befehle, weniger SRAM) 
oder mit anderen temporären Variablen überlagern.

Man kann auch den Pointer ganz weglassen und über den Schleifenzähler 
als Index auf das Element zugreifen. Dann kann sich der Compiler selber 
überlegen, wie er das optimiert.

Globale Variablen nimmt man nur, wenn sie auch funktionsübergreifend 
ihren Wert behalten müssen.

[Peter D. (peda)]

Thomas Z. schrieb:
> Tatsächlich ist die ptr
> Variable nicht global sondern in einer Interrupt Funktion lokal
> definiert

Dann würde ich erst recht nicht mit dem Hammer "generic Pointer" 
draufschlagen, sondern memory spezific Pointer nehmen.
Hast Du Dir mal den erzeugten Code dafür angesehen?

[Thomas Z. (usbman)]

Peter D. schrieb:
> Globale Variablen nimmt man nur, wenn sie auch funktionsübergreifend
> ihren Wert behalten müssen.

Der wert muss erhalten bleiben da ich immer nur Teile des Deskriptors 
pro Aufruf zurück geben kann. Deswegen auch die Deklaration als static 
in der Interrupt Funktion.

> Dann würde ich erst recht nicht mit dem Hammer "generic Pointer"
> draufschlagen, sondern memory spezific Pointer nehmen.

generic ist notwendig weil einige wenige Descriptoren im Ram liegen. 
(z.B. Seriennummer). Mir ist klar, dass der Zugriff über lib Funktionen 
passiert
und deswegen etwas mehr code erzeugt wird. Wie angemerkt funktioniert 
das mit Keil ohne Warnung. Der SDCC erzeugt halt ohne const diese 
Warnung.

[Peter D. (peda)]

Thomas Z. schrieb:
> generic ist notwendig weil einige wenige Descriptoren im Ram liegen.

Ist natürlich schwer nachzuvollziehen, da keiner den wirklichen Code 
kennt.
Ich würde die 18 Bytes mit in XDATA legen, ist ja nicht so viel. Sind es 
zusammen <256 Byte, dann muß der Compiler auch nur 1 Byte Index sichern, 
statt 3 Byte generic Pointer.

[Thomas Z. (usbman)]

Ich habe mich über die Feuertage noch mal intensiver mit den diversen 
Pointer Varianten  beschäftigt. Neben Keil habe ich auch SDCC und 
testweise mal IAR EW8051 benutzt.
Im Ergebnis ist wohl so dass bei Keil relativ gutmütig ist was Pointer 
angeht.  SDCC schaut da schon genauer hin. Bei IAR ist es so dass mein 
Code nur mit expliziten Typ Konvertierungen kompiliert.
Die zusätzlichen Typkonvertierungen stören übrigens weder Keil noch 
SDCC. Die Codegröße bleibt unverändert.

Ich bin da wahrscheinlich etwas vom Keil Pointer Konzept verwöhnt. Die 
anderen Compiler sind nicht so großzügig was den Umgang mit Pointern 
angeht.

[Harald K. (kirnbichler)]

Thomas Z. schrieb:
> Ich bin da wahrscheinlich etwas vom Keil Pointer Konzept verwöhnt.

"Verwöhnt" ist hier der falsche Begriff. Der Compiler ist 
"fehlertoleranter", d.h. prüft den Code weniger genau. Das mag auf den 
ersten Blick "bequemer" erscheinen, hilft aber, sich einen fehlerhaften 
Programmierstil anzugewöhnen, mit dem man irgendwann auf die Nase fallen 
wird, weil der Compiler daraus unter irgendwelchen Bedingungen dann doch 
anderen Code generiert, als erwartet.

Man sollte mit einem modernen Compilter und mit der schärfsten 
Warnungsstufe arbeiten und nur sehr offensichtliche Warnungen 
deaktivieren (wie z.B. "Rückgabewert von printf() wird nicht 
ausgewertet").

Und dann so programmieren, daß der Compiler keine Warnungen auswirft.

Der Gegenentwurf wäre ein "K&R"-Compiler für den Sprachstandard vor 
C89, der praktisch keinerlei Typüberprüfungen durchführen kann, weil 
eben der fossile K&R-Dialekt so etwas nicht vorsieht. Der ist dann 
richtig "bequem" und "verwöhnt" einen mit ohne Warnungen und 
Fehlermeldungen compilierendem Code.

[Peter D. (peda)]

Harald K. schrieb:
> "Verwöhnt" ist hier der falsche Begriff. Der Compiler ist
> "fehlertoleranter", d.h. prüft den Code weniger genau.

Nö.
Harward mit getrennten Adreßbereichen und damit verbundene generische 
Pointer sind nicht von C-Standard abgedeckt und daher immer 
implementationsabhängig. Man kann daher nicht sagen, das eine ist 
richtig und das andere ist falsch.

Im AVR-GCC werden ja auch viele Prüfungen ausgeschaltet, um Daten im 
Flash abzulegen. Da der keine generic und memory specific Pointer kennt, 
muß man spezielle Zugriffsfunktionen benutzen (strcmp_P usw.).

[Johann L. (gjlayde)]

Peter D. schrieb:
> Im AVR-GCC werden ja auch viele Prüfungen ausgeschaltet, um Daten im
> Flash abzulegen.

??? Was soll denn da "abgeschaltet" worden sein?

[Niklas G. (erlkoenig)]

Peter D. schrieb:
> Man kann daher nicht sagen, das eine ist richtig und das andere ist
> falsch.

Naja, das Verhalten von "const" da trotzdem konsistent durchzuziehen ist 
aber sinnvoll. Dass man auf Flash-Pointer nicht schreiben können soll 
macht ja Sinn. Welchen Sinn hätte "const" in der Sprache sonst, wenn der 
Compiler das einfach übergeht?

[Harald K. (kirnbichler)]

Peter D. schrieb:
> Harward mit getrennten Adreßbereichen und damit verbundene generische
> Pointer sind nicht von C-Standard abgedeckt und daher immer
> implementationsabhängig. Man kann daher nicht sagen, das eine ist
> richtig und das andere ist falsch.

Aha. Also ist ad-libitum-Programmieren nach dem Motto "Mach, was ich 
meinen könnte, aber ich sag's Dir nicht genau" die bessere 
Vorgehensweise?

[Thomas Z. (usbman)]

Harald K. schrieb:
> Aha. Also ist ad-libitum-Programmieren nach dem Motto "Mach, was ich
> meinen könnte, aber ich sag's Dir nicht genau" die bessere
> Vorgehensweise?

Peter hat schon recht. Pointer sind auf einem 8051 eine ziemlich teure 
Sache. Die verschiedenen Speicherklassen machen die Sache kompliziert. 
So hat jeder Compiler Hersteller sein eigenes Süppchen gekocht. Keil 
Raisonance und SDCC lösen die ganze Sache ziemlich ähnlich. Ein Pointer 
ohne zusätzliche Angaben ist erst mal ein generischer Pointer.

Anders ist es beim IAR. Da hängt der der Pointertyp von der gewählten 
Speicherklasse ab. Generische Pointer muss man extra mit __generic 
deklarieren. Auch Typkonvertierungen ändern daran nichts und führen ggv 
zu falschem Code.

[Peter D. (peda)]

Johann L. schrieb:
> ??? Was soll denn da "abgeschaltet" worden sein?

Er kann die Pointer nicht auf Bereiche (SRAM, Flash) prüfen.
Wenn ich z.B. statt sscanf_P nur sscanf schreibe, gibt es keinerlei 
Warnungen, aber er wird ins Leere lesen.
Ich benutze das AS7 (avvr-gcc 5.4.0).

[Philipp Klaus K. (pkk)]

Peter D. schrieb:
> Harward mit getrennten Adreßbereichen und damit verbundene generische
> Pointer sind nicht von C-Standard abgedeckt […]

Kommt darauf an, was man unter dem "C-Standard" versteht. Im 
eigentlichen Sprachstandard ISO/IEC 9899:1990 findet sich zwar nichts 
dazu, aber das gilt genauso auch für manch andere in C übliche 
Funktionalität, die sich stattdessen in TS/TR findet, z.B. 
IEEE-Arithmetik in ISO/IEC TS 18661.

Für die verschiedenen Adressbereiche des MCS-51 sind die Abschnitte zu 
"named address spaces" in ISO/IEC TR 18037 (auch "Embedded C"-Standard 
genannt) relevant. __code und __xdata in SDCC sind "address space type 
qualifier" für "intrinsic address spaces" im Sinne von ISO/IEC TR 18037.

[Johann L. (gjlayde)]

Peter D. schrieb:
> Johann L. schrieb:
>> ??? Was soll denn da "abgeschaltet" worden sein?
>
> Er kann die Pointer nicht auf Bereiche (SRAM, Flash) prüfen.
> Wenn ich z.B. statt sscanf_P nur sscanf schreibe, gibt es keinerlei
> Warnungen, aber er wird ins Leere lesen.

Da gibt es 2 Lösungen:

1. Verwende Standard-C, also sscanf etc.

2. Verwende ISO/IEC TR 18037 "Embedded C" mit __flash und passendem 
Prototyp:

extern int sscanf_flash (const char*, const __flash char*, ...);

[Peter D. (peda)]

Johann L. schrieb:
> 2. Verwende ISO/IEC TR 18037 "Embedded C" mit __flash und passendem
> Prototyp:
> extern int sscanf_flash (const char*, const __flash char*, ...);

Scheint wohl in AS7 nicht zu gehen:
C:\Work\Firmware\Modbus\PU-Parser/parse.c:22: undefined reference to 
`sscanf_flash'

[Johann L. (gjlayde)]

Peter D. schrieb:
> Johann L. schrieb:
>> 2. Verwende ISO/IEC TR 18037 "Embedded C" mit __flash und passendem
>> Prototyp:
>> extern int sscanf_flash (const char*, const __flash char*, ...);
>
> Scheint wohl in AS7 nicht zu gehen:
> C:\Work\Firmware\Modbus\PU-Parser/parse.c:22: undefined reference to
> `sscanf_flash'

Hmmm stimmt. Problem ist, dass sscanf_P in stdio.h deklariert ist, man 
wird den Prototypen also nicht so einfach los.  Was gehen sollte ist:

extern int sscanf_flash (const char*, const __flash char*, ...) __asm("sscanf_P");

#define FLASH_STR(s) \
    (__extension__({static const __flash char __c[] = (s); &__c[0];}))
int func (const char *buf)
{
    int i;
    sscanf_flash (buf, FLASH_STR ("%i"), &i);
    return i;
}

Das PSTR aus avr/pgmspace.h verwendet PROGMEM, das formal im Generic 
Space liegt.  Daher wird auch ein eigenes FLASH_STR verwendet, das in 
__flash allokiert, so dass -Waddr-space-convert keine Warnung wirft.

Das __asm setzt den Assembly-Name der Funktion, d.h. im generierten 
Assembly wird die Funktion mit sscanf_P angesprochen.

[Oliver S. (oliverso)]

Peter D. schrieb:
> Johann L. schrieb:
>> ??? Was soll denn da "abgeschaltet" worden sein?
>
> Er kann die Pointer nicht auf Bereiche (SRAM, Flash) prüfen.
> Wenn ich z.B. statt sscanf_P nur sscanf schreibe, gibt es keinerlei
> Warnungen, aber er wird ins Leere lesen.
> Ich benutze das AS7 (avvr-gcc 5.4.0).

Da ist aber nichts abgeschaltet, der Compiler kennt halt nur C, und das 
kennt kein Harvard-Konzept. Die ganze Flashbehandlung mit progmem, die 
ja in den _P-Funktionen steckt, findet auf Sourcelevelebene statt, nicht 
auf der Compilerebene.

Anders wäre das mit _flash, falls dein Compiler das schon kennt.

Oliver