8000 Prevent int128 from requiring more than MAXALIGN alignment. · koderP/postgres@d4e3848 · GitHub
[go: up one dir, main page]
Skip to content

Commit d4e3848

Browse files
tglsfdctglsfdc
committed
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 int128 might 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 space would be significant, and there would also be an on-disk compatibility break. Nor does it seem very practical to try to allow some data structures to have more-than-MAXALIGN alignment requirement, as we'd have to push knowledge of that throughout various code that copies data structures around. The only way out of the box is to make type 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't be 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 portable than that: it will also support int128 on compilers without __attribute__(aligned()), if the native alignment of their 128-bit-int type is no more than that of int64. Add a regression test case that exercises the one known instance of the problem, in parallel aggregation over a bigint column. Back-patch of commit 7518049. The code known to 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
1 parent cfc1570 commit d4e3848

File tree

6 files changed

+78
-12
lines changed

6 files changed

+78
-12
lines changed

‎config/c-compiler.m4

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,11 @@ undefine([Ac_cachevar])dnl
128128
# PGAC_TYPE_128BIT_INT
129129
# ---------------------
130130
# Check if __int128 is a working 128 bit integer type, and if so
131-
# define PG_INT128_TYPE to that typename. This currently only detects
132-
# a GCC/clang extension, but support for different environments may be
133-
# added in the future.
131+
# define PG_INT128_TYPE to that typename, and define ALIGNOF_PG_INT128_TYPE
132+
# as its alignment requirement.
133+
#
134+
# This currently only detects a GCC/clang extension, but support for other
135+
# environments may be added in the future.
134136
#
135137
# For the moment we only test for support for 128bit math; support for
136138
# 128bit literals and snprintf is not required.
@@ -160,6 +162,7 @@ return 1;
160162
[pgac_cv__128bit_int=no])])
161163
if test x"$pgac_cv__128bit_int" = xyes ; then
162164
AC_DEFINE(PG_INT128_TYPE, __int128, [Define to the name of a signed 128-bit integer type.])
165+
AC_CHECK_ALIGNOF(PG_INT128_TYPE)
163166
fi])# PGAC_TYPE_128BIT_INT
164167

165168

‎configure

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13961,7 +13961,10 @@ _ACEOF
1396113961

1396213962
# Compute maximum alignment of any basic type.
1396313963
# We assume long's alignment is at least as strong as char, short, or int;
13964-
# but we must check long long (if it exists) and double.
13964+
# but we must check long long (if it is being used for int64) and double.
13965+
# Note that we intentionally do not consider any types wider than 64 bits,
13966+
# as allowing MAXIMUM_ALIGNOF to exceed 8 would be too much of a penalty
13967+
# for disk and memory space.
1396513968

1396613969
MAX_ALIGNOF=$ac_cv_alignof_long
1396713970
if test $MAX_ALIGNOF -lt $ac_cv_alignof_double ; then
@@ -14035,7 +14038,7 @@ _ACEOF
1403514038
fi
1403614039

1403714040

14038-
# Check for extensions offering the integer scalar type __int128.
14041+
# Some compilers offer a 128-bit integer scalar type.
1403914042
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __int128" >&5
1404014043
$as_echo_n < 10000 span class="pl-pds">"checking for __int128... " >&6; }
1404114044
if ${pgac_cv__128bit_int+:} false; then :
@@ -14085,6 +14088,41 @@ if test x"$pgac_cv__128bit_int" = xyes ; then
1408514088

1408614089
$as_echo "#define PG_INT128_TYPE __int128" >>confdefs.h
1408714090

