Zeiger auf String-Array wird falsch

[Philipp S. (philipp_s918)]

Hallo zusammen,

bei der Portierung eines Programms vom STM32F1 auf STM32L1 bin ich auf 
ein Problem gestoßen, bei dem ich ohne Hilfe nicht mehr weiter komme.

Im betreffenden Programmteil geht es um die Ausgabe von Menüpunkten auf 
einem Display. Lasse ich das Menü anzeigen, erscheinen bei jedem 
Menüpunkt nur wirre Zeichen. Ich verwende die Menü Lib von hier: 
https://github.com/Jomelo/LCDMenuLib2/tree/master

Anbei der verwendete Code auf den fehlerhaften Teil reduziert:

char content_text[20];
LCDML_getContent(content_text,0);

Definitionen:

#define pgm_read_word(addr) (*(const unsigned short *)(addr))

#define LCDML_getContent(var,id)\
char* temp1 = (char*)pgm_read_word(&(g_LCDML_DISP_lang_lcdml_table[id])));\
strcpy_P(var,temp1);

In der Funktion soll der Content für eine Displayzeile aus dem Flash 
geholt und in den Buffer content_text geschrieben werden. Dabei wird 
aber von der falschen Addresse gelesen, daher werden falsche Zeichen in 
content_text geschrieben.

Nehmen wir als Beispiel den Menüpunkt mit der id=0 (Entspricht dem Array 
Element g_LCDML_DISP_lang_lcdml_table[0]). Dort ist im Debugger der 
richtige String "Menüpunkt 1" ab Adresse 0x8012bcb zu sehen. In der 
Variable temp1 kommt dann aber nur noch der Wert 0x2bcb an. 
Wahrscheinlich wird hier falsch gecastet, daher ist die Adresse nur 2 
byte lang, der Rest wird abgeschnitten.
In meiner Implementierung für den STM32F1 ist das Problem wahrscheinlich 
nicht aufgefallen, da hier die Strings zufälligerweise in den ersten 
64kB des Flashs lagen.

Ich verstehe leider nicht, was in "*(const unsigned short *)" die 
doppelte Benutzung von * bewirkt. Den Code habe ich nur stumpf 
zusammenkopiert. Könnte mir vielleicht einer von den C-Profis unter euch 
sagen, wo der Fehler liegt?
Meine Vermutung ist, dass die Definition von pgm_read_word falsch ist. 
Das würde mich aber wundern, da dies in vielen Quellen im Netz in der 
pgmspace.h Datei für den STM32 so zu sehen ist.

Vielen Dank im Voraus.

Gruß,
Philipp

[Matthias D. (marvin42)]

Der doppelte * ist so zu sehen, dass der Inhalt einer Speicheradresse 
gelesen wird, auf den der Pointer zeigt, das ist auch ok so.
Aber dein Problem ist wohl der "short*" Pointer, denn short ist nur 16 
Bit breit. Das wird so in einer 32-Bit Umgebung nicht klappen.
Kannst du den Code in einen "const unsigned int" ändern ?

[Philipp S. (philipp_s918)]

Vielen Dank für die schnelle Antwort!
Abändern auf "const unsigned int" konnte das Problem doch tatsächlich 
lösen!!!

Verstehe es aber trotzdem noch nicht:
1. Wieso ist "pgm_read_word(addr)" in allen Arduino Libraries für den 
STM32 als  "(*(const unsigned short *)(addr))" definiert?
2. Wieso kann  *(const unsigned short *) nicht auch eine 32bit lange 
Adresse enthalten?

[Stefan N. (stefan_n32)]

Philipp S. schrieb:
> Vielen Dank für die schnelle Antwort!
> Abändern auf "const unsigned int" konnte das Problem doch tatsächlich
> lösen!!!
>
> Verstehe es aber trotzdem noch nicht:
> 1. Wieso ist "pgm_read_word(addr)" in allen Arduino Libraries für den
> STM32 als  "(*(const unsigned short *)(addr))" definiert?

