Change the declaration of struct varlena so that the length word is

represented as "char ...[4]" not "int32".  Since the length word is never
supposed to be accessed via this struct member anyway, this won't break
any existing code that is following the rules.  The advantage is that C
compilers will no longer assume that a pointer to struct varlena is
word-aligned, which prevents incorrect optimizations in TOAST-pointer
access and perhaps other places.  gcc doesn't seem to do this (at least
not at -O2), but the problem is demonstrable on some other compilers.

I changed struct inet as well, but didn't bother to touch a lot of other
struct definitions in which it wouldn't make any difference because there
were other fields forcing int alignment anyway.  Hopefully none of those
struct definitions are used for accessing unaligned Datums.

diff --git a/doc/src/sgml/xtypes.sgml b/doc/src/sgml/xtypes.sgml
index 4856305e9fb215b48106b7e219248da4910eae35..0b650ab97622fe1dc1967eb984fc278e0d31dd0c 100644 (file)
--- a/doc/src/sgml/xtypes.sgml
+++ b/doc/src/sgml/xtypes.sgml
@@ -219,7 +219,7 @@ CREATE TYPE complex (
   <productname>PostgreSQL</productname> automatically provides support
   for arrays of that
   type.<indexterm><primary>array</primary><secondary>of user-defined
-  type</secondary></indexterm> For historical reasons, the array type
+  type</secondary></indexterm>  The array type typically
   has the same name as the base type with the underscore character
   (<literal>_</>) prepended.
  </para>
@@ -240,15 +240,16 @@ CREATE TYPE complex (
   If the values of your data type vary in size (in internal form), you should
   make the data type <acronym>TOAST</>-able (see <xref
   linkend="storage-toast">). You should do this even if the data are always
-  too small to be compressed or stored externally because
-  <productname>Postgres</> can save space on small data using
-  <acronym>TOAST</> as well. 
+  too small to be compressed or stored externally, because
+  <acronym>TOAST</> can save space on small data too, by reducing header
+  overhead.
  </para>
 
  <para>
   To do this, the internal representation must follow the standard layout for
-  variable-length data: the first four bytes must be an <type>int32</type>
-  which is never accessed directly (customarily named <literal>vl_len_</>). You
+  variable-length data: the first four bytes must be a <type>char[4]</type>
+  field which is never accessed directly (customarily named
+  <structfield>vl_len_</>). You
   must use <function>SET_VARSIZE()</function> to store the size of the datum
   in this field and <function>VARSIZE()</function> to retrieve it. The C
   functions operating on the data type must always be careful to unpack any
@@ -265,12 +266,25 @@ CREATE TYPE complex (
   to avoid some of the overhead of <function>PG_DETOAST_DATUM</>. You can use
   <function>PG_DETOAST_DATUM_PACKED</> instead (customarily hidden by
   defining a <function>GETARG_DATATYPE_PP</> macro) and using the macros
-  <function>VARSIZE_ANY_EXHDR</> and <function>VARDATA_ANY</> macros.
+  <function>VARSIZE_ANY_EXHDR</> and <function>VARDATA_ANY</> to access
+  a potentially-packed datum.
   Again, the data returned by these macros is not aligned even if the data
   type definition specifies an alignment. If the alignment is important you
   must go through the regular <function>PG_DETOAST_DATUM</> interface.
  </para>
 
+ <note>
+  <para>
+   Older code frequently declares <structfield>vl_len_</> as an
+   <type>int32</> field instead of <type>char[4]</>.  This is OK as long as
+   the struct definition has other fields that have at least <type>int32</>
+   alignment.  But it is dangerous to use such a struct definition when
+   working with a potentially unaligned datum; the compiler may take it as
+   license to assume the datum actually is aligned, leading to core dumps on
+   architectures that are strict about alignment.
+  </para>
+ </note>
+
  <para>
   For further details see the description of the
   <xref linkend="sql-createtype" endterm="sql-createtype-title"> command.

diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 2dcf0535b9b700552758094963776b8683e48f3a..a4283dbacedbb17787dbad83dee06add780defaf 100644 (file)
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -65,7 +65,8 @@
 #define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \
 do { \
        varattrib_1b_e *attre = (varattrib_1b_e *) (attr); \
-       Assert(VARSIZE_ANY_EXHDR(attre) == sizeof(toast_pointer)); \
+       Assert(VARATT_IS_EXTERNAL(attre)); \
+       Assert(VARSIZE_EXTERNAL(attre) == sizeof(toast_pointer) + VARHDRSZ_EXTERNAL); \
        memcpy(&(toast_pointer), VARDATA_EXTERNAL(attre), sizeof(toast_pointer)); \
 } while (0)
 

diff --git a/src/include/c.h b/src/include/c.h
index 2ec7a88369134a45a077dd5effad2c48561fcddb..f9537cefffd359e5aa38cdcedd5d3ff81d2419b9 100644 (file)
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -410,7 +410,7 @@ typedef struct
  */
 struct varlena
 {
-       int32           vl_len_;                /* Do not touch this field directly! */
+       char            vl_len_[4];             /* Do not touch this field directly! */
        char            vl_dat[1];
 };
 

diff --git a/src/include/utils/inet.h b/src/include/utils/inet.h
index 2035cf492fbff93de0808ee094354130fe207ab0..4134bcf6f3c98c4ee32ba07cc212f9ba71267086 100644 (file)
--- a/src/include/utils/inet.h
+++ b/src/include/utils/inet.h
@@ -49,7 +49,7 @@ typedef struct
  */
 typedef struct
 {
-       int32           vl_len_;                /* Do not touch this field directly! */
+       char            vl_len_[4];             /* Do not touch this field directly! */
        inet_struct inet_data;
 } inet;