c++tools, configury: Configure with C++; test checking status [PR98821].

Message ID 41224B72-4FFA-4CF6-AE5C-1A6F0F83DD87@sandoe.co.uk
State New
Headers show
Series
  • c++tools, configury: Configure with C++; test checking status [PR98821].
Related show

Commit Message

Iain Sandoe July 20, 2021, 3:21 p.m.
Hi Folks,

Following Jakub’s suggestions (on irc) here is a patch that works around
misconfiguration of the c++tools directory present for at least Linux and Darwin
(probably on any platform that does not have typedefs for the inet structs in its
 system headers).

This also pulls in tests for the checking configure flags (copied from libcpp) and the
implementations of gcc_assert (copied from gcc).  Actually, there’s not much original
code here - but the combination is new, of course.

Tested lightly on Linux and Darwin for master w/wout —disable-checking and on
gcc-11 with default (release).  At least the configures now seem to DTRT for those.

OK for master and GCC-11.2?
 (if a complete regtest for passes for both)

thanks
Iain


-- 
2.24.3 (Apple Git-128)

Comments

Bill Schmidt via Gcc-patches July 20, 2021, 3:37 p.m. | #1
On Tue, Jul 20, 2021 at 04:21:34PM +0100, Iain Sandoe wrote:

> --- a/c++tools/configure.ac

> +++ b/c++tools/configure.ac

> @@ -41,6 +41,8 @@ MISSING=`cd $ac_aux_dir && ${PWDCMD-pwd}`/missing

>  AC_CHECK_PROGS([AUTOCONF], [autoconf], [$MISSING autoconf])

>  AC_CHECK_PROGS([AUTOHEADER], [autoheader], [$MISSING autoheader])

>  

> +AC_LANG(C++)

> +

>  dnl Enabled by default

>  AC_MSG_CHECKING([whether to build C++ tools])

>    AC_ARG_ENABLE(c++-tools, 

> @@ -67,6 +69,62 @@ AC_MSG_RESULT([$maintainer_mode])

>  test "$maintainer_mode" = yes && MAINTAINER=yes

>  AC_SUBST(MAINTAINER)

>  

> +# Enable expensive internal checks

> +is_release=

> +if test -f $srcdir/../gcc/DEV-PHASE \

> +   && test x"`cat $srcdir/../gcc/DEV-PHASE`" != xexperimental; then

> +  is_release=yes

> +fi

> +

> +AC_ARG_ENABLE(checking,

> +[AS_HELP_STRING([[--enable-checking[=LIST]]],

> +		[enable expensive run-time checks.  With LIST,

> +		 enable only specific categories of checks.

> +		 Categories are: yes,no,all,none,release.

> +		 Flags are: misc,valgrind or other strings])],

> +[ac_checking_flags="${enableval}"],[

> +# Determine the default checks.

> +if test x$is_release = x ; then

> +  ac_checking_flags=yes

> +else

> +  ac_checking_flags=release

> +fi])

> +IFS="${IFS= 	}"; ac_save_IFS="$IFS"; IFS="$IFS,"

> +for check in release $ac_checking_flags

> +do

> +	case $check in

> +	# these set all the flags to specific states

> +	yes|all) ac_checking=1 ; ac_assert_checking=1 ; ac_valgrind_checking= ;;

> +	no|none) ac_checking= ; ac_assert_checking= ; ac_valgrind_checking= ;;

> +	release) ac_checking= ; ac_assert_checking=1 ; ac_valgrind_checking= ;;

> +	# these enable particular checks

> +	assert) ac_assert_checking=1 ;;

> +	misc) ac_checking=1 ;;

> +	valgrind) ac_valgrind_checking=1 ;;

> +	# accept

> +	*) ;;

> +	esac

> +done

> +IFS="$ac_save_IFS"

> +

> +if test x$ac_checking != x ; then

> +  AC_DEFINE(CHECKING_P, 1,

> +[Define to 1 if you want more run-time sanity checks.])

> +else

> +  AC_DEFINE(CHECKING_P, 0)

> +fi

> +

> +if test x$ac_assert_checking != x ; then

> +  AC_DEFINE(ENABLE_ASSERT_CHECKING, 1,

> +[Define if you want assertions enabled.  This is a cheap check.])

> +fi

> +

> +if test x$ac_valgrind_checking != x ; then

