VARATT_EXTERNAL_GET_POINTER is not quite there yet - Mailing list pgsql-hackers

From Tom Lane
Subject VARATT_EXTERNAL_GET_POINTER is not quite there yet
Date
Msg-id 3342.1203574116@sss.pgh.pa.us
Whole thread Raw
Responses Re: VARATT_EXTERNAL_GET_POINTER is not quite there yet  (Gregory Stark <stark@enterprisedb.com>)
List pgsql-hackers
I got around to testing PG 8.3 on HPUX on Itanium (feel free to play
along at www.testdrive.hp.com) ... and was dismayed to find that it
doesn't work.  If compiled with HP's C compiler, the regression tests
dump core.  Investigation shows that the problem occurs where
tuptoaster.c tries to copy a misaligned toast pointer datum into a
properly aligned local variable: it's using word-wide load and store
instructions to do that copying, which of course does not work on any
architecture that's picky about alignment.

We'd seen this before and tried to fix it by introducing a pointer cast
within VARATT_EXTERNAL_GET_POINTER(), but evidently that's not enough
for some non-gcc compilers.

After much experimentation I was able to get it to work by invoking
memcpy through a function pointer, which seems to be sufficient to
disable this particular compiler's built-in intelligence about memcpy.
I can't say that I find this a nice clean solution; but does anyone have
a better one?

The full patch that I'm thinking of applying is

*** src/backend/access/heap/tuptoaster.c.orig    Tue Jan  1 14:53:12 2008
--- src/backend/access/heap/tuptoaster.c    Wed Feb 20 20:28:13 2008
***************
*** 65,72 **** #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)); \
!     memcpy(&(toast_pointer), VARDATA_EXTERNAL(attre), sizeof(toast_pointer)); \ } while (0)  
--- 65,74 ---- #define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \ do { \     varattrib_1b_e *attre =
(varattrib_1b_e*) (attr); \
 
!     void *(*mcopy) (void *dest, const void *src, size_t sz) = memcpy; \
!     Assert(VARATT_IS_EXTERNAL(attre)); \
!     Assert(VARSIZE_EXTERNAL(attre) == sizeof(toast_pointer)+VARHDRSZ_EXTERNAL); \
!     mcopy(&(toast_pointer), VARDATA_EXTERNAL(attre), sizeof(toast_pointer)); \ } while (0)

(The Assert changes aren't necessary for this particular problem, but
were added after realizing that the original Assert didn't adequately
protect the subsequent use of VARDATA_EXTERNAL.  That macro assumes that
the datum has a 1-byte header.  I had first thought that the cast to
"varattrib_4b *" that occurs within one branch of VARSIZE_ANY_EXHDR
might be giving the compiler license to think that the pointer is
word-aligned.  After further experimentation I don't think that HP's
compiler thinks so; but some other compiler might, so it seems wise to
get rid of that.)

It's all pretty ugly, but I'm afraid that we're in for shenanigans like
this as compilers get more aggressive about optimization.  Has anyone
got a better suggestion?
        regards, tom lane


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Including PL/PgSQL by default
Next
From: "Peter Childs"
Date:
Subject: Re: Permanent settings