jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)

Message ID 1518556781-10049-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288)
Related show

Commit Message

David Malcolm Feb. 13, 2018, 9:19 p.m.
libgccjit fails to link on OS X and Solaris due to jit/Make-lang.in,
due to the assumption there that the linker is GNU ld.  Specifically,
jit/Make-lang.in hardcodes the use of two options: --version-script
and -soname.

* on Darwin, --version-script doesn't seem to exist in the linker, and
  it uses -install_name rather than -soname.

* on Solaris, ld doesn't support --version-script.  However, the version
  script used for libgccjit.so doesn't use any gld extensions, so one can
  just use -M instead.

This patch fixes these issues by using variables emitted by gcc's "configure"
rather than hardcoding the options in jit/Make-lang.in.

It's based on the first part of Rainer's patch for PR jit/84288, but I
made the following changes:
* the GNU ld case in configure.ac wasn't setting ld_version_script_option.
  I set it to "--version-script" for that case.
* I moved the:
    LD_VERSION_SCRIPT_OPTION = @ld_version_script_option@
  from gcc/jit/Make-lang.in to gcc/Makefile.in, as the Make-lang.in files
  aren't substituted, only the gcc/Makefile.in.
  Rainer: how did this work for you?
* added LD_SONAME_OPTION, done in the same way
* conditionalized the usage of the options in Make-lang.in to cope with
  empty LD_VERSION_SCRIPT_OPTION (as is presumably the case on OS X).
  I used ($if condition,then-part[,else-part]) for this.
  I had to add a $(COMMA) since the "then-part" contains commas, which
  need to be treated as part of the "then-part", rather than separators
  for the "else-part".
  Hopefully this is compatible with every "make" implementation that we
  support.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

I lightly tested the not-recognized case by hacking up the configure.ac
(on x86_64-pc-linux-gnu) and verifying that it links, and that a
smoketest of jit.dg/test-factorial works.

