diff mbox

[build,lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

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

Commit Message

Rainer Orth Feb. 7, 2011, 12:11 p.m. UTC
The new gcc.dg/lto/20110201-1_0.c test fails on Solaris 2 with Sun ld
(SPARC and x86, 32-bit only):

FAIL: gcc.dg/lto/20110201-1 c_lto_20110201-1_0.o-c_lto_20110201-1_0.o link,  -O0 -flto 
UNRESOLVED: gcc.dg/lto/20110201-1 c_lto_20110201-1_0.o-c_lto_20110201-1_0.o execute  -O0 -flto 

output is:
Undefined			first referenced
 symbol  			    in file
cabs                                c_lto_20110201-1_0.o
ld: fatal: symbol referencing errors. No output written to gcc-dg-lto-20110201-1-01

This is another instance of PR lto/46944: the dg-require-linker-plugin
test assumes that the linker in use supports -plugin when one can
compile a simple test program with -fuse-linker-plugin.  Unfortunately,
this is wrong: while Sun ld doesn't support -plugin, it does have a
-p <auditlib> option, thus treats -plugin as -p lugin and interprets the
next arg as linker input file.  For 32-bit, this seems to work, for a
64-bit link, the linker errors out since it expects 64-bit input files
while lto-plugin.so is a 32-bit shared object, thus the
dg-require-linker-plugin concludes that -plugin support is absent.

It always seemed bogus to me to accept/pass on a linker switch without
checking if the linker in use really supports it, and this failure
finally prompted me to develop a real check.  Obviously, one need not
only test if the linker accepts -plugin, but also if the plugin passed
is really used.  Otherwise, gcc -fuse-linker-plugin will now error out,
as well as collect2 -plugin.

I'm uncertain if the configure check below is acceptable, though: I do
need to build a shared object and cannot use libtool in
gcc/configure.ac.  Therefore I rely on gcc -shared, which only works in
stage2 and up, or if gcc is used as the bootstrap compiler.

The following patch was bootstrapped without regressions on
i386-pc-solaris2.11 both with Sun ld and gld 2.21.  As expected, with
Sun ld it fixed the gcc.dg/lto/20110201-1_0.c failure above, but with
GNU ld, the test still doesn't work.  This is obviously part of the
plugin enhancements that only went into binutils mainline so far (and
will perhaps be backported to 2.21.1).  I'm undecided what's the best
way to handle this: while one could certainly add something like
dg-require-enhanced-linker-plugin or something, this seems
overcomplicated to me.  I'd rather have us require versions of gld or
gold that fully work with -plugin (which ones do?), not only in a subset
of cases, and reject -fuse-linker-plugin if the linker version is too
old.  Everthing else will just confuse users.

Apart from this patch, there's also gfortran.dg/lto/pr41764_0.f which
could use dg-require-linker-plugin, but currently doesn't since the
basic -plugin support in gold doesn't work for it.  If we follow my
suggestion above, the patch could be made to use
dg-require-linker-plugin.  I'll post a followup if we go the route
above.

Ok for mainline?

	Rainer


2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	gcc:
	PR lto/46944
	* configure.ac (ld_plugin): Check for real ld -plugin support
	(HAVE_LD_PLUGIN): Define.
	* configure: Regenerate.
	* config.in: Regenerate.
	* gcc.c (PLUGIN_COND): Only define if HAVE_LD_PLUGIN.
	(PLUGIN_COND_CLOSE): Likewise.
	LINK_PLUGIN_SPEC: Define
	(LINK_COMMAND_SPEC): Use it.
	* collect2.c (main) [!HAVE_LD_PLUGIN]: Error on -plugin.

Comments

Ralf Wildenhues Feb. 14, 2011, 7:39 p.m. UTC | #1
Hello Rainer,

Sorry for the delay, I somehow missed this mail.

* Rainer Orth wrote on Mon, Feb 07, 2011 at 01:11:39PM CET:
> This is another instance of PR lto/46944: the dg-require-linker-plugin
> test assumes that the linker in use supports -plugin when one can
> compile a simple test program with -fuse-linker-plugin.  Unfortunately,
> this is wrong: while Sun ld doesn't support -plugin, it does have a
> -p <auditlib> option, thus treats -plugin as -p lugin and interprets the
> next arg as linker input file.  For 32-bit, this seems to work, for a
> 64-bit link, the linker errors out since it expects 64-bit input files
> while lto-plugin.so is a 32-bit shared object, thus the
> dg-require-linker-plugin concludes that -plugin support is absent.
> 
> It always seemed bogus to me to accept/pass on a linker switch without
> checking if the linker in use really supports it, and this failure
> finally prompted me to develop a real check.  Obviously, one need not
> only test if the linker accepts -plugin, but also if the plugin passed
> is really used.  Otherwise, gcc -fuse-linker-plugin will now error out,
> as well as collect2 -plugin.
> 
> I'm uncertain if the configure check below is acceptable, though: I do
> need to build a shared object and cannot use libtool in
> gcc/configure.ac.

You can.  Put
  LT_OUTPUT

in configure.ac before the code that uses './libtool'.
(It would need to be cleaned up from Makefile.in.)

When using libtool you need to ensure that you get all flags you want to
test past libtool: prepend them with '-Wc,'.

I'm not quite sure which compilers this test needs to work with.
It looks however like your patch will not work on some systems
even with GCC, see below for details.

> Therefore I rely on gcc -shared, which only works in
> stage2 and up, or if gcc is used as the bootstrap compiler.

> Ok for mainline?

I'm afraid not.  I only review the build system bits.

> 2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	gcc:
> 	PR lto/46944
> 	* configure.ac (ld_plugin): Check for real ld -plugin support
> 	(HAVE_LD_PLUGIN): Define.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* gcc.c (PLUGIN_COND): Only define if HAVE_LD_PLUGIN.
> 	(PLUGIN_COND_CLOSE): Likewise.
> 	LINK_PLUGIN_SPEC: Define
> 	(LINK_COMMAND_SPEC): Use it.
> 	* collect2.c (main) [!HAVE_LD_PLUGIN]: Error on -plugin.

> --- a/gcc/configure.ac	Sat Feb 05 09:17:53 2011 +0100
> +++ b/gcc/configure.ac	Sat Feb 05 15:12:39 2011 +0100
> @@ -4771,6 +4771,27 @@
>  
>  if test x"$enable_plugin" = x"yes"; then
>  
> +  AC_MSG_CHECKING([for -plugin])
> +  ld_plugin=no
> +  echo 'int main() {return 0;}' > conftest.c
> +  ${CC} ${CFLAGS} -c conftest.c > /dev/null 2>&1

Could add $CPPFLAGS too.  I've seen -m32 in there before.

Dropping error output and not showing the commands used in config.log is
fairly ugly for debugging.  It is more helpful to use AC_COMPILE_IFELSE
here.  Below, it is even more important, as the commands are likely to
fail.  The above could simply be
  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [])],
    [ yep that worked, let's try the rest in here.
      ...
    ])

