Re: BUG #14897: Segfault on statitics SQL request

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>, Michael Paquier <michael.paquier@gmail.com>, Vincent Lachenal <vincent.lachenal@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Date: 2017-11-11T18:11:45Z
Lists: pgsql-bugs
On 2017-11-11 12:54:29 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2017-11-11 12:41:40 -0500, Tom Lane wrote:
> >> Yeah, changing MAXALIGN is out of the question.  I'm thinking about
> >> another flag bit for MemoryContextAllocExtended.  Do we need to think
> >> about other use-cases besides int128?  Should we just force 16-byte
> >> alignment on all architectures, or does it need to be platform-specific?
> 
> > I'm not sure we want to
> > a) Rely on one alignment being enough for everybody.
> > b) Additionally burden already hot code paths with a growing number of
> >    alignment flag tests, and the necessary math.
> 
> Well, (a) we could have more flag bits later if there are other use-cases
> that require even stricter alignment

Right, that's my point. It's not a scalable interface.


> and (b) I do not believe that MemoryContextAllocExtended is a hot code
> path at present; there are not enough call sites.

It's not that hot, true.


> > How about a MemoryContextAllocAligned(context, size, alignto, flags)
> > that passes on most flags but adds the necessary overhead to size, and
> > padds the result appropriately?
> 
> This'd result in hard-wiring the alignment requirement at call sites,
> which I think might not be a great idea.

Well, it'd require using a macro specifying the alignment (or if we were
using a modern C dialect alignof()), not the alignment spelled out.


> For example, one plausible future use-case is "align on cacheline
> boundaries".

Unless we go to threads that doesn't seem *that* likely, given what
palloc's used for.


> I think that would be better served by a flag like
> MCXT_ALLOC_ALIGN_CACHELINE than by having the callers demand a
> specific numeric alignment value --- it'd be a lot easier to make the
> alignment match actual hardware requirements if it were being inserted
> at one specific place.

Specifying MCXT_ALLOC_ALIGN_CACHELINE rather than CACHELINE_ALIGNMENT
doesn't seem to make it meaningfully harder to adjust. Or are you
thinking of probing the hardware?

Greetings,

Andres Freund


Commits

  1. Prevent int128 from requiring more than MAXALIGN alignment.

  2. Rearrange c.h to create a "compiler characteristics" section.