Re: [BUGS] BUG #14897: Segfault on statitics SQL request - Mailing list pgsql-bugs
From | Tom Lane |
---|---|
Subject | Re: [BUGS] BUG #14897: Segfault on statitics SQL request |
Date | |
Msg-id | 18815.1510674826@sss.pgh.pa.us Whole thread Raw |
In response to | Re: [BUGS] BUG #14897: Segfault on statitics SQL request (Michael Paquier <michael.paquier@gmail.com>) |
Responses |
Re: [BUGS] BUG #14897: Segfault on statitics SQL request
(Tom Lane <tgl@sss.pgh.pa.us>)
|
List | pgsql-bugs |
Michael Paquier <michael.paquier@gmail.com> writes: > On Tue, Nov 14, 2017 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> As I said before, I don't like moving the int128 typedefs into a section >> where they don't belong, but that's just cosmetic --- this is good enough >> for testing. > Section 3 could be moved after the section 4 listing the alignment > macros. It seems that it won't hurt to back-patch the refactoring as > well. After some further consideration of what's where in c.h, I propose that we redefine section 1 (hacks to cope with non-ANSI C compilers) as "compiler characteristics", and then move all of these items into it: "#ifdef PG_FORCE_DISABLE_INLINE" stanza all the pg_attribute_xxx() macros, also pg_noinline PG_USED_FOR_ASSERTS_ONLY maybe pg_unreachable(), although it's not insane to keep it in section 7 ditto for likely()/unlikely() If anyone's really hot to keep the "non-ANSI hacks" segregated from those, we could invent a "section 1.1" for that ... but I'd say that at least one of the stanzas that are in there now is unrelated to ANSI compatibility already. Having gotten pg_attribute_aligned in front of the int128 stanza, my vision for a full fix for the bug is: * Explicitly document somewhere that MAXALIGN ignores int128. * Have configure test for and define ALIGNOF_PG_INT128_TYPE if it defines PG_INT128_TYPE. * Set up the int128 stanza like this: #if defined(PG_INT128_TYPE) #if defined(pg_attribute_aligned) || ALIGNOF_PG_INT128_TYPE <= MAXIMUM_ALIGNOF #define HAVE_INT128 1 typedef PG_INT128_TYPE int128 #if defined(pg_attribute_aligned) pg_attribute_aligned(MAXIMUM_ALIGNOF) #endif ; typedef unsigned PG_INT128_TYPE uint128 #if defined(pg_attribute_aligned) pg_attribute_aligned(MAXIMUM_ALIGNOF) #endif ; #endif #endif That is, even if configure finds an int128 type, we'll only use it if it fits or can be made to fit into our system-wide MAXALIGN assumption. * It'd be kind of nice to insert aStaticAssertStmt(alignof(int128) <= MAXIMUM_ALIGNOF) someplace to cross-check this, but as far as I can find we aren't relying on alignof() to exist, so I don't see a good way to do it. BTW, looking at the existing uses of pg_attribute_aligned along with this example, I can't help but think that this decision: /** NB: aligned and packed are not given default definitions because they* affect code functionality; they *must* be implementedby the compiler* if they are to be used.*/ was just useless pedantry. If we're going to ifdef around it everywhere the macro is of use, why not just #define it to empty? That would certainly make the above a lot easier to read. We could add a HAVE_PG_ATTRIBUTE_ALIGNED symbol for use in #if tests. Barring objections, I'll get on with making this happen. regards, tom lane
pgsql-bugs by date: