diff mbox

Error out on -fvtable-verify without --enable-vtable-verify

Message ID ydd1t5cltho.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth May 8, 2016, 10:44 a.m. UTC 
With the recent change not to install libvtv without
--enable-vtable-verify, I noticed that gcc/g++ would still accept
-fvtable-verify without errors, only to emit obscure link-time errors
about missing vtv_*.o (which hadn't been installed in that situation
before) and libvtv.

It seems to me a much better user experience to emit a clear error
message in this case, which is what this patch does.

Bootstrapped without regressions (without and with
--enable-vtable-verify) on i386-pc-solaris2.12 and x86_64-pc-linux-gnu.
Manually tested that I get (or don't get) the expected errors for
-fvtable-verfy=(none|std|preinit].

Ok for mainline?

	Rainer


2016-05-04  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.ac (enable_vtable_verify): Handle --enable-vtable-verify.
	* configure: Regenerate.
	* config.in: Regenerate.
	* gcc.c (VTABLE_VERIFICATION_SPEC) [!ENABLE_VTABLE_VERIFY]: Error
	on -fvtable-verify.
	* config/sol2.h [!ENABLE_VTABLE_VERIFY] (STARTFILE_VTV_SPEC): Define.
	(ENDFILE_VTV_SPEC): Define.

Comments

Bernd Schmidt May 9, 2016, 9:23 a.m. UTC | #1 
On 05/08/2016 12:44 PM, Rainer Orth wrote:
> With the recent change not to install libvtv without
> --enable-vtable-verify, I noticed that gcc/g++ would still accept
> -fvtable-verify without errors, only to emit obscure link-time errors
> about missing vtv_*.o (which hadn't been installed in that situation
> before) and libvtv.
>
> It seems to me a much better user experience to emit a clear error
> message in this case, which is what this patch does.

Generally ok, but...

> +AC_ARG_ENABLE(vtable-verify,
> +[AS_HELP_STRING([--enable-vtable-verify],
> +		[enable vtable verification feature])],
> +[case "$enableval" in
> + yes) enable_vtable_verify=yes ;;
> + no)  enable_vtable_verify=no ;;
> + *)   enable_vtable_verify=no;;
> + esac],
> +[enable_vtable_verify=no])
> +vtable_verify=`if test $enable_vtable_verify != no; then echo 1; else echo 0; fi`
> +AC_DEFINE_UNQUOTED(ENABLE_VTABLE_VERIFY, $vtable_verify,
> +[Define 0/1 if vtable verification feature is enabled.])

That looks a little overly complicated. Don't you get the enable_ 
variables set by autoconf? And if you do need the case statement, you 
might as well set things to 0/1 directly and skip the 
enable_vtable_verify variable entirely.


Bernd
diff mbox

Patch

# HG changeset patch
# Parent  26d037ddd624a6e7b738c1db2b6dbdeb90c9b01c
Error out on -fvtable-verify without --enable-vtable-verify

diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h
--- a/gcc/config/sol2.h
+++ b/gcc/config/sol2.h
@@ -166,21 +166,26 @@  along with GCC; see the file COPYING3.  
 #define STARTFILE_CRTBEGIN_SPEC	"crtbegin.o%s"
 #endif
 
+#if ENABLE_VTABLE_VERIFY
 #if SUPPORTS_INIT_PRIORITY
 #define STARTFILE_VTV_SPEC \
   "%{fvtable-verify=none:%s; \
      fvtable-verify=preinit:vtv_start_preinit.o%s; \
      fvtable-verify=std:vtv_start.o%s}"
-
 #define ENDFILE_VTV_SPEC \
   "%{fvtable-verify=none:%s; \
      fvtable-verify=preinit:vtv_end_preinit.o%s; \
      fvtable-verify=std:vtv_end.o%s}"
-#else
+#else /* !SUPPORTS_INIT_PRIORITY */
 #define STARTFILE_VTV_SPEC \
-  "%{fvtable-verify:%e-fvtable-verify is not supported in this configuration}"
+  "%{fvtable-verify=*: \
+     %e-fvtable-verify=%* is not supported in this configuration}"
 #define ENDFILE_VTV_SPEC ""
-#endif
+#endif /* !SUPPORTS_INIT_PRIORITY */
+#else /* !ENABLE_VTABLE_VERIFY */
+#define STARTFILE_VTV_SPEC ""
+#define ENDFILE_VTV_SPEC ""
+#endif /* !ENABLE_VTABLE_VERIFY */
 
 /* We don't use the standard svr4 STARTFILE_SPEC because it's wrong for us.  */
 #undef STARTFILE_SPEC
diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -865,6 +865,19 @@  Valid choices are 'yes' and 'no'.]) ;;
   esac
 ], [enable_tls=''])
 
+AC_ARG_ENABLE(vtable-verify,
+[AS_HELP_STRING([--enable-vtable-verify],
+		[enable vtable verification feature])],
+[case "$enableval" in
+ yes) enable_vtable_verify=yes ;;
+ no)  enable_vtable_verify=no ;;
+ *)   enable_vtable_verify=no;;
+ esac],
+[enable_vtable_verify=no])
+vtable_verify=`if test $enable_vtable_verify != no; then echo 1; else echo 0; fi`
+AC_DEFINE_UNQUOTED(ENABLE_VTABLE_VERIFY, $vtable_verify,
+[Define 0/1 if vtable verification feature is enabled.])
+
 AC_ARG_ENABLE(objc-gc,
 [AS_HELP_STRING([--enable-objc-gc],
 		[enable the use of Boehm's garbage collector with
diff --git a/gcc/gcc.c b/gcc/gcc.c
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -989,9 +989,18 @@  proper position among the other output f
     the vtable verification runtime functions are in libstdc++, so we use
     the spec just below this one.  */
 #ifndef VTABLE_VERIFICATION_SPEC
+#if ENABLE_VTABLE_VERIFY
 #define VTABLE_VERIFICATION_SPEC "\
 %{!nostdlib:%{fvtable-verify=std: -lvtv -u_vtable_map_vars_start -u_vtable_map_vars_end}\
     %{fvtable-verify=preinit: -lvtv -u_vtable_map_vars_start -u_vtable_map_vars_end}}"
+#else
+#define VTABLE_VERIFICATION_SPEC "\
+%{fvtable-verify=none:} \
+%{fvtable-verify=std: \
+  %e-fvtable-verify=std is not supported in this configuration} \
+%{fvtable-verify=preinit: \
+  %e-fvtable-verify=preinit is not supported in this configuration}"
+#endif
 #endif
 
 #ifndef CHKP_SPEC