VARATT_EXTERNAL_GET_POINTER is not quite there yet

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: VARATT_EXTERNAL_GET_POINTER is not quite there yet
Date: 2008-02-21 06:08:36
Message-ID: 3342.1203574116@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Childs 2008-02-21 07:38:01 Re: Permanent settings
Previous Message Tom Lane 2008-02-21 05:40:03 Re: Including PL/PgSQL by default