Wireshark-dev: Re: [Wireshark-dev] cast increases required alignment of target type in packet-d

From: "Peter Johansson" <peterjohansson73@xxxxxxxxx>
Date: Wed, 16 Jul 2008 23:28:21 +0200
2008/7/15 Jeff Morriss <jeff.morriss.ws@gmail.com>:

Hi folks,

My Solaris/SPARC compiles (with gcc-3.4.6) die with an alignment warning
in packet-diameter.c:

> packet-diameter.c: In function `dissect_diameter_avp':
> packet-diameter.c:340: warning: cast increases required alignment of target type

(followed by several more such warnings)

The relevant code for the first warning is:

>     109 typedef struct _diam_vnd_t {
>     110         guint32 code;
>     111         GArray* vs_avps;
>     112         GArray* vs_cmds;
>     113 } diam_vnd_t;
[...]
>     126 #define VND_AVP_VS(v) ((value_string*)((v)->vs_avps->data))
[...]
>     319         const diam_vnd_t* vendor;
[...]
>     340         vendor_avp_vs = VND_AVP_VS(vendor);

GArray comes from GLIB's garray.h:

>      34 typedef struct _GArray          GArray;
[...]
>      38 struct _GArray
>      39 {
>      40   gchar *data;
>      41   guint len;
>      42 };


The Solaris buildbot is not getting this warning but I think the warning
is valid: an array of chars may not be aligned correctly to be accessed
as a 'value_string' (a guint32 followed by a pointer).  And it may cause
bus errors because GArrays make copies of the data passed in--so we have
no guarantee how it will be aligned.

Unfortunately GArrays come in only 3 flavors: with pointers to chars,
pointers to guint8s, and pointers to pointers.

Is there an easy way to solve this?

Adding an intermediate cast to void*:

#define VND_AVP_VS(v) ((value_string*)(void*)((v)->vs_avps->data))

solves the warning and it appears this is the approach used by the glib
team:

http://svn.gnome.org/viewvc/glib?view=revision&revision=6092

but I don't think it's correct.  But at the moment I also think the
example (storing gint values) used in the GArrays documentation:

http://library.gnome.org/devel/glib/unstable/glib-Arrays.html

is also not correct/safe on SPARCs.  Can someone please tell me I'm wrong?
_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
https://wireshark.org/mailman/listinfo/wireshark-dev
 
 
An easy way to solve alignment issues like this is to add a variable of the data type that is of a size equal to that of the platform's pointer size. This variable should be added as a member of a union with the original data (an array of characters in this case).
 
Example:
 
The original data layout is this:
 
   char someData[50];
 
Casting someData to a data type other than char is never safe from an alignment perspective. However if the data layout instead would be like this:
 
   union AUnion
   {
      char someData[50];
      guint64 *ensuresProperAlignmentOfSomeData;
   };
 
This will force the compiler to allocate someData aligned on an 8-byte boundary (safe to cast to any pointer on up to 64-bit platforms). Replacing guint64 with a data type that varies in size with the platform (so that it will be 32 bits on a 32 bit platform for instance) would of course be the better approach, however I cannot recall if there is such a data type in glib (there probably is one).
 
/ Peter