> +  # Check if plugin is really used.
> +  echo 'int onload () {puts ("yes"); return 0;}' > confplugin.c
> +  # This depends on gcc, otherwise would have to use libtool.
> +  if ${CC} ${CFLAGS} ${LDFLAGS} -shared -static-libgcc -fPIC \

Likewise for CPPFLAGS.

This will likely fail on MinGW due to the .so name, and anyway when
cross-compiling.

This might fail on Darwin unless you replace -shared with what the other
test nearby does modifying LDFLAGS.

> +      -o confplugin.so confplugin.c > /dev/null 2>&1 \
> +    && test x$gcc_cv_ld != x \
> +    && ld_plugin_out=`$gcc_cv_ld -plugin ./confplugin.so \
> +      -o conftest conftest.o -lc 2>/dev/null` \

I don't quite understand how this test is supposed to work in a cross
compile setup.  You compile something with $CC and try to load it with
$gcc_cv_ld, but IIUC you are using the test result for the compiler that
you are generating.  What am I missing?  Or is this test simply
irrelevant in cross compile mode anyway?

> +    && test x$ld_plugin_out = xyes; then
> +    ld_plugin=yes
> +  fi

rm -f confplugin.* conftest.$ac_objext conftest.c

> +  if test x$ld_plugin = xyes; then
> +    AC_DEFINE(HAVE_LD_PLUGIN, 1,
> +      [Define if your linker supports -plugin.])
> +  fi
> +  AC_MSG_RESULT([$ld_plugin])
> +
>    AC_MSG_CHECKING([for exported symbols])
>    if test "x$export_sym_check" != x; then
>      echo "int main() {return 0;} int foobar() {return 0;}" > conftest.c
> @@ -4834,7 +4855,7 @@
>      if test x"$default_plugin" != x"yes"; then
>        AC_MSG_ERROR([
>  Building GCC with plugin support requires a host that supports
> --fPIC, -shared, -ldl and -rdynamic.])
> +-plugin, -fPIC, -shared, -ldl and -rdynamic.])
>      fi
>    fi
>  fi
diff mbox

Patch

