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.
This commit is contained in:
Tom Lane 2008-02-23 19:11:45 +00:00
parent 870993e871
commit 9713c06319
4 changed files with 29 additions and 14 deletions

View File

@ -1,4 +1,4 @@
<!-- $PostgreSQL: pgsql/doc/src/sgml/xtypes.sgml,v 1.29 2007/05/15 17:39:54 momjian Exp $ --> <!-- $PostgreSQL: pgsql/doc/src/sgml/xtypes.sgml,v 1.30 2008/02/23 19:11:45 tgl Exp $ -->
<sect1 id="xtypes"> <sect1 id="xtypes">
<title>User-Defined Types</title> <title>User-Defined Types</title>
@ -219,7 +219,7 @@ CREATE TYPE complex (
<productname>PostgreSQL</productname> automatically provides support <productname>PostgreSQL</productname> automatically provides support
for arrays of that for arrays of that
type.<indexterm><primary>array</primary><secondary>of user-defined 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 has the same name as the base type with the underscore character
(<literal>_</>) prepended. (<literal>_</>) prepended.
</para> </para>
@ -240,15 +240,16 @@ CREATE TYPE complex (
If the values of your data type vary in size (in internal form), you should If the values of your data type vary in size (in internal form), you should
make the data type <acronym>TOAST</>-able (see <xref make the data type <acronym>TOAST</>-able (see <xref
linkend="storage-toast">). You should do this even if the data are always linkend="storage-toast">). You should do this even if the data are always
too small to be compressed or stored externally because too small to be compressed or stored externally, because
<productname>Postgres</> can save space on small data using <acronym>TOAST</> can save space on small data too, by reducing header
<acronym>TOAST</> as well. overhead.
</para> </para>
<para> <para>
To do this, the internal representation must follow the standard layout for 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> variable-length data: the first four bytes must be a <type>char[4]</type>
which is never accessed directly (customarily named <literal>vl_len_</>). You 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 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 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 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 to avoid some of the overhead of <function>PG_DETOAST_DATUM</>. You can use
<function>PG_DETOAST_DATUM_PACKED</> instead (customarily hidden by <function>PG_DETOAST_DATUM_PACKED</> instead (customarily hidden by
defining a <function>GETARG_DATATYPE_PP</> macro) and using the macros 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 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 type definition specifies an alignment. If the alignment is important you
must go through the regular <function>PG_DETOAST_DATUM</> interface. must go through the regular <function>PG_DETOAST_DATUM</> interface.
</para> </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> <para>
For further details see the description of the For further details see the description of the
<xref linkend="sql-createtype" endterm="sql-createtype-title"> command. <xref linkend="sql-createtype" endterm="sql-createtype-title"> command.

View File

@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.81 2008/01/01 19:45:46 momjian Exp $ * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.82 2008/02/23 19:11:45 tgl Exp $
* *
* *
* INTERFACE ROUTINES * INTERFACE ROUTINES
@ -65,7 +65,8 @@
#define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \ #define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \
do { \ do { \
varattrib_1b_e *attre = (varattrib_1b_e *) (attr); \ 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)); \ memcpy(&(toast_pointer), VARDATA_EXTERNAL(attre), sizeof(toast_pointer)); \
} while (0) } while (0)

View File

@ -12,7 +12,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/c.h,v 1.222 2008/01/01 19:45:56 momjian Exp $ * $PostgreSQL: pgsql/src/include/c.h,v 1.223 2008/02/23 19:11:45 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -410,7 +410,7 @@ typedef struct
*/ */
struct varlena 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]; char vl_dat[1];
}; };

View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/utils/inet.h,v 1.28 2008/01/01 19:45:59 momjian Exp $ * $PostgreSQL: pgsql/src/include/utils/inet.h,v 1.29 2008/02/23 19:11:45 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -49,7 +49,7 @@ typedef struct
*/ */
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_struct inet_data;
} inet; } inet;