Problem mit C struct auf ARM

[Timo .. (waswasi)]

Hallo,

ich portiere gerade meinen Code für den ENC28J60 von 8-bit 
(AtMegea1284p) auf 32-bit (AtSamD20E17).

Ich habe inzwischen alles zum Laufen gebracht, stehe jetzt aber vor 
einem mir unerklärlichen Phänomen.

In meinem Code für den NTP-Request gibt es folgende C-Structure:

// the ntp time message
struct ntp_msg {
  uint8_t flags;
  uint8_t stratum;
  uint8_t poll;
  int8_t precision;

  uint16_t root_delay[2];
  uint16_t root_dispersion[2];
  uint32_t reference_identifier;

  // timestamps:
  uint32_t reference_timestamp[2];
  uint32_t originate_timestamp[2];
  uint32_t receive_timestamp[2];
  uint32_t transmit_timestamp[2];
};

Das hat alles auf dem AtMega wunderbar funktioniert. Sobald er aber mit 
dem ARM auf eine beliebige Variable mit uint32_t zugreift bleibt er 
hängen. So sieht es anhand von UART-Ausgaben zumindest aus.

Gibt es hier irgendetwas, was ich als ARM-Neuling noch nicht weiß? Eine 
32-bit Variable sollte doch nicht zu einem solchen Fehler führen oder?

Kann mir irgendjemand hier weiterhelfen?

[Jim M. (turboj)]

Wenn da noch ein Pointer Cast dabei ist: HardFault wegen nicht aligned 
Zugriff. Cortex M0 kann auf Words (32 Bit) nur aligned (letzte beiden 
Bits gleich Null) zugreifen.

[Timo .. (waswasi)]

Aha. Das klingt ja tatsächlich nach einer Arm (ist ein Cortex-m0+) 
Besonderheit.

Ich greife z.B über ntp_msg->reference_identifier zu.

Hier wird ja der Pointer auf die struct dereferenziert. Wenn ich dich 
richtig verstanden habe führt das bei 32bit Membern zu einem Fehler?

Wie sollte ich es dann machen??

[(prx) A. K. (prx)]

Der Ethernet Header ist 14 Bytes lang. Ist der aligned, ist es der 
Inhalt nicht.

[Lothar (Gast)]

Jim M. schrieb:
> HardFault wegen nicht aligned Zugriff

Die struct ist doch aligned also durch 4 teilbar. Selbst eine 
"gefährliche" Aktion wie (uint32_t*)& sollte funktionieren.

Im Zweifel kann man ja eine LED anschalten um zu sehen ob wirklich 
HardFault kommt:

void HardFault_Handler(void)
{
  Panic_LED();
  while (1);
}

Aber wieder ein Grund mehr statt M0 besser M3 zu nehmen, ist 
schliesslich nicht teurer.

[Torsten C. (torsten_c)]

Allein schon wegen der Endianess sind an solchen Stellen im C-Code 
Byte-Arrays angebracht.

[(prx) A. K. (prx)]

Lothar schrieb:
> Die struct ist doch aligned also durch 4 teilbar. Selbst eine
> "gefährliche" Aktion wie (uint32_t*)& sollte funktionieren.

Nicht wenn es sich um den Inhalt eines TCP oder UDP Paketes in einem 
Ethernet Frame handelt. Manche sorgen deshalb dafür, dass der Ethernet 
Frame gezielt um 2 Bytes misaligned ist.

[(prx) A. K. (prx)]

Torsten C. schrieb:
> Allein schon wegen der Endianess sind an solchen Stellen im C-Code
> Byte-Arrays angebracht.

Eher nicht. Typisch ist vielmehr die Verwendung der üblichen 
Socket-Funktionen ntohl/htonl etc. Und natürlich passendes Alignment der 
struct im Frame.

[Hanswurst (Gast)]

Timo .. schrieb:
> Aha. Das klingt ja tatsächlich nach einer Arm (ist ein Cortex-m0+)
> Besonderheit.
>
> Ich greife z.B über ntp_msg->reference_identifier zu.
>
> Hier wird ja der Pointer auf die struct dereferenziert. Wenn ich dich
> richtig verstanden habe führt das bei 32bit Membern zu einem Fehler?

Nicht grundsätzlich! Wenn wir von Variablen mit dieser Struktur reden, 
die im C-Code definiert wurden, gibt es kein Problem, denn der Compiler 
wird sie auch wieder 32-Bit-aligned anordnen.

