ARM-GCC/AVR-GCC: Type punning

[Walter T. (nicolas)]

Hallo zusammen,
ich habe ein struct in einer Headerdatei wie folgt definiert:

  // Diagnosedaten
  __extension__ typedef struct {
    unsigned char stepper_error    : 1;
    unsigned char stepper_initfail : 1;
    unsigned char stepper_steploss : 1;
    unsigned char stepper_readfail : 1;
    unsigned char stepper_i2cfail  : 1;
    unsigned char eeprom_error     : 1;
    uint16_t stuff : 10;    
  } __attribute__((__may_alias__)) edm_diag_t;
  
  volatile edm_diag_t gls_edm_diag;

und will in einem if-statement auslesen, ob irgendeins der Flags gesetzt 
ist. Also mache ich:

  if ( *( (uint16_t*) (&gls_edm_diag)) ) {
    goto error;
  }

Beim AVR-GCC wird dies auch klaglos angenommen. Beim ARM-GCC werde ich 
dagegen gewarnt:
 "dereferencing type-punned pointer will break strict-aliasing rules".

Habe ich irgendein Attribut vergessen oder irgendeine 
Integer-Propagation-Regel nicht beachtet?

Viele Grüße
W.T.

[Michael R. (Firma: Brainit GmbH) (fisa)]

Die Warnung hatte ich auch ab und zu, die ist "lästig" weil etwas schwer 
zu verstehen (für mich)

Eine gute Erklärung findest du hier: 
Beitrag "dereferencing type-punned pointer - was ist das?"

ich hab dann immer -fno-strict-aliasing verwendet (das waren aber andere 
Fälle)

In deinem Fall würde sich entweder eine union anbieten, oder du 
schmeisst das bitfeld ganz raus und arbeitest direkt mit den einzelnen 
Bits

#define E_STEPPER_ERROR (1<<0)
#define E_STEPPER_INITFAIL (1<<1)

[tictactoe (Gast)]

Man kann in C auf ein Objekt (und gls_edm_diag ist "ein Objekt") nur mit 
seinem tatsächlichen Typ (mit ein paar Ausnahmen, die hier aber nicht 
zutreffen) oder als ein Array von char, unsigned char oder signed char 
zugreifen. Das ganze nennt sich "strict aliasing".

Wenn du also auf ein Objekt, das vom Typ edm_diag_t ist (ich nehme jetzt 
mal an, dass gls_edm_diag so deklariert worden ist), aber mit dem Typ 
uint16_t zugreifst, dann verletzt du diese "strict aliasing rule".

Folgendes könnte funktionieren:

unsigned char* x = (unsigned char*)&gls_edm_diag;
if (x[0] || x[1])
    goto error

[Jörg W. (dl8dtl) (Moderator)]

Walter Tarpan schrieb:
> Beim AVR-GCC wird dies auch klaglos angenommen.

Da derartige Warnungen ein allgemeines GCC-Feature (der jeweiligen
Version) sind, heißt das vermutlich nur, dass dein AVR-GCC (sehr) viel
älter ist als dein ARM-GCC.

Du solltest die Warnung ernst nehmen und es einfach korrekt machen.
Die bereits genannte union scheint in diesem Falle ein sinnvolles
Mittel zu sein, die gewünschte Funktionalität standardkonform zu
erreichen.

[Michael R. (Firma: Brainit GmbH) (fisa)]

Walter Tarpan schrieb:
> __attribute__((_may_alias_))

Könnte auch sein dass dein ARM-GCC dieses Attribut (noch) nicht kennt?

[Walter T. (nicolas)]

Michael Reinelt schrieb:
> Könnte auch sein dass dein ARM-GCC dieses Attribut (noch) nicht kennt?

Nein, eher umgekehrt. Der ARM-GCC basiert auf GCC 4.7, der AVR-GCC auf 
4.6.1.

Also wird's mal wieder Zeit, die C-Bücher zu konsultieren.

Gibt es wohl eine Art "anonymes struct", das in einem union oder struct 
keine weitere Tiefe hinzufügt? So daß S.a, S.b, S.c nicht um eine 
weitere Ebene S.e.a, S.e.b, S.e.c ergänzt werden muß, um es als S.all 
auslesen zu können? Ich habe mich gegen das Union "damals" ja nur 
deswegen entschieden, weil das Struct in seiner jetzigen Form gut lesbar 
ist, da es die Programmlogik gut widerspiegelt. Der "if"-Fall ist 
sozusagen die einzige Stelle, wo das Struct als Gesamtheit sinnvoll 
genutzt wird.