"Word" steht im allgemeinen für ein Doppelbyte, also 16 Bit. Ein 
32-Bit-Wert heißt dann "DWord". Findest du eventuell ein 
`pgm_read_dword` in den Headern?

> 2. Wieso kann  *(const unsigned short *) nicht auch eine 32bit lange
> Adresse enthalten?

`unsigned short` ist 16 bit. Die Adresse wird zunächst durch `(const 
unsigned short *)` als Zeiger auf einen solchen 16-Bit-Wert 
interpretiert. Danach wird dieser Zeiger durch das `*` links 
dereferenziert und heraus kommt verständlicherweise ein 16-Bit-Wert.

[Johann L. (gjlayde)]

Das ganze pgm_read_xxx Zeugs kommt aus der avr-gcc Welt; auf einem STM32 
braucht man sowas nicht.

> #define pgm_read_word(addr) (*(const unsigned short *)(addr))
> #define LCDML_getContent(var,id)\
> char* temp1 = (char*)pgm_read_word(&(g_LCDML_DISP_lang_lcdml_table[id])));\
> strcpy_P(var,temp1);

Die Semantik von pgm_read_xxx ist einfach:

#define pgm_read_xxx(addr) (*(addr))

vorausgesetzt die Zeiger haben die richtigen Typen.  Dementsprechend ist

pgm_read_xxx (& var) // entspricht var

const char *temp1 = & g_LCDML_DISP_lang_lcdml_table[id];

Ebenso wird strcpy_P nicht gebraucht: Einfach strcpy verwenden:

strcpy (var, & g_LCDML_DISP_lang_lcdml_table[id]);

Das alles unter der Annahme, dass dein Code correkt ist.  Hier wäre es 
hilfreich, die Deklaration von g_LCDML_DISP_lang_lcdml_table[] zu sehen.

[Philipp S. (philipp_s918)]

Vielen Dank für eure zahlreichen Lösungsvorschläge! Diese haben mir 
letztendlich den richtigen Denkanstoß gegeben, um das Problem in Gänze - 
so meine ich jedenfalls - zu verstehen.

Grundsätzlich habt ihr natürlich recht und man sollte das unnötige 
pgm_read_word komplett weglassen. Es ist auf dem STM32 gänzlich unnötig. 
Ich möchte aber dennoch den originalen Library Code so wenig wie möglich 
ändern. So bleibe ich für den Fall eines Updates kompatibel.

Um es für die Nachwelt zu dokumentieren (und für mich selbst mein 
Verständnis zu prüfen), beschreibe ich nachfolgend den Fehler nochmal im 
Detail:

Dazu vorweg erstmal eine Richtigstellung: 
g_LCDML_DISP_lang_lcdml_table[] ist eine Array, das die Startadressen zu 
den Zeichenketten für die einzelnen Menüpunkte enthält! Da ich auf einem 
32bit Prozessor arbeite, sind diese Startadressen jeweils 32bit lang.

Mit pgm_read_word(&(g_LCDML_DISP_lang_lcdml_table[id])); werden aber von 
der  Startadresse eines Menüpunkts 16 bit (=word) gelesen. Und genau 
hier liegt der Fehler! Es müssten - zumindestens für meine Prozessor 
Architektur - 32bit gelesen werden!
Anschließend wird mit (*char) aus dem falschen Adresswert noch ein 
Zeiger gemacht, der dann später in strcpy_P als Quelle für das Kopieren 
der Zeichenkette verwendet wird.

Für den Bug habe ich auch einen Github-Issue geöffnet und hoffe, dass 
das bald im offiziellen Projekt gefixed wird.

Nochmal ein ganz großes Dankeschön für eure Hilfe!!!

[Bruno V. (bruno_v)]

Ob es ein 8,16 oder 32 Bit Prozessor ist, hat mit dem Fehler nichts zu 
tun. Es geht nur darum, ob Adressen mit 8, 16 oder 32 Bit in der Tabelle 
abgelegt werden und dass man beim auslesen genau diese Anzahl an Bits 
verwendet.

Am einfachsten ist es, wenn man die natürlichen Pointer-Größen des 
Prozessors nimmt. Dann entfallen alle casts und der Compiler kann das 
prüfen.