Does this fix the jit linker issues on OS X and Solaris?
(I didn't include the autogenerate configure changes)

gcc/ChangeLog:
	PR jit/64089
	PR jit/84288
	* Makefile.in (LD_VERSION_SCRIPT_OPTION, LD_SONAME_OPTION): New.
	* configure: Regenerate.
	* configure.ac ("linker --version-script option"): New.
	("linker soname option"): New.

gcc/jit/ChangeLog:
	PR jit/64089
	PR jit/84288
	* Make-lang.in (COMMA): New.
	(LIBGCCJIT_VERSION_SCRIPT_OPTION): New.
	(LIBGCCJIT_SONAME_OPTION): New.
	(jit): Move --version-script and -soname linker options to the
	above.
---
 gcc/Makefile.in      |  4 ++++
 gcc/configure.ac     | 38 ++++++++++++++++++++++++++++++++++++++
 gcc/jit/Make-lang.in | 17 +++++++++++++++--
 3 files changed, 57 insertions(+), 2 deletions(-)

Comments

Rainer Orth Feb. 13, 2018, 9:30 p.m. | #1
Hi David,

> libgccjit fails to link on OS X and Solaris due to jit/Make-lang.in,
> due to the assumption there that the linker is GNU ld.  Specifically,
> jit/Make-lang.in hardcodes the use of two options: --version-script
> and -soname.
>
> * on Darwin, --version-script doesn't seem to exist in the linker, and
>   it uses -install_name rather than -soname.
>
> * on Solaris, ld doesn't support --version-script.  However, the version
>   script used for libgccjit.so doesn't use any gld extensions, so one can
>   just use -M instead.
>
> This patch fixes these issues by using variables emitted by gcc's "configure"
> rather than hardcoding the options in jit/Make-lang.in.
>
> It's based on the first part of Rainer's patch for PR jit/84288, but I
> made the following changes:
> * the GNU ld case in configure.ac wasn't setting ld_version_script_option.
>   I set it to "--version-script" for that case.

That's weird: the configure.ac part starts with

ld_version_script_option='--version-script'

only overriding it in the Solaris case to keep things for other hosts
as they were.  Besides, I'm pretty sure I tested the patch on Solaris
with gas/gld to make certain that combo continues to work as is...

> * I moved the:
>     LD_VERSION_SCRIPT_OPTION = @ld_version_script_option@
>   from gcc/jit/Make-lang.in to gcc/Makefile.in, as the Make-lang.in files
>   aren't substituted, only the gcc/Makefile.in.
>   Rainer: how did this work for you?

It didn't: what I'd attached to the PR was an initial version of the
patch in the middle between manually hacking gcc/jit/Make-lang.in and
properly autoconfiguring things.

> * added LD_SONAME_OPTION, done in the same way
> * conditionalized the usage of the options in Make-lang.in to cope with
>   empty LD_VERSION_SCRIPT_OPTION (as is presumably the case on OS X).
>   I used ($if condition,then-part[,else-part]) for this.
>   I had to add a $(COMMA) since the "then-part" contains commas, which
>   need to be treated as part of the "then-part", rather than separators
>   for the "else-part".
>   Hopefully this is compatible with every "make" implementation that we
>   support.
>
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
>
> I lightly tested the not-recognized case by hacking up the configure.ac
> (on x86_64-pc-linux-gnu) and verifying that it links, and that a
> smoketest of jit.dg/test-factorial works.
>
> Does this fix the jit linker issues on OS X and Solaris?

I'll give it a whirl tomorrow, including the jit-recording.c part of my
patch to allow the build to complete.

Thanks.
        Rainer
Rainer Orth Feb. 14, 2018, 1:36 p.m. | #2
Hi David,

>> * added LD_SONAME_OPTION, done in the same way
[...]
>> Does this fix the jit linker issues on OS X and Solaris?
>
> I'll give it a whirl tomorrow, including the jit-recording.c part of my
> patch to allow the build to complete.

actually, I've replaced the Makefile and configure parts of my patch
with yours and did a jit-only bootstrap on i386-pc-solaris2.11 with
as/ld and gas/ld.  Both went fine with a minor caveat: I noticed that
LD_SONAME_OPTION wasn't set in gcc/Makefile.  Fixed with the following
(so far untested) snippet:
diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3715,6 +3715,12 @@ elif test x$gcc_cv_ld != x; then
       gcc_cv_ld_soname=yes
       ld_soname_option='-install_name'
       ;;
+    # Solaris 2 ld always supports -h.  It also supports --soname for GNU
+    # ld compatiblity since some Solaris 10 update.
+    *-*-solaris2*)
+      gcc_cv_ld_soname=yes
+      ld_soname_option='-h'
+      ;;
   esac
 fi
 # Don't AC_DEFINE result, only used in jit/Make-lang.in so far.
I've also checked that the original Solaris 10 release didn't support ld
-soname, so it's safer to always use the Solaris-native -h option
instead.

	Rainer
FX Feb. 14, 2018, 5:28 p.m. | #3
> Does this fix the jit linker issues on OS X and Solaris?

The patch fails to bootstrap on x86_64-apple-darwin17. gcc/config.log says:

gcc_cv_ld_version_script=no
ld_version_script_option='--version-script’

gcc/Makefile says:

LD_VERSION_SCRIPT_OPTION = --version-script
LD_SONAME_OPTION = -install_name

which makes it try to link with:

	      -Wl,--version-script,../../trunk/gcc/jit/libgccjit.map \
	      -Wl,-install_name,libgccjit.so.0

which fails with: ld: unknown option: --version-script

I think the patch to gcc/configure.ac should be:

+AC_MSG_CHECKING(linker --version-script option)
+gcc_cv_ld_version_script=no
+ld_version_script_option=''


FX
FX Feb. 14, 2018, 10:36 p.m. | #4
I can confirm that, with the attached revised patch, a bootstrap with --enable-languages=c,c++,jit --enable-host-shared is successful on macOS.

FX

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 6c37e46..903da58 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1116,6 +1116,10 @@  endif
 
 LANG_MAKEFRAGS = @all_lang_makefrags@
 
+# Used by gcc/jit/Make-lang.in
+LD_VERSION_SCRIPT_OPTION = @ld_version_script_option@
+LD_SONAME_OPTION = @ld_soname_option@
+
 # Flags to pass to recursive makes.
 # CC is set by configure.
 # ??? The choices here will need some experimenting with.
diff --git a/gcc/configure.ac b/gcc/configure.ac
index b7f9728..265920c 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3640,6 +3640,44 @@  if test x"$gcc_cv_ld_static_dynamic" = xyes; then
 fi
 AC_MSG_RESULT($gcc_cv_ld_static_dynamic)
 
