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 17615.1510630283@sss.pgh.pa.us
Whole thread Raw
In response to Re: [BUGS] BUG #14897: Segfault on statitics SQL request  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [BUGS] BUG #14897: Segfault on statitics SQL request  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-bugs
I wrote:
> 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.)

Concretely, the attached patch fixes it for me.  I've verified by
examining the assembly code that this stops gcc from using movdqa or
movaps in numeric.c, except for one place where it apparently can
prove that it's dealing with a sufficiently-aligned local variable.

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.

BTW, for me, gcc only seems to want to generate 16-aligned instructions
in functions used for parallel aggregation.  That may explain why we've
not seen a report before: the vulnerable code exists in 9.6, but it's not
used by default, since parallelization isn't on by default.

            regards, tom lane

diff --git a/src/include/c.h b/src/include/c.h
index 852551c121..19574a1a16 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -301,17 +301,6 @@ typedef unsigned long long int uint64;
 #define UINT64_FORMAT "%" INT64_MODIFIER "u"

 /*
- * 128-bit signed and unsigned integers
- *        There currently is only a limited support for the type. E.g. 128bit
- *        literals and snprintf are not supported; but math is.
- */
-#if defined(PG_INT128_TYPE)
-#define HAVE_INT128
-typedef PG_INT128_TYPE int128;
-typedef unsigned PG_INT128_TYPE uint128;
-#endif
-
-/*
  * stdint.h limits aren't guaranteed to be present and aren't guaranteed to
  * have compatible types with our fixed width types. So just define our own.
  */
@@ -642,6 +631,25 @@ typedef NameData *Name;
 #define pg_attribute_noreturn()
 #endif

+/*
+ * 128-bit signed and unsigned integers
+ *        There currently is only a limited support for the type. E.g. 128bit
+ *        literals and snprintf are not supported; but math is.
+ */
+#if defined(PG_INT128_TYPE)
+#define HAVE_INT128
+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
+

 /*
  * Forcing a function not to be inlined can be useful if it's the slow path of

pgsql-bugs by date:

Previous
From: xkevin.hk@gmail.com
Date:
Subject: BUG #14905: can't restart successfully because of existed pid file ofhttpd
Next
From: Michael Paquier
Date:
Subject: Re: [BUGS] BUG #14897: Segfault on statitics SQL request