From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dmitry Dolgov <9erthalion6@gmail.com>
cc: Andres Freund <andres@anarazel.de>,
        Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
        Michael Paquier <michael.paquier@gmail.com>,
        Vincent Lachenal <vincent.lachenal@gmail.com>,
        PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Subject: Re: [BUGS] BUG #14897: Segfault on statitics SQL request
In-reply-to: 
 <CA+q6zcWnWYX0ArC6QYC3YeRk3WnsyuexV1FzqBuBpGqODEK3fQ@mail.gmail.com>
References: <20171110185747.31519.28038@wrigleys.postgresql.org>
 <1415.1510340891@sss.pgh.pa.us>
 <CAA2tJ7o-VK+ghsxifDAszFN5fikmG6U3sWHCEvc9nAVOypfjAw@mail.gmail.com>
 <5652.1510344738@sss.pgh.pa.us>
 <CAA2tJ7qsopt0hHbsm397H62LMsaE4hjRwaUu3FQdNgo5AYAGXA@mail.gmail.com>
 <7710.1510346813@sss.pgh.pa.us>
 <CAA2tJ7odG4KB=wvu_+5FfssxckbLTnEuR9C_TXRyeU_35z+CSA@mail.gmail.com>
 <10197.1510349123@sss.pgh.pa.us>
 <CAA2tJ7p72wbuHEzOA_g2V4YcmeDPHYKggenTR4dXG=kSpCiO8Q@mail.gmail.com>
 <14075.1510352167@sss.pgh.pa.us>
 <CA+q6zcXe9aV5NV88u9Y=vMp+=fP7QOXgTyFQhzuvmCvTBvb_kQ@mail.gmail.com>
 <17617.1510355215@sss.pgh.pa.us>
 <CAB7nPqR7mWvtnALZF93oez_v03NFCJ8dXP9CNicaUDv3KjDbYw@mail.gmail.com>
 <3192.1510508352@sss.pgh.pa.us>
 <CAB7nPqReMmn3Pg942we4aK73SFSVamdM0SAUO0BZHcvzA71h+A@mail.gmail.com>
 <CA+q6zcWnWYX0ArC6QYC3YeRk3WnsyuexV1FzqBuBpGqODEK3fQ@mail.gmail.com>
Comments: In-reply-to Dmitry Dolgov <9erthalion6@gmail.com>
	message dated "Mon, 13 Nov 2017 17:35:45 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <10395.1510621709.1@sss.pgh.pa.us>
Date: Mon, 13 Nov 2017 20:08:29 -0500
Message-ID: <10396.1510621709@sss.pgh.pa.us>
Precedence: bulk

Dmitry Dolgov <9erthalion6@gmail.com> writes:
> I've managed to reproduce this on Ubuntu. I tried multiple gcc versions,
> and for gcc-5.4 everything is fine, for gcc-6.4 there is an issue.

I found out I can reproduce this on Fedora 26 as well; I'd been
fat-fingering the test somehow.

The place where it is actually crashing for me is in int8_avg_combine(),
and *the struct that is misaligned is one that has been passed back from
a worker process*.  This means that the problem is even worse than we
thought before, because there is no reasonable way that the code that
palloc'd that struct within the current process would know that it needs
more than MAXALIGN alignment.  It is not hard to think of other cases
where an int128-containing struct might get passed through code that
doesn't know what is in it.

So what I am now thinking is that the only practical answer is to stop
gcc from believing that it is safe to use 16-aligned instructions on
int128's.  (Some reading on the net suggests that the actual performance
penalty for that is minimal anyway on modern Intel chips.)

It looks like we can do that with very little code impact by attaching
alignment pragmas to the typedefs for [u]int128:

typedef PG_INT128_TYPE int128
#if defined(pg_attribute_aligned)
pg_attribute_aligned(8)
#endif
;
typedef unsigned PG_INT128_TYPE uint128
#if defined(pg_attribute_aligned)
pg_attribute_aligned(8)
#endif
;

When I do that, the coredump goes away for me.  Moreover, this seems
like an easily back-patchable fix, and we do need to do something
back to 9.5.

Now, one small problem is that c.h defines pg_attribute_aligned further
down than where these typedefs currently appear.  This seems to me to
indicate that we ought to rethink the contents, or at least the ordering,
of the "sections" in c.h.  I wonder if anyone has any strong opinions
about exactly how to do that.

			regards, tom lane