14091+
# The cast to long int works around a bug in the HP C Compiler,
14092+
# see AC_CHECK_SIZEOF for more information.
14093+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking alignment of PG_INT128_TYPE" >&5
14094+
$as_echo_n "checking alignment of PG_INT128_TYPE... " >&6; }
14095+
if ${ac_cv_alignof_PG_INT128_TYPE+:} false; then :
14096+
$as_echo_n "(cached) " >&6
14097+
else
14098+
if ac_fn_c_compute_int "$LINENO" "(long int) offsetof (ac__type_alignof_, y)" "ac_cv_alignof_PG_INT128_TYPE" "$ac_includes_default
14099+
#ifndef offsetof
14100+
# define offsetof(type, member) ((char *) &((type *) 0)->member - (char *) 0)
14101+
#endif
14102+
typedef struct { char x; PG_INT128_TYPE y; } ac__type_alignof_;"; then :
14103+
14104+
else
14105+
if test "$ac_cv_type_PG_INT128_TYPE" = yes; then
14106+
{ { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
14107+
$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
14108+
as_fn_error 77 "cannot compute alignment of PG_INT128_TYPE
14109+
See \`config.log' for more details" "$LINENO" 5; }
14110+
else
14111+
ac_cv_alignof_PG_INT128_TYPE=0
14112+
fi
14113+
fi
14114+
14115+
fi
14116+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_alignof_PG_INT128_TYPE" >&5
14117+
$as_echo "$ac_cv_alignof_PG_INT128_TYPE" >&6; }
14118+
14119+
14120+
14121+
cat >>confdefs.h <<_ACEOF
14122+
#define ALIGNOF_PG_INT128_TYPE $ac_cv_alignof_PG_INT128_TYPE
14123+
_ACEOF
14124+
14125+
1408814126
fi
1408914127

1409014128
# Check for various atomic operations now that we have checked how to declare

‎configure.in

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,7 +1805,10 @@ AC_CHECK_ALIGNOF(double)
18051805

18061806
# Compute maximum alignment of any basic type.
18071807
# We assume long's alignment is at least as strong as char, short, or int;
1808-
# but we must check long long (if it exists) and double.
1808+
# but we must check long long (if it is being used for int64) and double.
1809+
# Note that we intentionally do not consider any types wider than 64 bits,
1810+
# as allowing MAXIMUM_ALIGNOF to exceed 8 would be too much of a penalty
1811+
# for disk and memory space.
18091812

18101813
MAX_ALIGNOF=$ac_cv_alignof_long
18111814
if test $MAX_ALIGNOF -lt $ac_cv_alignof_double ; then
@@ -1826,7 +1829,7 @@ AC_CHECK_TYPES([int8, uint8, int64, uint64], [], [],
18261829
# C, but is missing on some old platforms.
18271830
AC_CHECK_TYPES(sig_atomic_t, [], [], [#include <signal.h>])
18281831

1829-
# Check for extensions offering the integer scalar type __int128.
1832+
# Some compilers offer a 128-bit integer scalar type.
18301833
PGAC_TYPE_128BIT_INT
18311834

18321835
# Check for various atomic operations now that we have checked how to declare

‎src/include/c.h

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,13 +361,29 @@ typedef unsigned long long int uint64;
361361

362362
/*
363363
* 128-bit signed and unsigned integers
364-
* There currently is only a limited support for the type. E.g. 128bit
365-
* literals and snprintf are not supported; but math is.
364+
* There currently is only limited support for such types.
365+
* E.g. 128bit literals and snprintf are not supported; but math is.
366+
* Also, because we exclude such types when choosing MAXIMUM_ALIGNOF,
367+
* it must be possible to coerce the compiler to allocate them on no
368+
* more than MAXALIGN boundaries.
366369
*/
367370
#if defined(PG_INT128_TYPE)
368-
#define HAVE_INT128
369-
typedef PG_INT128_TYPE int128;
370-
typedef unsigned PG_INT128_TYPE uint128;
371+
#if defined(pg_attribute_aligned) || ALIGNOF_PG_INT128_TYPE <= F438 MAXIMUM_ALIGNOF
372+
#define HAVE_INT128 1
373+
374+
typedef PG_INT128_TYPE int128
375+
#if defined(pg_attribute_aligned)
376+
pg_attribute_aligned(MAXIMUM_ALIGNOF)
377+
#endif
378+
;
379+
380+
typedef unsigned PG_INT128_TYPE uint128
381+
#if defined(pg_attribute_aligned)
382+
pg_attribute_aligned(MAXIMUM_ALIGNOF)
383+
#endif
384+
;
385+
386+
#endif
371387
#endif
372388

373389
/*

‎src/include/pg_config.h.in

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
/* The normal alignment of `long long int', in bytes. */
2828
#undef ALIGNOF_LONG_LONG_INT
2929

30+
/* The normal alignment of `PG_INT128_TYPE', in bytes. */
31+
#undef ALIGNOF_PG_INT128_TYPE
32+
3033
/* The normal alignment of `short', in bytes. */
3134
#undef ALIGNOF_SHORT
3235

‎src/include/pg_config.h.win32

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
/* The alignment requirement of a `long long int'. */
3535
#define ALIGNOF_LONG_LONG_INT 8
3636

37+
/* The normal alignment of `PG_INT128_TYPE', in bytes. */
38+
#undef ALIGNOF_PG_INT128_TYPE
39+
3740
/* The alignment requirement of a `short'. */
3841
#define ALIGNOF_SHORT 2
3942

0 commit comments

Comments
 (0)
0