+AC_MSG_CHECKING(linker --version-script option)
+gcc_cv_ld_version_script=no
+ld_version_script_option='--version-script'
+if test $in_tree_ld = yes || test x"$gnu_ld" = xyes; then
+  gcc_cv_ld_version_script=yes
+  ld_version_script_option='--version-script'
+elif test x$gcc_cv_ld != x; then
+  case "$target" in
+    # Solaris 2 ld always supports -M.  It also supports a subset of
+    # --version-script since Solaris 11.4, but requires
+    # -z gnu-version-script-compat to activate.
+    *-*-solaris2*)
+      gcc_cv_ld_version_script=yes
+      ld_version_script_option='-M'
+      ;;
+  esac
+fi
+# Don't AC_DEFINE result, only used in jit/Make-lang.in so far.
+AC_MSG_RESULT($gcc_cv_ld_version_script)
+AC_SUBST(ld_version_script_option)
+
+AC_MSG_CHECKING(linker soname option)
+gcc_cv_ld_soname=no
+if test $in_tree_ld = yes || test x"$gnu_ld" = xyes; then
+  gcc_cv_ld_soname=yes
+  ld_soname_option='-soname'
+elif test x$gcc_cv_ld != x; then
+  case "$target" in
+    *-*-darwin*)
+      gcc_cv_ld_soname=yes
+      ld_soname_option='-install_name'
+      ;;
+  esac
+fi
+# Don't AC_DEFINE result, only used in jit/Make-lang.in so far.
+AC_MSG_RESULT($gcc_cv_ld_soname)
+AC_SUBST(ld_soname_option)
+
 if test x"$demangler_in_ld" = xyes; then
   AC_MSG_CHECKING(linker --demangle support)
   gcc_cv_ld_demangle=no
diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in
index d4362a9..ba78f8e 100644
--- a/gcc/jit/Make-lang.in
+++ b/gcc/jit/Make-lang.in
@@ -51,6 +51,19 @@  LIBGCCJIT_FILENAME = \
 LIBGCCJIT_LINKER_NAME_SYMLINK = $(LIBGCCJIT_LINKER_NAME)
 LIBGCCJIT_SONAME_SYMLINK = $(LIBGCCJIT_SONAME)
 
+# Conditionalize the use of the LD_VERSION_SCRIPT_OPTION and
+# LD_SONAME_OPTION depending if configure found them, using $(if)
+# We have to define a COMMA here, otherwise the commas in the "true"
+# result are treated as separators by the $(if).
+COMMA := ,
+LIBGCCJIT_VERSION_SCRIPT_OPTION = \
+	$(if $(LD_VERSION_SCRIPT_OPTION),\
+	  -Wl$(COMMA)$(LD_VERSION_SCRIPT_OPTION)$(COMMA)$(srcdir)/jit/libgccjit.map)
+
+LIBGCCJIT_SONAME_OPTION = \
+	$(if $(LD_SONAME_OPTION), \
+	     -Wl$(COMMA)$(LD_SONAME_OPTION)$(COMMA)$(LIBGCCJIT_SONAME))
+
 jit: $(LIBGCCJIT_FILENAME) \
 	$(LIBGCCJIT_SYMLINK) \
 	$(LIBGCCJIT_LINKER_NAME_SYMLINK) \
@@ -85,8 +98,8 @@  $(LIBGCCJIT_FILENAME): $(jit_OBJS) \
 	     $(jit_OBJS) libbackend.a libcommon-target.a libcommon.a \
 	     $(CPPLIB) $(LIBDECNUMBER) $(LIBS) $(BACKENDLIBS) \
 	     $(EXTRA_GCC_OBJS) \
-	     -Wl,--version-script=$(srcdir)/jit/libgccjit.map \
-	     -Wl,-soname,$(LIBGCCJIT_SONAME)
+	     $(LIBGCCJIT_VERSION_SCRIPT_OPTION) \
+	     $(LIBGCCJIT_SONAME_OPTION)
 
 $(LIBGCCJIT_SONAME_SYMLINK): $(LIBGCCJIT_FILENAME)
 	ln -sf $(LIBGCCJIT_FILENAME) $(LIBGCCJIT_SONAME_SYMLINK)