> +  AC_DEFINE(ENABLE_VALGRIND_CHECKING, 1,

> +[Define if you want to workaround valgrind (a memory checker) warnings about

> + possible memory leaks because of libcpp use of interior pointers.])

> +fi


I guess we could simplify it, I think at least right now we only care
about ENABLE_ASSERT_CHECKING and nothing else, so the is_release computation
could go, make ac_checking_flags just default to yes, drop the ac_checking,
ac_valgrind_checking variables and simplify the case so that it perhaps
handles all we care together.  That would be
	case $check in
	yes|all|release|assert) ac_assert_checking=1 ; ;;
	no|none) ac_assert_checking= ; ;;
	*) ;;
	esac
or so. and then only AC_DEFINE ENABLE_ASSERT_CHECKING and from server.cc
drop the CHECKING_P stuff.
> @@ -92,6 +96,35 @@ along with GCC; see the file COPYING3.  If not see

>  #define DIR_SEPARATOR '/'

>  #endif

>  

> +/* Imported from libcpp/system.h

> +   Use gcc_assert(EXPR) to test invariants.  */

> +#if ENABLE_ASSERT_CHECKING || CHECKING_P

> +#define gcc_assert(EXPR)                                                \

> +   ((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0))

> +#elif (GCC_VERSION >= 4005)

> +#define gcc_assert(EXPR)                                                \

> +  ((void)(__builtin_expect (!(EXPR), 0) ? __builtin_unreachable (), 0 : 0))

> +#else

> +/* Include EXPR, so that unused variable warnings do not occur.  */

> +#define gcc_assert(EXPR) ((void)(0 && (EXPR)))

> +#endif

> +

> +#if CHECKING_P

> +#define gcc_checking_assert(EXPR) gcc_assert (EXPR)

> +#else

> +/* N.B.: in release build EXPR is not evaluated.  */

> +#define gcc_checking_assert(EXPR) ((void)(0 && (EXPR)))

> +#endif


I'd drop the gcc_checking_assert macro, we don't use it...

Otherwise LGTM.

	Jakub
Bill Schmidt via Gcc-patches July 29, 2021, 2:59 a.m. | #2
On 7/20/21 11:21 AM, Iain Sandoe wrote:
> Hi Folks,

> 

> Following Jakub’s suggestions (on irc) here is a patch that works around

> misconfiguration of the c++tools directory present for at least Linux and Darwin

> (probably on any platform that does not have typedefs for the inet structs in its

>   system headers).

> 

> This also pulls in tests for the checking configure flags (copied from libcpp) and the

> implementations of gcc_assert (copied from gcc).  Actually, there’s not much original

> code here - but the combination is new, of course.

> 

> Tested lightly on Linux and Darwin for master w/wout —disable-checking and on

> gcc-11 with default (release).  At least the configures now seem to DTRT for those.

> 

> OK for master and GCC-11.2?

>   (if a complete regtest for passes for both)


OK.

> thanks

> Iain

> 

> 

> ====

> 

> The c++tools configure fragments need to be built with a C++ compiler.

> 

> In addition, the stand-alone server uses diagnostic mechanisms in common

> with GCC, but needs to define implementations of the asserts and

> supporting output functions.

> 

> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

> 

> PR c++/98821 - modules : c++tools configures with CC but code fragments assume CXX.

> 

> 	PR c++/98821

> 

> c++tools/ChangeLog:

> 

> 	* config.h.in: Regenerate.

> 	* configure: Regenerate.

> 	* configure.ac: Configure using C++.  Pull logic to

> 	detect enabled checking modes.

> 	* server.cc (AI_NUMERICSERV): Define a fallback value.

> 	(gcc_assert): New.

> 	(gcc_checking_assert): New.

> 	(gcc_unreachable): New.

> 	(fancy_abort): Only build when checking is enabled.

> 

> Co-authored-by: Jakub Jelinek <jakub@redhat.com>

> ---

>   c++tools/config.h.in  |  10 +

>   c++tools/configure    | 766 +++++++-----------------------------------

>   c++tools/configure.ac |  58 ++++

>   c++tools/server.cc    |  35 ++

>   4 files changed, 228 insertions(+), 641 deletions(-)

> 

> diff --git a/c++tools/configure.ac b/c++tools/configure.ac

> index 70fcb641db9..cb67dabf191 100644

> --- a/c++tools/configure.ac

> +++ b/c++tools/configure.ac