Aber die andere, und hier naheliegende, Möglichkeit ist das Du so eine 
Struktur vom Netzwerk empfängst und "irgendwo" in einem Buffer, - im 
allgemeinsten Fall ein char array -, speicherst. Wenn keine besonderen 
Maßnahmen vorgesehen sind, - und dafür spricht, dass der Code von einem 
AVR portiert wird, der das Problem ja nicht hat -, dann kann es sein, 
dass die Bytefolge der Struktur im Buffer nicht an einer 32-Bit-aligned 
Adresse beginnt. Ein cast dieser Adresse und der Versuch dann auf die 
Strukturmitglieder zuzugreifen, führt dann zu einem unaligned access.

An sich aber solltest Du versuchen den Fehler zumindest grob 
einzukreisen.
Strukturen allein sind nie Fehlerursache und wir können da unmöglich 
irgendwas zu sagen. Das unaligned ist ja auch nur eine Vermutung.

[Timo .. (waswasi)]

Ok Danke schonmal für die interessanten Antworten. Wenn ich von der 
Arbeit zu Hause bin werde ich als erstes mal den HardFault_Handler 
implementieren um zu sehen dass er hier wirklich abschmiert.

Melde mich dann nochmal...

[Jay (Gast)]

A. K. schrieb:
> Torsten C. schrieb:
>> Allein schon wegen der Endianess sind an solchen Stellen im C-Code
>> Byte-Arrays angebracht.
>
> Eher nicht. Typisch ist vielmehr die Verwendung der üblichen
> Socket-Funktionen ntohl/htonl etc. Und natürlich passendes Alignment der
> struct im Frame.

Byte-Arrays sind schon die richtige Lösung.

Der Code stammt vermutlich aus avr-uip. Der wichtige fehlende Teil ist 
vermutlich:

/*---------------------------------------------------------------------------*/
static void
send_request(void)
{
  struct ntp_time time;
  struct ntp_msg *m = (struct ntp_msg *)uip_appdata;

  // build ntp request
  m->flags = FLAGS_LI_ALARM | FLAGS_VN(4) | FLAGS_MODE_CLIENT;
  m->stratum = 0;                    // unavailable
  m->poll = 4;                       // 2^4 = 16 sec
  m->precision = -6;                 // 2^-6 = 0.015625 sec = ~1/100 sec

  m->root_delay[0] = HTONS(1);       // 1.0 sec
  m->root_delay[1] = 0;

  m->root_dispersion[0] = HTONS(1);  // 1.0 sec
  m->root_dispersion[1] = 0;

  // we don't have a reference clock
  m->reference_identifier = 0;
  m->reference_timestamp[0] = m->reference_timestamp[1] = 0;

  // we don't know any time
  m->originate_timestamp[0] = m->originate_timestamp[1] = 0;
  m->receive_timestamp[0]   = m->receive_timestamp[1]   = 0;

  // put our own time into the transmit frame (whatever our own time is)
  clock_get_ntptime(&time);
  m->transmit_timestamp[0]  = ntp_read_u32(time.seconds);
  m->transmit_timestamp[1]  = ntp_read_u32(time.fraction);

  // and finally send request
  uip_send(uip_appdata, sizeof(struct ntp_msg));
}