EDIT: OK, in den Büchern, die ich bis jetzt durchsucht habe, gibt es das 
wohl nicht. Also nutze ich mal den Sonntag, um mich mit dem - bislang 
ungelesenen - K&R in die Sonne zu setzen.

[nur einmal so (Gast)]

Wer garantiert denn,dass die Bitvariablen schön hintereinander im 
Speicher sterben? Die darf der Compiler doch anordnen, wie er will.

[Jörg W. (dl8dtl) (Moderator)]

nur einmal so schrieb:
> Wer garantiert denn,dass die Bitvariablen schön hintereinander im
> Speicher sterben?

Der C-Standard, in gewissen Grenzen.  Die Reihenfolge darf der
Compiler nicht ändern, er dürfte höchstens zwischen den Feldern
padding einfügen.  Bei einem Typ "char" jedoch ist nie padding
vonnöten (per definitionem, denn es ist die kleinste für sich im
Speicher zuordenbare Einheit).

Da er aber offenbar ohnehin nur Flags braucht, ist das auch egal, wie
sie tatsächlich angeordnet sind.

Walter Tarpan schrieb:
> So daß S.a, S.b, S.c nicht um eine weitere Ebene S.e.a, S.e.b, S.e.c
> ergänzt werden muß, um es als S.all auslesen zu können?

Portabel geht das mit Makros:

#define a e.a

(Man sollte sich dann wohl einen besseren Namen als "a" ausdenken. ;-)

Andererseits, da wir hier im GCC-Forum sind: wenn du das nur mit GCC
compilieren können musst/willst, dann kannst du das hier benutzen:

http://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html

[Michael R. (Firma: Brainit GmbH) (fisa)]

Jörg Wunsch schrieb:

> Andererseits, da wir hier im GCC-Forum sind: wenn du das nur mit GCC
> compilieren können musst/willst, dann kannst du das hier benutzen:
>
> http://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html

Das ist ja cool! Danke für die info!

[Walter T. (nicolas)]

nur einmal so schrieb:
> Wer garantiert denn,dass die Bitvariablen schön hintereinander im
> Speicher sterben?

Hm, stimmt. Streng genommen müßten die Variablendeklaration und 
-Defitinion so aussehen:

  // Diagnosedaten
  __extension__ typedef struct {
    unsigned int stepper_error    : 1;
    unsigned int stepper_initfail : 1;
    unsigned int stepper_steploss : 1;
    unsigned int stepper_readfail : 1;
    unsigned int stepper_i2cfail  : 1;
    unsigned int eeprom_error     : 1;
    unsigned int stuff            : 10;
  } __attribute__((__may_alias__)) __attribute__((packed)) edm_diag_t;
  
  volatile edm_diag_t gls_edm_diag;
  
  static_assert(sizeof(edm_diag_t)==2,"miscalulated size");

da "char" als Datentyp gar nicht erlaubt ist. Und die Bitreihenfolge ist 
ja eh egal, weil ich nur testen will, ob irgendein Bit gesetzt ist.

[Walter T. (nicolas)]

Jörg Wunsch schrieb:
> Andererseits, da wir hier im GCC-Forum sind: wenn du das nur mit GCC
> compilieren können musst/willst, dann kannst du das hier benutzen:
>
> http://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html

Danke für die Info! "unnamed struct" klingt genau nach dem, was ich 
suche. Und so wahnsinnig inkompatibel scheint das auch gar nicht zu 
sein- zumindest der Keil und der MSVC (da heißen sie "anonymous struct") 
bieten das auch:

http://msdn.microsoft.com/en-us/library/z2cx9y4f.aspx
http://www.keil.com/support/man/docs/armccref/armccref_Ciajccbj.htm

Bleibt nur die Frage, wie ich jetzt noch bei dem schönen Wetter heute 
noch die Motivation kriege, die restlichen 249 Seiten des K&R zu 
lesen...

Vielen Grüße
W.T.

P.S.:
WTF?: Wenn ich das struct so definiere:

  // Diagnosedaten
  __extension__ typedef union {
      struct {
        unsigned int stepper_error    : 1;
        unsigned int stepper_initfail : 1;
        unsigned int stepper_steploss : 1;
        unsigned int stepper_readfail : 1;
        unsigned int stepper_i2cfail  : 1;
        unsigned int eeprom_error     : 1;
        unsigned int stuff            : 10;
      } __attribute__((packed));
      uint16_t bits;
  } __attribute__((__may_alias__)) __attribute__((packed)) edm_diag_t;

  volatile edm_diag_t gls_edm_diag;
  
  static_assert(sizeof(edm_diag_t)==2,"miscalulated size");