> @@ -41,6 +41,8 @@ MISSING=`cd $ac_aux_dir && ${PWDCMD-pwd}`/missing

>   AC_CHECK_PROGS([AUTOCONF], [autoconf], [$MISSING autoconf])

>   AC_CHECK_PROGS([AUTOHEADER], [autoheader], [$MISSING autoheader])

>   

> +AC_LANG(C++)

> +

>   dnl Enabled by default

>   AC_MSG_CHECKING([whether to build C++ tools])

>     AC_ARG_ENABLE(c++-tools,

> @@ -67,6 +69,62 @@ AC_MSG_RESULT([$maintainer_mode])

>   test "$maintainer_mode" = yes && MAINTAINER=yes

>   AC_SUBST(MAINTAINER)

>   

> +# Enable expensive internal checks

> +is_release=

> +if test -f $srcdir/../gcc/DEV-PHASE \

> +   && test x"`cat $srcdir/../gcc/DEV-PHASE`" != xexperimental; then

> +  is_release=yes

> +fi

> +

> +AC_ARG_ENABLE(checking,

> +[AS_HELP_STRING([[--enable-checking[=LIST]]],

> +		[enable expensive run-time checks.  With LIST,

> +		 enable only specific categories of checks.

> +		 Categories are: yes,no,all,none,release.

> +		 Flags are: misc,valgrind or other strings])],

> +[ac_checking_flags="${enableval}"],[

> +# Determine the default checks.

> +if test x$is_release = x ; then

> +  ac_checking_flags=yes

> +else

> +  ac_checking_flags=release

> +fi])

> +IFS="${IFS= 	}"; ac_save_IFS="$IFS"; IFS="$IFS,"

> +for check in release $ac_checking_flags

> +do

> +	case $check in

> +	# these set all the flags to specific states

> +	yes|all) ac_checking=1 ; ac_assert_checking=1 ; ac_valgrind_checking= ;;

> +	no|none) ac_checking= ; ac_assert_checking= ; ac_valgrind_checking= ;;

> +	release) ac_checking= ; ac_assert_checking=1 ; ac_valgrind_checking= ;;

> +	# these enable particular checks

> +	assert) ac_assert_checking=1 ;;

> +	misc) ac_checking=1 ;;

> +	valgrind) ac_valgrind_checking=1 ;;

> +	# accept

> +	*) ;;

> +	esac

> +done

> +IFS="$ac_save_IFS"

> +

> +if test x$ac_checking != x ; then

> +  AC_DEFINE(CHECKING_P, 1,

> +[Define to 1 if you want more run-time sanity checks.])

> +else

> +  AC_DEFINE(CHECKING_P, 0)

> +fi

> +

> +if test x$ac_assert_checking != x ; then

> +  AC_DEFINE(ENABLE_ASSERT_CHECKING, 1,

> +[Define if you want assertions enabled.  This is a cheap check.])

> +fi

> +

> +if test x$ac_valgrind_checking != x ; then

> +  AC_DEFINE(ENABLE_VALGRIND_CHECKING, 1,

> +[Define if you want to workaround valgrind (a memory checker) warnings about

> + possible memory leaks because of libcpp use of interior pointers.])

> +fi

> +

>   # Check whether --enable-default-pie was given.