diff -r b84c1dcc61f1 gcc/collect2.c
--- a/gcc/collect2.c	Sat Feb 05 09:17:53 2011 +0100
+++ b/gcc/collect2.c	Sat Feb 05 15:12:39 2011 +0100
@@ -1,7 +1,7 @@ 
 /* Collect static initialization info into data structures that can be
    traversed by C++ initialization and finalization routines.
    Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010
+   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010, 2011
    Free Software Foundation, Inc.
    Contributed by Chris Smith (csmith@convex.com).
    Heavily modified by Michael Meissner (meissner@cygnus.com),
@@ -1215,8 +1215,12 @@ 
 	  lto_mode = LTO_MODE_NONE;
         else if (! strcmp (argv[i], "-plugin"))
 	  {
+#ifdef HAVE_LD_PLUGIN
 	    use_plugin = true;
 	    lto_mode = LTO_MODE_NONE;
+#else
+	    error ("linker doesn't support -plugin");
+#endif
 	  }
 #ifdef COLLECT_EXPORT_LIST
 	/* since -brtl, -bexport, -b64 are not position dependent
diff -r b84c1dcc61f1 gcc/configure.ac
--- a/gcc/configure.ac	Sat Feb 05 09:17:53 2011 +0100
+++ b/gcc/configure.ac	Sat Feb 05 15:12:39 2011 +0100
@@ -4771,6 +4771,27 @@ 
 
 if test x"$enable_plugin" = x"yes"; then
 
+  AC_MSG_CHECKING([for -plugin])
+  ld_plugin=no
+  echo 'int main() {return 0;}' > conftest.c
+  ${CC} ${CFLAGS} -c conftest.c > /dev/null 2>&1
+  # Check if plugin is really used.
+  echo 'int onload () {puts ("yes"); return 0;}' > confplugin.c
+  # This depends on gcc, otherwise would have to use libtool.
+  if ${CC} ${CFLAGS} ${LDFLAGS} -shared -static-libgcc -fPIC \
+      -o confplugin.so confplugin.c > /dev/null 2>&1 \
+    && test x$gcc_cv_ld != x \
+    && ld_plugin_out=`$gcc_cv_ld -plugin ./confplugin.so \
+      -o conftest conftest.o -lc 2>/dev/null` \
+    && test x$ld_plugin_out = xyes; then
+    ld_plugin=yes
+  fi
+  if test x$ld_plugin = xyes; then
+    AC_DEFINE(HAVE_LD_PLUGIN, 1,
+      [Define if your linker supports -plugin.])
+  fi
+  AC_MSG_RESULT([$ld_plugin])
+
   AC_MSG_CHECKING([for exported symbols])
   if test "x$export_sym_check" != x; then
     echo "int main() {return 0;} int foobar() {return 0;}" > conftest.c
@@ -4834,7 +4855,7 @@ 
     if test x"$default_plugin" != x"yes"; then
       AC_MSG_ERROR([
 Building GCC with plugin support requires a host that supports
--fPIC, -shared, -ldl and -rdynamic.])
+-plugin, -fPIC, -shared, -ldl and -rdynamic.])
     fi
   fi
 fi
diff -r b84c1dcc61f1 gcc/gcc.c
--- a/gcc/gcc.c	Sat Feb 05 09:17:53 2011 +0100
+++ b/gcc/gcc.c	Sat Feb 05 15:12:39 2011 +0100
@@ -627,6 +627,7 @@ 
    not useable.  For GCC 4.6 we don't support slim LTO and thus we can enable
    plugin only when LTO is enabled.  We still honor explicit
    -fuse-linker-plugin.  */
+#ifdef HAVE_LD_PLUGIN
 #ifdef HAVE_LTO_PLUGIN
 #define PLUGIN_COND "!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin"
 #define PLUGIN_COND_CLOSE "}"
@@ -634,7 +635,17 @@ 
 #define PLUGIN_COND "fuse-linker-plugin"
 #define PLUGIN_COND_CLOSE ""
 #endif
-
+#define LINK_PLUGIN_SPEC \
+    "%{"PLUGIN_COND": \
+    -plugin %(linker_plugin_file) \
+    -plugin-opt=%(lto_wrapper) \
+    -plugin-opt=-fresolution=%u.res \
+    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
+    }"PLUGIN_COND_CLOSE
+#else
+#define LINK_PLUGIN_SPEC "%{fuse-linker-plugin:\
+    %e-fuse-linker-plugin is not supported in this configuration}"
+#endif
 
 /* -u* was put back because both BSD and SysV seem to support it.  */
 /* %{static:} simply prevents an error message if the target machine
@@ -647,14 +658,9 @@ 
 #ifndef LINK_COMMAND_SPEC
 #define LINK_COMMAND_SPEC "\
 %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
-    %(linker) \
-    %{"PLUGIN_COND": \
-    -plugin %(linker_plugin_file) \
-    -plugin-opt=%(lto_wrapper) \
-    -plugin-opt=-fresolution=%u.res \
-    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
-    }"PLUGIN_COND_CLOSE" \
-    %{flto|flto=*:%<fcompare-debug*} \
+    %(linker) " \
+    LINK_PLUGIN_SPEC \
+    "%{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\