On 11/15/17 15:13, Peter Eisentraut wrote:
> I'm going to put this patch set as Returned With Feedback for now. The
> GinNullCategory issues look like they will need quite a bit of work.
> But it will be worth picking this up some time.
I think the issue with GinNullCategory is practically unfixable. This
is on-disk data that needs to be castable to an array of bool. So
tolerating a bool of size other than 1 would either require a disk
format change or extensive code changes, neither of which seem
worthwhile at this point.
So here is a minimal patch set to perhaps wrap this up for the time
being. I have added static assertions that check the sizes of
GinNullCategory and GinTernaryValue, which I think are the two critical
places that require compatibility with bool. And then we include
<stdbool.h> only if its bool has size 1.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From c9b5eb5024fcfc48bee93d621778dcacba8dd575 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 15 Dec 2017 17:01:03 -0500
Subject: [PATCH v4 1/3] Add static assertions about size of GinNullCategory vs
bool
We need these to be the same because some code uses an array of one as
the other. If bool were a different size (e.g., because stdbool.h is
used, which could make sizeof(bool) == 4 in some configurations), then
this would silently break. Since GinNullCategory is signed char and
bool is a char of unspecified signedness, there need to be explicit
casts between the two, otherwise the compiler would complain. But those
casts also silently hide any size mismatches.
---
src/backend/access/gin/ginscan.c | 6 +++++-
src/backend/access/gin/ginutil.c | 7 ++++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 7ceea7a741..eaf4ae6fdf 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -367,8 +367,12 @@ ginNewScanKey(IndexScanDesc scan)
}
}
}
- /* now we can use the nullFlags as category codes */
+ /*
+ * now we can use the nullFlags as category codes
+ */
+ StaticAssertStmt(sizeof(GinNullCategory) == sizeof(bool),
+ "sizes of GinNullCategory and
bool are not equal");
ginFillScanKey(so, skey->sk_attno,
skey->sk_strategy, searchMode,
skey->sk_argument, nQueryValues,
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index d9c6483437..28912e416a 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -540,7 +540,12 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum,
for (i = 0; i < *nentries; i++)
nullFlags[i] = (nullFlags[i] ? true : false);
}
- /* now we can use the nullFlags as category codes */
+
+ /*
+ * now we can use the nullFlags as category codes
+ */
+ StaticAssertStmt(sizeof(GinNullCategory) == sizeof(bool),
+ "sizes of GinNullCategory and bool are
not equal");
*categories = (GinNullCategory *) nullFlags;
/*
base-commit: 7d3583ad9ae54b44119973a9d6d731c9cc74c86e
--
2.15.1
From b1b89da6ccae2778c4d2561ffd3ba522c56586a4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 19 Dec 2017 13:54:05 -0500
Subject: [PATCH v4 2/3] Add static assertions about size of GinTernaryValue vs
bool
We need these to be the same because some code casts between pointers to
them.
---
src/backend/utils/adt/tsginidx.c | 4 +++-
src/include/access/gin.h | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c
index aba456ed88..23b98a3d50 100644
--- a/src/backend/utils/adt/tsginidx.c
+++ b/src/backend/utils/adt/tsginidx.c
@@ -309,7 +309,9 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS)
* query.
*/
gcv.first_item = GETQUERY(query);
- gcv.check = check;
+ StaticAssertStmt(sizeof(GinTernaryValue) == sizeof(bool),
+ "sizes of GinTernaryValue and
bool are not equal");
+ gcv.check = (GinTernaryValue *) check;
gcv.map_item_operand = (int *) (extra_data[0]);
gcv.need_recheck = recheck;
diff --git a/src/include/access/gin.h b/src/include/access/gin.h
index ec83058095..19fdf132b4 100644
--- a/src/include/access/gin.h
+++ b/src/include/access/gin.h
@@ -51,8 +51,8 @@ typedef struct GinStatsData
/*
* A ternary value used by tri-consistent functions.
*
- * For convenience, this is compatible with booleans. A boolean can be
- * safely cast to a GinTernaryValue.
+ * This must be of the same size as a bool because some code will cast a
+ * pointer to a bool to a pointer to a GinTernaryValue.
*/
typedef char GinTernaryValue;
--
2.15.1
From 7e5db87f02129c333fb82b6913b0d1d6789c42c5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 19 Dec 2017 14:24:43 -0500
Subject: [PATCH v4 3/3] Use stdbool.h if suitable
Using the standard bool type provided by C allows some recent compilers
and debuggers to give better diagnostics. Also, some extension code and
third-party headers are increasingly pulling in stdbool.h, so it's
probably saner if everyone uses the same definition.
But PostgreSQL code is not prepared to handle bool of a size other than
1, so we keep our own old definition if we encounter a stdbool.h with a
bool of a different size. (Apparently, that includes Power CPUs and
some very old compilers that declared bool to be an enum.) We have
static assertions in critical places that check that bool is of the
right size.
---
configure | 213 ++++++++++++++++++++++++++++++++++++---------
configure.in | 7 ++
src/include/c.h | 14 ++-
src/include/pg_config.h.in | 9 ++
src/pl/plperl/plperl.h | 10 +--
5 files changed, 205 insertions(+), 48 deletions(-)
diff --git a/configure b/configure
index d9b7b8d7ec..36f5550720 100755
--- a/configure
+++ b/configure
@@ -1996,116 +1996,116 @@ $as_echo "$ac_res" >&6; }
} # ac_fn_c_check_func
-# ac_fn_c_check_member LINENO AGGR MEMBER VAR INCLUDES
-# ----------------------------------------------------
-# Tries to find if the field MEMBER exists in type AGGR, after including
-# INCLUDES, setting cache variable VAR accordingly.
-ac_fn_c_check_member ()
+# ac_fn_c_check_type LINENO TYPE VAR INCLUDES
+# -------------------------------------------
+# Tests whether TYPE exists after having included INCLUDES, setting cache
+# variable VAR accordingly.
+ac_fn_c_check_type ()
{
as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2.$3" >&5
-$as_echo_n "checking for $2.$3... " >&6; }
-if eval \${$4+:} false; then :
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
+$as_echo_n "checking for $2... " >&6; }
+if eval \${$3+:} false; then :
$as_echo_n "(cached) " >&6
else
+ eval "$3=no"
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
-$5
+$4
int
main ()
{
-static $2 ac_aggr;
-if (ac_aggr.$3)
-return 0;
+if (sizeof ($2))
+ return 0;
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
- eval "$4=yes"
-else
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
-$5
+$4
int
main ()
{
-static $2 ac_aggr;
-if (sizeof ac_aggr.$3)
-return 0;
+if (sizeof (($2)))
+ return 0;
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
- eval "$4=yes"
+
else
- eval "$4=no"
+ eval "$3=yes"
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
-eval ac_res=\$$4
+eval ac_res=\$$3
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
$as_echo "$ac_res" >&6; }
eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
-} # ac_fn_c_check_member
+} # ac_fn_c_check_type
-# ac_fn_c_check_type LINENO TYPE VAR INCLUDES
-# -------------------------------------------
-# Tests whether TYPE exists after having included INCLUDES, setting cache
-# variable VAR accordingly.
-ac_fn_c_check_type ()
+# ac_fn_c_check_member LINENO AGGR MEMBER VAR INCLUDES
+# ----------------------------------------------------
+# Tries to find if the field MEMBER exists in type AGGR, after including
+# INCLUDES, setting cache variable VAR accordingly.
+ac_fn_c_check_member ()
{
as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
-$as_echo_n "checking for $2... " >&6; }
-if eval \${$3+:} false; then :
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2.$3" >&5
+$as_echo_n "checking for $2.$3... " >&6; }
+if eval \${$4+:} false; then :
$as_echo_n "(cached) " >&6
else
- eval "$3=no"
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
-$4
+$5
int
main ()
{
-if (sizeof ($2))
- return 0;
+static $2 ac_aggr;
+if (ac_aggr.$3)
+return 0;
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
+ eval "$4=yes"
+else
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
-$4
+$5
int
main ()
{
-if (sizeof (($2)))
- return 0;
+static $2 ac_aggr;
+if (sizeof ac_aggr.$3)
+return 0;
;
return 0;
}
_ACEOF
if ac_fn_c_try_compile "$LINENO"; then :
-
+ eval "$4=yes"
else
- eval "$3=yes"
+ eval "$4=no"
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
-eval ac_res=\$$3
+eval ac_res=\$$4
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
$as_echo "$ac_res" >&6; }
eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
-} # ac_fn_c_check_type
+} # ac_fn_c_check_member
# ac_fn_c_compute_int LINENO EXPR VAR INCLUDES
# --------------------------------------------
@@ -10683,6 +10683,100 @@ fi
## Header files
##
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for stdbool.h that conforms
to C99" >&5
+$as_echo_n "checking for stdbool.h that conforms to C99... " >&6; }
+if ${ac_cv_header_stdbool_h+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+ #include <stdbool.h>
+ #ifndef bool
+ "error: bool is not defined"
+ #endif
+ #ifndef false
+ "error: false is not defined"
+ #endif
+ #if false
+ "error: false is not 0"
+ #endif
+ #ifndef true
+ "error: true is not defined"
+ #endif
+ #if true != 1
+ "error: true is not 1"
+ #endif
+ #ifndef __bool_true_false_are_defined
+ "error: __bool_true_false_are_defined is not defined"
+ #endif
+
+ struct s { _Bool s: 1; _Bool t; } s;
+
+ char a[true == 1 ? 1 : -1];
+ char b[false == 0 ? 1 : -1];
+ char c[__bool_true_false_are_defined == 1 ? 1 : -1];
+ char d[(bool) 0.5 == true ? 1 : -1];
+ /* See body of main program for 'e'. */
+ char f[(_Bool) 0.0 == false ? 1 : -1];
+ char g[true];
+ char h[sizeof (_Bool)];
+ char i[sizeof s.t];
+ enum { j = false, k = true, l = false * true, m = true * 256 };
+ /* The following fails for
+ HP aC++/ANSI C B3910B A.05.55 [Dec 04 2003]. */
+ _Bool n[m];
+ char o[sizeof n == m * sizeof n[0] ? 1 : -1];
+ char p[-1 - (_Bool) 0 < 0 && -1 - (bool) 0 < 0 ? 1 : -1];
+ /* Catch a bug in an HP-UX C compiler. See
+ http://gcc.gnu.org/ml/gcc-patches/2003-12/msg02303.html
+
http://lists.gnu.org/archive/html/bug-coreutils/2005-11/msg00161.html
+ */
+ _Bool q = true;
+ _Bool *pq = &q;
+
+int
+main ()
+{
+
+ bool e = &s;
+ *pq |= q;
+ *pq |= ! q;
+ /* Refer to every declared value, to avoid compiler
optimizations. */
+ return (!a + !b + !c + !d + !e + !f + !g + !h + !i + !!j + !k +
!!l
+ + !m + !n + !o + !p + !q + !pq);
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ ac_cv_header_stdbool_h=yes
+else
+ ac_cv_header_stdbool_h=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_header_stdbool_h" >&5
+$as_echo "$ac_cv_header_stdbool_h" >&6; }
+ ac_fn_c_check_type "$LINENO" "_Bool" "ac_cv_type__Bool"
"$ac_includes_default"
+if test "x$ac_cv_type__Bool" = xyes; then :
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE__BOOL 1
+_ACEOF
+
+
+fi
+
+
+if test $ac_cv_header_stdbool_h = yes; then
+
+$as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
+
+fi
+
+
for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h
langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/pstat.h sys/resource.h
sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h
ucred.h utime.h wchar.h wctype.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
@@ -12861,6 +12955,43 @@ if test "$ac_cv_sizeof_off_t" -lt 8 -a "$segsize" !=
"1"; then
as_fn_error $? "Large file support is not enabled. Segment size cannot be
larger than 1GB." "$LINENO" 5
fi
+# The cast to long int works around a bug in the HP C Compiler
+# version HP92453-01 B.11.11.23709.GP, which incorrectly rejects
+# declarations like `int a3[[(sizeof (unsigned char)) >= 0]];'.
+# This bug is HP SR number 8606223364.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking size of bool" >&5
+$as_echo_n "checking size of bool... " >&6; }
+if ${ac_cv_sizeof_bool+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ if ac_fn_c_compute_int "$LINENO" "(long int) (sizeof (bool))"
"ac_cv_sizeof_bool" "#ifdef HAVE_STDBOOL_H
+#include <stdbool.h>
+#endif
+"; then :
+
+else
+ if test "$ac_cv_type_bool" = yes; then
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error 77 "cannot compute sizeof (bool)
+See \`config.log' for more details" "$LINENO" 5; }
+ else
+ ac_cv_sizeof_bool=0
+ fi
+fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_sizeof_bool" >&5
+$as_echo "$ac_cv_sizeof_bool" >&6; }
+
+
+
+cat >>confdefs.h <<_ACEOF
+#define SIZEOF_BOOL $ac_cv_sizeof_bool
+_ACEOF
+
+
+
##
## Functions, global variables
diff --git a/configure.in b/configure.in
index 5245899109..fd03711176 100644
--- a/configure.in
+++ b/configure.in
@@ -1149,6 +1149,8 @@ AC_SUBST(UUID_LIBS)
## Header files
##
+AC_HEADER_STDBOOL
+
AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h
ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/pstat.h
sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h
termios.h ucred.h utime.h wchar.h wctype.h])
# On BSD, test for net/if.h will fail unless sys/socket.h
@@ -1407,6 +1409,11 @@ if test "$ac_cv_sizeof_off_t" -lt 8 -a "$segsize" !=
"1"; then
AC_MSG_ERROR([Large file support is not enabled. Segment size cannot be
larger than 1GB.])
fi
+AC_CHECK_SIZEOF([bool], [],
+[#ifdef HAVE_STDBOOL_H
+#include <stdbool.h>
+#endif])
+
##
## Functions, global variables
diff --git a/src/include/c.h b/src/include/c.h
index 11fcffbae3..0a2c49c217 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -247,12 +247,21 @@
* bool
* Boolean value, either true or false.
*
- * XXX for C++ compilers, we assume the compiler has a compatible
- * built-in definition of bool.
+ * Use stdbool.h if available and its bool has size 1. That's useful for
+ * better compiler and debugger output and for compatibility with third-party
+ * libraries. But PostgreSQL currently cannot deal with bool of other sizes;
+ * there are static assertions around the code to prevent that.
+ *
+ * For C++ compilers, we assume the compiler has a compatible built-in
+ * definition of bool.
*/
#ifndef __cplusplus
+#if defined(HAVE_STDBOOL_H) && SIZEOF_BOOL == 1
+#include <stdbool.h>
+#else
+
#ifndef bool
typedef char bool;
#endif
@@ -265,6 +274,7 @@ typedef char bool;
#define false ((bool) 0)
#endif
+#endif
#endif /* not C++ */
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 0aa6be4666..b7d00d4164 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -479,6 +479,9 @@
/* Define to 1 if you have the `SSL_get_current_compression' function. */
#undef HAVE_SSL_GET_CURRENT_COMPRESSION
+/* Define to 1 if stdbool.h conforms to C99. */
+#undef HAVE_STDBOOL_H
+
/* Define to 1 if you have the <stdint.h> header file. */
#undef HAVE_STDINT_H
@@ -678,6 +681,9 @@
/* Define to 1 if you have the <winldap.h> header file. */
#undef HAVE_WINLDAP_H
+/* Define to 1 if the system has the type `_Bool'. */
+#undef HAVE__BOOL
+
/* Define to 1 if your compiler understands __builtin_bswap16. */
#undef HAVE__BUILTIN_BSWAP16
@@ -787,6 +793,9 @@
RELSEG_SIZE requires an initdb. */
#undef RELSEG_SIZE
+/* The size of `bool', as computed by sizeof. */
+#undef SIZEOF_BOOL
+
/* The size of `long', as computed by sizeof. */
#undef SIZEOF_LONG
diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h
index aac95f8d2c..d8030398de 100644
--- a/src/pl/plperl/plperl.h
+++ b/src/pl/plperl/plperl.h
@@ -50,6 +50,11 @@
#define __inline__ inline
#endif
+/*
+ * Prevent perl from redefining "bool".
+ */
+#define HAS_BOOL 1
+
/*
* Get the basic Perl API. We use PERL_NO_GET_CONTEXT mode so that our code
@@ -91,11 +96,6 @@
#define NEED_sv_2pv_flags
#include "ppport.h"
-/* perl may have a different width of "bool", don't buy it */
-#ifdef bool
-#undef bool
-#endif
-
/* supply HeUTF8 if it's missing - ppport.h doesn't supply it, unfortunately */
#ifndef HeUTF8
#define HeUTF8(he) ((HeKLEN(he) == HEf_SVKEY) ?
\
--
2.15.1