verschwindet auch das Warning zum type-punning, das der ganze Auslöser 
von der Frage war.

[Jörg W. (dl8dtl) (Moderator)]

Walter Tarpan schrieb:
> da "char" als Datentyp gar nicht erlaubt ist.

Der Standard gestattet neben _Bool, signed int und unsigned int noch
implementierungsabhängige weitere Typen.

Aber unsigned int geht natürlich genauso.

Dein attribute(packed) ist übrigens unnötig:

An implementation may allocate any addressable storage unit large enough to hold a bit-
field. If enough space remains, a bit-field that immediately follows another bit-field in a
structure shall be packed into adjacent bits of the same unit. If insufficient space remains,
whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is
implementation-defined. The order of allocation of bit-fields within a unit (high-order to
low-order or low-order to high-order) is implementation-defined. The alignment of the
addressable storage unit is unspecified.

Da du nur 1-bit-Felder hast, müssen sie also aufeinanderfolgend
gespeichert werden.  Allerdings ist das gut gemeinte 10-bit-Feld am
Ende hier ggf. der Show-Stopper: der Compiler könnte auf die Idee
kommen, die auf einer neuen Wortgrenze auszurichten.  Lass es einfach
weg, es erfüllt ohnehin keinen Zweck.

[Walter T. (nicolas)]

Jörg Wunsch schrieb:
> Lass es einfach
> weg, es erfüllt ohnehin keinen Zweck.

Du hast Recht. Im Union ist es zwecklos geworden.

Jörg Wunsch schrieb:
> Allerdings ist das gut gemeinte 10-bit-Feld am
> Ende hier ggf. der Show-Stopper

Und auch hier: Treffer! Wie Du an der letzten Zeile siehst, gehörte das 
zu auch meinen Befürchtungen und ist der Grund für das innere 
__attribute__((packed)). Ohne die Überprüfung hätte ich erst einmal 
geglaubt, daß sich das auf die gesamte union inklusive innerer struct 
auswirkt.

[Walter T. (nicolas)]

Jetzt wird es allerdings wirklich merkwürdig: Wenn ich das innere 
"attribute packed" weglasse, bekomme ich einen Fehler beim static 
assert.

  // Diagnosedaten
  __extension__ typedef union {
      struct {
        unsigned stepper_error    : 1;
        unsigned stepper_initfail : 1;
        unsigned stepper_steploss : 1;
        unsigned stepper_readfail : 1;
        unsigned stepper_i2cfail  : 1;
        unsigned eeprom_error     : 1;
      } __attribute__((packed));
      uint16_t bits;
  } __attribute__((__may_alias__)) edm_diag_t;

  volatile edm_diag_t gls_edm_diag;
  
  static_assert(sizeof(edm_diag_t)==2,"miscalulated size");

Wenn ich aus dem "unsigned int" einen "unsigned char" mache, stimmt die 
Größe wieder.

[Ben (Gast)]

Jörg Wunsch schrieb:
> Die bereits genannte union scheint in diesem Falle ein sinnvolles Mittel zu
> sein, die gewünschte Funktionalität standardkonform zu erreichen.

Ich dachte, es dürfe nur lesend auf das union-Member zugegriffen werden 
das zuletzt beschrieben wurde.
Ansonsten wird doch auch die strict-aliasing Regel verletzt?!

[Johann L. (gjlayde)]

Ben schrieb:
> Jörg Wunsch schrieb:
>> Die bereits genannte union scheint in diesem Falle ein sinnvolles Mittel zu
>> sein, die gewünschte Funktionalität standardkonform zu erreichen.
>
> Ich dachte, es dürfe nur lesend auf das union-Member zugegriffen werden
> das zuletzt beschrieben wurde.

Das ist eine GCC-Erweiterung: Man kann ein Union-Element lesen, auch 
wenn inzwischen ein anderes Member geschrieben wurde. Das steht im 
Kleingedruckten:

Casting does not work as expected when optimization is turned on.
 
   This is often caused by a violation of aliasing rules, which are
   part of the ISO C standard.

   ...

   To fix the code above, you can use a union instead of a cast 
   (note that this is a GCC extension which might not work with
   other compilers)

http://gcc.gnu.org/bugs/#nonbugs_c

[Jörg W. (dl8dtl) (Moderator)]

Walter Tarpan schrieb:
> Wenn ich aus dem "unsigned int" einen "unsigned char" mache, stimmt die
> Größe wieder.

Wenn du die Bitfelder als "unsigned int" deklarierst, belegt der
Compiler mindestens die Größe eines unsigned int für die ganze
union.