Re: [PATCH] Fix crash in int8_avg_combine(). - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCH] Fix crash in int8_avg_combine().
Date
Msg-id 15003.1511668500@sss.pgh.pa.us
Whole thread Raw
In response to [PATCH] Fix crash in int8_avg_combine().  (Hadi Moshayedi <hadi@moshayedi.net>)
Responses Re: [PATCH] Fix crash in int8_avg_combine().  (Hadi Moshayedi <hadi@moshayedi.net>)
List pgsql-hackers
Hadi Moshayedi <hadi@moshayedi.net> writes:
> While doing some tests on REL_10_STABLE, I was getting run-time exceptions
> at int8_avg_combine() at the following line:
> state1->sumX = state2->sumX;
> After some debugging, I noticed that palloc()’s alignment is 8-bytes, while
> this statement (which moves a __int128 from one memory location to another
> memory location) expects 16-byte memory alignments. So when either state1
> or state2 is not 16-byte aligned, this crashes.

I believe we already dealt with this:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: REL_10_STABLE [619a8c47d] 2017-11-14 17:49:49 -0500
Branch: REL9_6_STABLE [4a15f87d2] 2017-11-14 17:49:49 -0500
Branch: REL9_5_STABLE [d4e38489f] 2017-11-14 17:49:49 -0500
   Prevent int128 from requiring more than MAXALIGN alignment.      Our initial work with int128 neglected alignment
considerations,an   oversight that came back to bite us in bug #14897 from Vincent Lachenal.   It is unsurprising that
int128might have a 16-byte alignment requirement;   what's slightly more surprising is that even notoriously lax Intel
chips  sometimes enforce that.      Raising MAXALIGN seems out of the question: the costs in wasted disk and   memory
spacewould be significant, and there would also be an on-disk   compatibility break.  Nor does it seem very practical
totry to allow some   data structures to have more-than-MAXALIGN alignment requirement, as we'd   have to push
knowledgeof that throughout various code that copies data   structures around.      The only way out of the box is to
maketype int128 conform to the system's   alignment assumptions.  Fortunately, gcc supports that via its
__attribute__(aligned())pragma; and since we don't currently support   int128 on non-gcc-workalike compilers, we
shouldn'tbe losing any platform   support this way.      Although we could have just done
pg_attribute_aligned(MAXIMUM_ALIGNOF)and   called it a day, I did a little bit of extra work to make the code more
portablethan that: it will also support int128 on compilers without   __attribute__(aligned()), if the native alignment
oftheir 128-bit-int   type is no more than that of int64.      Add a regression test case that exercises the one known
instanceof the   problem, in parallel aggregation over a bigint column.      Back-patch of commit 751804998.  The code
knownto be affected only exists   in 9.6 and later, but we do have some stuff using int128 in 9.5, so patch   back to
9.5.     Discussion: https://postgr.es/m/20171110185747.31519.28038@wrigleys.postgresql.org 

Does that solution not work for you?
        regards, tom lane


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [PATCH] Fix crash in int8_avg_combine().
Next
From: 高增琦
Date:
Subject: Re: no library dependency in Makefile?