Die struct wird zwar mit hton-ähnlichen Funktionen gefüllt, aber dann 
1:1 rausgehauen :-( Letzteres ist ein Kardinalfehler. Schlimmer ist 
eigentlich nur noch das Lesen von Datenpaketen direkt in eine struct.

Warum das direkte Schreiben ein Kardinalfehler ist zeigt das Beispiel 
schön. Es funktioniert nicht überall, man liefert sich dem Compiler aus 
und darf hoffen, dass er es hinter den Kulissen zufällig richtig macht. 
Wenn man es unbedingt machen möchte sollte man unbedingt zusehen dass 
für die struct Padding ausgeschaltet ist.

[Daniel A. (daniel-a)]

Jay schrieb:
> Die struct wird zwar mit hton-ähnlichen Funktionen gefüllt, aber dann
> 1:1 rausgehauen :-( Letzteres ist ein Kardinalfehler. Schlimmer ist
> eigentlich nur noch das Lesen von Datenpaketen direkt in eine struct.

Das sehe ich anders. Wenn man jeweils 2 structs hat, eines mit 
__atribute__((packed)) fürs Senden und Empfangen, und eines für die 
Interne verarbeitung, macht dass die Sache massiv einfacher zu lesen.

[Jay (Gast)]

Daniel A. schrieb:
> Das sehe ich anders. Wenn man jeweils 2 structs hat, eines mit
> __atribute__((packed)) fürs Senden und Empfangen, und eines für die
> Interne verarbeitung, macht dass die Sache massiv einfacher zu lesen.

Bis du dann merkst, dass der gerade verwendete Compiler nichts von dem 
Attribut weiß, oder wie im Fall den wir hier diskutieren, es vergessen 
wurde.

Beim Empfangen gibt es noch den Extra-Spaß, dass das direkte Lesen in 
eine Datenstruktur ein beliebter Ansatz für Hacker ist. Denn dabei wird 
gerne viel vergessen und geschlampt. Das "massiv einfacher" führt dazu 
dass  viele Programmierer glauben nichts weiter als casten zu müssen. 
Hingegen den Empfangspuffer als Byte-Array zu betrachten zwingt dazu 
jeden einzelnen Wert "in die Hand" zu nehmen. Da ist es viel schwerer 
notwendige Tests zu übersehen.

[Daniel A. (daniel-a)]

@Jay (Gast)

Casts sind keine notwendig, wenn man es richtig macht. Hier mal ein 
Beispiel, wie würdest du das sicherer ohne structs machen? Die 
Arraygrenzen muss man ja trozdem Prüfen:

 // ungetestet
struct bla_serialized {
  uint8_t a;
  uint16_t b;
  char name[10];
  uint16_t len;
  uint8_t data[];
} __attribute((packed));

struct bla {
  uint16_t b;
  uint16_t len;
  uint8_t a;
  char name[10];
  uint16_t buffer_max;
  void* data;
};

bool deserialize_bla( void* buffer, size_t length, struct bla* res ){

  if( !res || !res->data && res->buffer_max
   || length < sizeof(struct bla_serialized)
  ) return false;

  struct bla_serialized* s = buffer;
  long data_len = ntoh(s->len);
  if( s->buffer_max < data_len
   || length < data_len + sizeof(struct bla_serialized)
  ) return false;

  res->len = data_len;
  res->a = s->a;
  res->b = ntoh(s->b);
  memcpy(res->name,s->name,sizeof(res->name));
  memcpy(res->data,s->data,res->len);

  return true;
}

[Bernd K. (prof7bit)]

Jay schrieb:
> Es funktioniert nicht überall, man liefert sich dem Compiler aus und
> darf hoffen, dass er es hinter den Kulissen zufällig richtig macht

1. Da wirkt kein Zufall mit, das ist alles aufs penibelste und haarklein 
definiert in der jeweiligen ABI.

2. Wenn er zusieht dass alle struct-member ihrer jeweiligen Größe 
entsprechend aligned sind und zusieht dass das für das ganze struct 
ebenfalls gilt (also unbedachtes Casten von Pointern mit kleineren 
Alignment auf solche mit größerem Alignment möglichst vermeiden) dann 
gibts weder Padding noch Hardfaults noch sonstige Überraschungen.

3. Packed ist Pfusch und sollte um jeden Preis gemieden werden, 
stattdessen Empfehlungen aus 2 anwenden.

[(prx) A. K. (prx)]

Jay schrieb:
>> Typisch ist vielmehr die Verwendung der üblichen
>> Socket-Funktionen ntohl/htonl etc. Und natürlich passendes Alignment der
>> struct im Frame.
>
> Byte-Arrays sind schon die richtige Lösung.

Kann man natürlich so machen, es geht beides. Ich habe freilich schon 
genug existierenden Socket Code aus Unix&Co gesehen. Die erwähnten 
Funktionen für die Byteorder gibt es darin nicht zufällig. Strukturen 
für Networking sind üblicherweise so definiert, dass dies geht, ausser 
eben Ethernet selbst.

Ist auch eine recht effiziente Methode, wenngleich das hier keine Rolle 
spielen dürfte. ARM/PPC/MIPS Prozessoren mit Schwerpunkt im Networking 
sind deshalb gerne big endian, um diese Byteorder-Funktionen zu 
Leermakros zu machen.

[Timo .. (waswasi)]

Hallo,

also zu Jays 1. Beitrag. Ja den Code habe ich aus avr-uip übernommen. 
Ich hatte aber nicht damit gerechnet dass sich hier ein solcher Fehler 
beim Portieren einstellt :).

Ich werde es jetzt wohl so machen dass ich direkt aus dem Buffer 
uip_appdata (hier uint8_t[300]) die Werte rauslese. Das ist zwar sehr 
viel mühsamer, aber die struct zu packen scheint ja auch nicht als 
astrein durchzugehen.

So wird man den Fehler wohl am sichersten umgehen können.

Danke nochmal für die Anmerkungen, man lernt nicht aus :) ...