>   AC_ARG_ENABLE(default-pie,

>   [AS_HELP_STRING([--enable-default-pie],

> diff --git a/c++tools/server.cc b/c++tools/server.cc

> index fae3e78dc5d..3056352e24b 100644

> --- a/c++tools/server.cc

> +++ b/c++tools/server.cc

> @@ -61,6 +61,10 @@ along with GCC; see the file COPYING3.  If not see

>   # define gai_strerror(X) ""

>   #endif

>   

> +#ifndef AI_NUMERICSERV

> +#define AI_NUMERICSERV 0

> +#endif

> +

>   #include <getopt.h>

>   

>   // Select or epoll

> @@ -92,6 +96,35 @@ along with GCC; see the file COPYING3.  If not see

>   #define DIR_SEPARATOR '/'

>   #endif

>   

> +/* Imported from libcpp/system.h

> +   Use gcc_assert(EXPR) to test invariants.  */

> +#if ENABLE_ASSERT_CHECKING || CHECKING_P

> +#define gcc_assert(EXPR)                                                \

> +   ((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0))

> +#elif (GCC_VERSION >= 4005)

> +#define gcc_assert(EXPR)                                                \

> +  ((void)(__builtin_expect (!(EXPR), 0) ? __builtin_unreachable (), 0 : 0))

> +#else

> +/* Include EXPR, so that unused variable warnings do not occur.  */

> +#define gcc_assert(EXPR) ((void)(0 && (EXPR)))

> +#endif

> +

> +#if CHECKING_P

> +#define gcc_checking_assert(EXPR) gcc_assert (EXPR)

> +#else

> +/* N.B.: in release build EXPR is not evaluated.  */

> +#define gcc_checking_assert(EXPR) ((void)(0 && (EXPR)))

> +#endif

> +

> +/* Use gcc_unreachable() to mark unreachable locations (like an

> +   unreachable default case of a switch.  Do not use gcc_assert(0).  */

> +#if (GCC_VERSION >= 4005) && !ENABLE_ASSERT_CHECKING

> +#define gcc_unreachable() __builtin_unreachable ()

> +#else

> +#define gcc_unreachable() (fancy_abort (__FILE__, __LINE__, __FUNCTION__))

> +#endif

> +

> +

>   #if NETWORKING

>   struct netmask {

>     in6_addr addr;

> @@ -202,11 +235,13 @@ internal_error (const char *fmt, ...)

>   

>   /* Hooked to from gcc_assert & gcc_unreachable.  */

>   

> +#if ENABLE_ASSERT_CHECKING || CHECKING_P

>   void ATTRIBUTE_NORETURN ATTRIBUTE_COLD

>   fancy_abort (const char *file, int line, const char *func)

>   {

>     internal_error ("in %s, at %s:%d", func, trim_src_file (file), line);

>   }

> +#endif

>   

>   /* Exploded on a signal.  */

>   

>

Patch

====

The c++tools configure fragments need to be built with a C++ compiler.

In addition, the stand-alone server uses diagnostic mechanisms in common
with GCC, but needs to define implementations of the asserts and
supporting output functions.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

PR c++/98821 - modules : c++tools configures with CC but code fragments assume CXX.

	PR c++/98821

c++tools/ChangeLog:

	* config.h.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Configure using C++.  Pull logic to
	detect enabled checking modes.
	* server.cc (AI_NUMERICSERV): Define a fallback value.
	(gcc_assert): New.
	(gcc_checking_assert): New.
	(gcc_unreachable): New.
	(fancy_abort): Only build when checking is enabled.

Co-authored-by: Jakub Jelinek <jakub@redhat.com>
---
 c++tools/config.h.in  |  10 +
 c++tools/configure    | 766 +++++++-----------------------------------
 c++tools/configure.ac |  58 ++++
 c++tools/server.cc    |  35 ++
 4 files changed, 228 insertions(+), 641 deletions(-)

diff --git a/c++tools/configure.ac b/c++tools/configure.ac
index 70fcb641db9..cb67dabf191 100644
--- a/c++tools/configure.ac
+++ b/c++tools/configure.ac
@@ -41,6 +41,8 @@  MISSING=`cd $ac_aux_dir && ${PWDCMD-pwd}`/missing
 AC_CHECK_PROGS([AUTOCONF], [autoconf], [$MISSING autoconf])
 AC_CHECK_PROGS([AUTOHEADER], [autoheader], [$MISSING autoheader])
 
+AC_LANG(C++)
+
 dnl Enabled by default
 AC_MSG_CHECKING([whether to build C++ tools])
   AC_ARG_ENABLE(c++-tools, 
@@ -67,6 +69,62 @@  AC_MSG_RESULT([$maintainer_mode])
 test "$maintainer_mode" = yes && MAINTAINER=yes
 AC_SUBST(MAINTAINER)
 
+# Enable expensive internal checks
+is_release=
+if test -f $srcdir/../gcc/DEV-PHASE \
+   && test x"`cat $srcdir/../gcc/DEV-PHASE`" != xexperimental; then
+  is_release=yes
+fi
+
+AC_ARG_ENABLE(checking,
+[AS_HELP_STRING([[--enable-checking[=LIST]]],
+		[enable expensive run-time checks.  With LIST,
+		 enable only specific categories of checks.
+		 Categories are: yes,no,all,none,release.
+		 Flags are: misc,valgrind or other strings])],
+[ac_checking_flags="${enableval}"],[
+# Determine the default checks.
+if test x$is_release = x ; then
+  ac_checking_flags=yes
+else
+  ac_checking_flags=release
+fi])
+IFS="${IFS= 	}"; ac_save_IFS="$IFS"; IFS="$IFS,"
+for check in release $ac_checking_flags
+do
+	case $check in
+	# these set all the flags to specific states
+	yes|all) ac_checking=1 ; ac_assert_checking=1 ; ac_valgrind_checking= ;;
+	no|none) ac_checking= ; ac_assert_checking= ; ac_valgrind_checking= ;;
+	release) ac_checking= ; ac_assert_checking=1 ; ac_valgrind_checking= ;;
+	# these enable particular checks
+	assert) ac_assert_checking=1 ;;
+	misc) ac_checking=1 ;;
+	valgrind) ac_valgrind_checking=1 ;;
+	# accept
+	*) ;;
+	esac
+done
+IFS="$ac_save_IFS"
+
+if test x$ac_checking != x ; then
+  AC_DEFINE(CHECKING_P, 1,
+[Define to 1 if you want more run-time sanity checks.])
+else
+  AC_DEFINE(CHECKING_P, 0)
+fi
+
+if test x$ac_assert_checking != x ; then
+  AC_DEFINE(ENABLE_ASSERT_CHECKING, 1,
+[Define if you want assertions enabled.  This is a cheap check.])
+fi
+
+if test x$ac_valgrind_checking != x ; then
+  AC_DEFINE(ENABLE_VALGRIND_CHECKING, 1,
+[Define if you want to workaround valgrind (a memory checker) warnings about
+ possible memory leaks because of libcpp use of interior pointers.])
+fi
+
 # Check whether --enable-default-pie was given.
 AC_ARG_ENABLE(default-pie,
 [AS_HELP_STRING([--enable-default-pie],
diff --git a/c++tools/server.cc b/c++tools/server.cc
index fae3e78dc5d..3056352e24b 100644
--- a/c++tools/server.cc
+++ b/c++tools/server.cc
@@ -61,6 +61,10 @@  along with GCC; see the file COPYING3.  If not see
 # define gai_strerror(X) ""
 #endif
 
+#ifndef AI_NUMERICSERV
+#define AI_NUMERICSERV 0
+#endif
+
 #include <getopt.h>
 
 // Select or epoll
@@ -92,6 +96,35 @@  along with GCC; see the file COPYING3.  If not see
 #define DIR_SEPARATOR '/'
 #endif
 
+/* Imported from libcpp/system.h
+   Use gcc_assert(EXPR) to test invariants.  */
+#if ENABLE_ASSERT_CHECKING || CHECKING_P
+#define gcc_assert(EXPR)                                                \
+   ((void)(!(EXPR) ? fancy_abort (__FILE__, __LINE__, __FUNCTION__), 0 : 0))
+#elif (GCC_VERSION >= 4005)
+#define gcc_assert(EXPR)                                                \
+  ((void)(__builtin_expect (!(EXPR), 0) ? __builtin_unreachable (), 0 : 0))
+#else
+/* Include EXPR, so that unused variable warnings do not occur.  */
+#define gcc_assert(EXPR) ((void)(0 && (EXPR)))
+#endif
+
+#if CHECKING_P
+#define gcc_checking_assert(EXPR) gcc_assert (EXPR)
+#else
+/* N.B.: in release build EXPR is not evaluated.  */
+#define gcc_checking_assert(EXPR) ((void)(0 && (EXPR)))
+#endif
+
+/* Use gcc_unreachable() to mark unreachable locations (like an
+   unreachable default case of a switch.  Do not use gcc_assert(0).  */
+#if (GCC_VERSION >= 4005) && !ENABLE_ASSERT_CHECKING
+#define gcc_unreachable() __builtin_unreachable ()
+#else
+#define gcc_unreachable() (fancy_abort (__FILE__, __LINE__, __FUNCTION__))
+#endif
+
+
 #if NETWORKING
 struct netmask {
   in6_addr addr;
@@ -202,11 +235,13 @@  internal_error (const char *fmt, ...)
 
 /* Hooked to from gcc_assert & gcc_unreachable.  */
 
+#if ENABLE_ASSERT_CHECKING || CHECKING_P
 void ATTRIBUTE_NORETURN ATTRIBUTE_COLD
 fancy_abort (const char *file, int line, const char *func)
 {
   internal_error ("in %s, at %s:%d", func, trim_src_file (file), line);
 }
+#endif
 
 /* Exploded on a signal.  */