Wenn Du Deine Daten in den ersten 64kB ablegen kannst, funktioniert der 
Code auch so.

[Sherlock 🕵🏽‍♂️ (rubbel-die-katz)]

Philipp S. schrieb:
> Ich verstehe leider nicht, was in "*(const unsigned short *)" die
> doppelte Benutzung von * bewirkt.

Gebe mir den Wert der Speicherzelle auf die der Zeiger vom Typ unsigned 
short (integer), auch bekannt als word, oder 0-65535.

Das erste * bedeutet "wert der Speicherzelle auf die der Zeiger zeigt".

Dahinter steht innerhalb der Klammern der Typ des Parameters: Zeiger auf 
unsigned short (integer)

[Johann L. (gjlayde)]

Philipp S. schrieb:
> Für den Bug habe ich auch einen Github-Issue geöffnet und hoffe, dass
> das bald im offiziellen Projekt gefixed wird.

Wenn die LIB für einen AVR geschrieben wurde, dann wurde sie eben für 
einen AVR / avr-gcc geschrieben.  Der Bug besteht dann darin, dass du 
den Code für andere Controller / Compiler verwendest.

Wie bereits oben beschreiben kannst du:

#if defined(__GNUC__) && defined(__AVR__)
#include <avr/pgmspace.h>
#else
#define PROGMEM /* empty */
#define pgm_read_byte(x) (*(x))
#define pgm_read_word(x) (*(x))
#define pgm_read_dword(x) (*(x))
#define pgm_read_qword(x) (*(x))
#define pgm_read_float(x) (*(x))
#define pgm_read_ptr(x) (*(x))
...
#endif

Das setzt voraus, dass der Code Pointer Correct ist, d.h. dass man nicht 
etwa ein Byte von zum Beispiel einem uint32_t liest per

uint32_t x;
pgm_read_byte (&x);

Genau genommen sollte der Originalcode pgm_read_ptr verwenden.  Aber das 
würde auch nicht helfen, weil:

/** \ingroup avr_pgmspace
    \def pgm_read_ptr_near(__addr)
    Read a pointer from the program space with a 16-bit (near) byte-address. */
#define pgm_read_ptr_near(__addr) \
    ((void*) __LPM_word ((uint16_t)(__addr)))

/** \ingroup avr_pgmspace
    \def pgm_read_ptr(__addr)
    Read a pointer from the program space with a 16-bit (near) byte-address. */
#define pgm_read_ptr(__addr)     pgm_read_ptr_near(__addr)

Selbst wenn man das uint16_t durch das korrekte uintptr_t ersetzt ist es 
immer noch falsch auf ARM wegen dem __LPM_word.

[Philipp S. (philipp_s918)]

Danke für die nochmalige Aufklärung. Ich hatte das ganze bereits 
korrigiert, es funktioniert jetzt auch.

Habe natürlich auch schon verstanden, dass das ganze Konstrukt aus der 
AVR Welt stammt. Allerdings muss es doch für die Arduino IDE in 
Verbindung mit dem STM32 auch eine Implementierung der pgm_read_xxx 
Funktionen geben. Genau diese wollte ich mir eben holen. Meine 
Überraschung bestand eben darin, dass diese auf 32bit Architektur nicht 
funktioniert hat.

[Harald K. (kirnbichler)]

Philipp S. schrieb:
> Allerdings muss es doch für die Arduino IDE in
> Verbindung mit dem STM32 auch eine Implementierung der pgm_read_xxx
> Funktionen geben.

Nö, wieso sollte es das? Da sind solche Funktionen komplett überflüssig, 
wie auf jedem anderen Von-Neumann-System auch.

[Bruno V. (bruno_v)]

Philipp S. schrieb:
> Meine Überraschung bestand eben darin, dass diese auf
> 32bit Architektur nicht funktioniert hat.

Wie schon beschrieben, hat es nichts mit der Architektur zu tun. Sondern 
mit der Größe der verwendeten Pointer, die unabhängig von der 
Architektur sind (auch wenn das korreliert)