[Timo .. (waswasi)]

Eine Frage hätte ich übrigens noch. Reicht es wenn ich alle structs mit 
32 bit Membern umstelle oder muss ich auch alle structs mit 16 bit 
Membern umstellen?

Wenn ich mich recht erinnere wird dieses casten auf struct an so einigen 
Stellen verwendet im originalen uip-code.

[(prx) A. K. (prx)]

Timo .. schrieb:
> Eine Frage hätte ich übrigens noch. Reicht es wenn ich alle structs mit
> 32 bit Membern umstelle oder muss ich auch alle structs mit 16 bit
> Membern umstellen?

Wenn schon denn schon. Wenn du passendes Alignment der Daten nicht 
sicherstellen kannst, dann müssen alle Daten größer als ein Byte 
umgestellt werden. Andernfalls hängt dein Code dann doch wieder von 
Annahmen über das Alignment ab und du könntest ebenso den erwähnten 
Offset von 2 Bytes verwenden.

[Bernd K. (prof7bit)]

Timo .. schrieb:
> Eine Frage hätte ich übrigens noch. Reicht es wenn ich alle structs mit
> 32 bit Membern umstelle oder muss ich auch alle structs mit 16 bit
> Membern umstellen?

Jedes Member soll an einem vielfachen seiner eigenen Größe zu liegen 
kommen. Also zum Beispiel:

// ich habe eine Leerzeile eingefügt alle 8 Bytes zur Übersicht:

int8_t
int8_t
int16_t
int32_t

int32_t
int16_t
int16_t

int64_t

aber zum Beispiel NICHT

int8_t    // ok
int8_t    // ok
int32_t   // falsch, offset 2 ist keine 4-er Zahl, Compiler würde zwei Bytes Padding davor einfügen
int16_t   // wieder ok, aber das Kind ist weiter oben schon in den Brunnen gefallen
int64_t   // wieder falsch wegen obigem Padding nun an Offset 10, ist keine 8-er-Zahl, nochmal 6 Bytes Padding :-(

obiges einfach passend umsortieren dann gehts:

int8_t    // offset 0 ist durch 1 teilbar, OK
int8_t    // offset 1 ist durch 1 teilbar, OK
int16_t   // offset 2 ist durch 2 teilbar, OK
int32_t   // offset 4 ist durch 4 teilbar, OK
int64_t   // offset 8 ist durch 8 teilbat, OK

Weiterführende Literatur:
http://www.catb.org/esr/structure-packing/
Eric S. Raymond - The Lost Art of C Structure Packing

[Timo .. (waswasi)]

Danke, habe es nun denke ich verstanden.


Der Link sieht ganz nett aus, werde ich mir mal wenn ich Zeit habe 
genauer durchlesen.

[Arc N. (arc)]

Beim gcc wäre eine Option -munaligned-access bzw. -mno-unaligned-access 
je nachdem was gewünscht ist.
https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html

Beispiel aus einem Stack Overflow-Thread:
http://stackoverflow.com/questions/34951458/why-does-gcc-produce-illegal-unalign-accesses-for-arm-cortex-a9

struct B
{
    char c1;
    char c2;
    char c3;
    char c4;
};

struct A
{
    char c;
    struct B b;
    char c2;
};

int main(int argc, char** argv)
{
    struct A a;
    a.c2 = 42;    // Works fine
    a.b.c1 = 42   // Works fine, too
    struct B b;
    b = a.b;      // Crashes because of unaligned access
    return 0;
}

und da hat imo der gcc (4.9.2) schlicht Mist gebaut, da das absolut 
valides C ist.

-Wall und -Wextra schalten übrigens nicht die entsprechende Warnung 
-Wcast-align an...
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html