Skip site navigation (1) Skip section navigation (2)

Re: GIST code doesn't build on strict 64-bit machines

From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Oleg Bartunov <oleg(at)sai(dot)msu(dot)su>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: GIST code doesn't build on strict 64-bit machines
Date: 2004-03-29 14:45:49
Message-ID: 4068369D.9040201@sigaev.ru (view raw or flat)
Thread:
Lists: pgsql-hackers
Tom Lane wrote:
> I've just found out the hard way that Postgres doesn't even build on
> recent gcc releases for 64-bit HPPA.  The reason is that the compiler
> now notices and complains about alignment errors that will lead to
> core dump at runtime, and GIST has got some.  The particular code that
> fails to compile is in gist.c:
>                 gistentryinit(((GISTENTRY *) VARDATA(evec))[1],
>                     ((GISTENTRY *) VARDATA(evec))[0].key, r, NULL,
>                               (OffsetNumber) 0, ((GISTENTRY *) VARDATA(evec))[0].bytes, FALSE);
> 
> Since VARDATA() is at a 4-byte offset from the start of the datum, this
> is trying to overlay a GISTENTRY struct at a 4-byte boundary.  When
> compiling in 64-bit mode, Datum is 8 bytes, and so the GISTENTRY struct
> is not sufficiently well aligned.  Unlike Intel machines, the HP chips
> *require* 8-byte loads and stores to be 8-byte-aligned.

Hm.

evec is defined as
storage = (char *) palloc(n * sizeof(GISTENTRY) + MAXALIGN(VARHDRSZ));
evec = (bytea *) (storage + MAXALIGN(VARHDRSZ) - VARHDRSZ);


VARDATA is defined as:
#define VARDATA(__PTR)               VARATT_DATA(__PTR)
#define VARATT_DATA(PTR)     (((varattrib *)(PTR))->va_content.va_data)

and VARHDRSZ is
#define VARHDRSZ            ((int32) sizeof(int32))


Look follow:
VARATT_SIZEP(evec) = 2 * sizeof(GISTENTRY) + VARHDRSZ;
#define VARATT_SIZEP(_PTR)   (((varattrib *)(_PTR))->va_header)


So, if  ((varattrib *)evec)->va_content.va_data - (char*)evec == 4 then
((GISTENTRY *) VARDATA(evec))[i] is 8-byte aligned, but evec - no.

But if ((varattrib *)evec)->va_content.va_data - (char*)evec == 8 then
both evec and ((GISTENTRY *) VARDATA(evec))[i] isn't 8-byte aligned.

I don't afraid to say some rubbish :)
I wrote simple test:
#include <stdio.h>
#include "c.h"
typedef struct {
         int32   len;
         char    data[1];
} TST;

int main(int argn, char *argv[]) {
         TST t;
         TST *ptr = &t;
         printf("%d\n", (ptr->data - (char*)ptr));
         return 0;
}

It prints 4 for my Intel systems and for Alpha system, but I havn't access to 
HPUX. If result is equal to 8 on HPUX, then I suppose that replacing to
evec = (bytea *) storage
will resolve our problem (but VARHDRSZ has inconsistent definition).
But if result is 4 then we should use
evec = (bytea *) storage
and replace all VARDATA macro to something like
#define MY_VARDATA(PTR)	( ((char*)PTR) + MAXALIGN(VARHDRSZ) )

But all of this is strage for me, because we already faced to problem with 
8-bytes strict aliasing in GiST code, and we had resolved problem on Sun and 
Alpha boxes. What was it changed?



> I suppose that a correct fix involves doing MAXALIGN(VARDATA(evec)),
> but I do not know what places need to change to support this.

Its only union and picksplit user-defined methods in contrib modules.

-- 
Teodor Sigaev                                  E-mail: teodor(at)sigaev(dot)ru

In response to

Responses

pgsql-hackers by date

Next:From: Bruce MomjianDate: 2004-03-29 15:00:48
Subject: Re: Fuzzy cost comparison to eliminate redundant planning
Previous:From: Fabien COELHODate: 2004-03-29 14:12:20
Subject: Re: pg_advisor schema proof of concept

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group