[PATCH] Fix linking with -findirect-dispatch

[PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Linking with -findirect-dispatch fails with this error:

x86_64-linux-gcj -o ecjx -findirect-dispatch --main=org.eclipse.jdt.internal.compiler.batch.GCCMain ../../../gcc/libjava/../ecj.jar ecjx.o  
/usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: /tmp/ccppO92n.o: undefined reference to symbol '_Jv_MonitorExit'
/usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: note: '_Jv_MonitorExit' is defined in DSO /usr/lib64/libgcj.so.13 so try adding it to the linker command line

Andreas.

	* configure.ac (LIBGCJ_SPEC_LGCJ_BC): Append -lgcj.
	(libgcj_spec_lgcj_bc_override): Likewise.
	* configure: Regenerate.

diff --git a/libjava/configure.ac b/libjava/configure.ac
index ba6b363..d378125 100644
--- a/libjava/configure.ac
+++ b/libjava/configure.ac
@@ -266,7 +266,7 @@ if test "$enable_libgcj_sublibs" = yes ; then
   # words in the string and you're relying on the ordering to
   # append the correct one.
   libgcj_spec_lgcj_override="-lgcj-noncore -lgcj"
-  libgcj_spec_lgcj_bc_override="-lgcj-noncore -lgcj_bc"
+  libgcj_spec_lgcj_bc_override="-lgcj-noncore -lgcj_bc -lgcj"
 fi
 
 
@@ -1165,7 +1165,7 @@ AC_CONFIG_LINKS(sysdep/backtrace.h:$fallback_backtrace_h)
 AC_CONFIG_LINKS(sysdep/descriptor.h:$descriptor_h)
 
 LIBGCJ_SPEC_LGCJ=-lgcj
-LIBGCJ_SPEC_LGCJ_BC=-lgcj_bc
+LIBGCJ_SPEC_LGCJ_BC="-lgcj_bc -lgcj"
 if test x"$libgcj_spec_lgcj_override" != x ; then
   LIBGCJ_SPEC_LGCJ=$libgcj_spec_lgcj_override
 fi
-- 
1.8.2.1

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Jakub Jelinek]

On Tue, Apr 16, 2013 at 10:35:29AM +0200, Andreas Schwab wrote:
> Linking with -findirect-dispatch fails with this error:
> 
> x86_64-linux-gcj -o ecjx -findirect-dispatch --main=org.eclipse.jdt.internal.compiler.batch.GCCMain ../../../gcc/libjava/../ecj.jar ecjx.o  
> /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: /tmp/ccppO92n.o: undefined reference to symbol '_Jv_MonitorExit'
> /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: note: '_Jv_MonitorExit' is defined in DSO /usr/lib64/libgcj.so.13 so try adding it to the linker command line
> 
> Andreas.
> 
> 	* configure.ac (LIBGCJ_SPEC_LGCJ_BC): Append -lgcj.
> 	(libgcj_spec_lgcj_bc_override): Likewise.
> 	* configure: Regenerate.

That doesn't look right, if -findirect-dispatch now newly needs
_Jv_MonitorExit (when has that been added?), then the symbol should
be added to libgcj_bc.so.

	Jakub

Re: [PATCH] Fix linking with -findirect-dispatch

[Bryce McKinlay]

On Tue, Apr 16, 2013 at 9:38 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Apr 16, 2013 at 10:35:29AM +0200, Andreas Schwab wrote:
>> Linking with -findirect-dispatch fails with this error:
>>
>> x86_64-linux-gcj -o ecjx -findirect-dispatch --main=org.eclipse.jdt.internal.compiler.batch.GCCMain ../../../gcc/libjava/../ecj.jar ecjx.o
>> /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: /tmp/ccppO92n.o: undefined reference to symbol '_Jv_MonitorExit'
>> /usr/lib64/gcc/x86_64-suse-linux/4.7/../../../../x86_64-suse-linux/bin/ld: note: '_Jv_MonitorExit' is defined in DSO /usr/lib64/libgcj.so.13 so try adding it to the linker command line
>>
>> Andreas.
>>
>>       * configure.ac (LIBGCJ_SPEC_LGCJ_BC): Append -lgcj.
>>       (libgcj_spec_lgcj_bc_override): Likewise.
>>       * configure: Regenerate.
>
> That doesn't look right, if -findirect-dispatch now newly needs
> _Jv_MonitorExit (when has that been added?), then the symbol should
> be added to libgcj_bc.so.

That's right. -findirect-dispatch code should not link libgcj directly.

_Jv_MonitorExit has always been defined in libgcj_bc (see
libgcj_bc.c), so something else is going wrong here.

Bryce

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> That doesn't look right, if -findirect-dispatch now newly needs
> _Jv_MonitorExit (when has that been added?), then the symbol should
> be added to libgcj_bc.so.

libgcj_bc.so is just a dummy library.  You need to look at the installed
file, not the one in the build directory.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Jakub Jelinek]

On Tue, Apr 16, 2013 at 11:13:35AM +0200, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > That doesn't look right, if -findirect-dispatch now newly needs
> > _Jv_MonitorExit (when has that been added?), then the symbol should
> > be added to libgcj_bc.so.
> 
> libgcj_bc.so is just a dummy library.  You need to look at the installed
> file, not the one in the build directory.

libgcj_bc.so is a dummy library that says what symbols are allowed in
-findirect-dispatch linking at link time, and at dynamic link time it is a
dummy library with no symbols that just adds DT_NEEDED of the latest and
greatest libgcj.so.N, which provides all the symbols.

	Jakub

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> at dynamic link time it is a dummy library with no symbols that just
> adds DT_NEEDED of the latest and greatest libgcj.so.N, which provides
> all the symbols.

Which is exactly the problem.  --no-copy-dt-needed-entries has been the
default for a long time now.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Jakub Jelinek]

On Tue, Apr 16, 2013 at 11:37:07AM +0200, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > at dynamic link time it is a dummy library with no symbols that just
> > adds DT_NEEDED of the latest and greatest libgcj.so.N, which provides
> > all the symbols.
> 
> Which is exactly the problem.  --no-copy-dt-needed-entries has been the
> default for a long time now.

Why would that be a problem?  libgcj.so the linker sees (i.e. the dummy
library) doesn't intentionally have DT_NEEDED libgcj.so.N, programs and
shared libraries linked with -findirect-dispatch should be adding
libgcj_bc.so to DT_NEEDED, not libgcj.so.N.

If this is caused by some recent broken linker change, then that should be
better reverted.

	Jakub

Re: [PATCH] Fix linking with -findirect-dispatch

[Matthias Klose]

Am 16.04.2013 11:48, schrieb Jakub Jelinek:
> On Tue, Apr 16, 2013 at 11:37:07AM +0200, Andreas Schwab wrote:
>> Jakub Jelinek <jakub@redhat.com> writes:
>>
>>> at dynamic link time it is a dummy library with no symbols that just
>>> adds DT_NEEDED of the latest and greatest libgcj.so.N, which provides
>>> all the symbols.
>>
>> Which is exactly the problem.  --no-copy-dt-needed-entries has been the
>> default for a long time now.
> 
> Why would that be a problem?  libgcj.so the linker sees (i.e. the dummy
> library) doesn't intentionally have DT_NEEDED libgcj.so.N, programs and
> shared libraries linked with -findirect-dispatch should be adding
> libgcj_bc.so to DT_NEEDED, not libgcj.so.N.
> 
> If this is caused by some recent broken linker change, then that should be
> better reverted.

I don't see this with binutils 2.23.2.

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> Why would that be a problem?  libgcj.so the linker sees (i.e. the dummy
> library) doesn't intentionally have DT_NEEDED libgcj.so.N, programs and
> shared libraries linked with -findirect-dispatch should be adding
> libgcj_bc.so to DT_NEEDED, not libgcj.so.N.

But the dummy libgcj_bc.so doesn't define _Jv_MonitorExit, or any other
relevant symbol.

> If this is caused by some recent broken linker change, then that should be
> better reverted.

It has been this way since almost two years.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Jakub Jelinek]

On Tue, Apr 16, 2013 at 11:57:54AM +0200, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > Why would that be a problem?  libgcj.so the linker sees (i.e. the dummy
> > library) doesn't intentionally have DT_NEEDED libgcj.so.N, programs and
> > shared libraries linked with -findirect-dispatch should be adding
> > libgcj_bc.so to DT_NEEDED, not libgcj.so.N.
> 
> But the dummy libgcj_bc.so doesn't define _Jv_MonitorExit, or any other
> relevant symbol.

That is not true.  Build from yesterday, on x86_64-linux:

$ readelf -Wa libjava/.libs/libgcj_bc.so | grep _Jv_MonitorExit; readelf -d libjava/.libs/libgcj_bc.so | grep NEEDED; echo ==; readelf -Wa libjava/.libs/libgcj_bc.so.1 | grep _Jv_MonitorExit; readelf -d libjava/.libs/libgcj_bc.so.1 | grep NEEDED
    25: 0000000000001250     2 FUNC    GLOBAL DEFAULT   10 _Jv_MonitorExit
    77: 0000000000001250     2 FUNC    GLOBAL DEFAULT   10 _Jv_MonitorExit
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
==
 0x0000000000000001 (NEEDED)             Shared library: [libgcj.so.14]

The point of -findirect-dispatch linking against -lgcj_bc rather than -lgcj
is that whenever N in libgcj.so.N is bumped, you don't need to rebuild all the
-findirect-dispatch compiled/linked programs and shared libraries, only if
you build a direct dispatch programs or shared libraries.

So, -findirect-dispatch programs and shared libraries should have:
 0x0000000000000001 (NEEDED)             Shared library: [libgcj_bc.so.1]
rather than:
 0x0000000000000001 (NEEDED)             Shared library: [libgcj.so.14]

	Jakub

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> $ readelf -Wa libjava/.libs/libgcj_bc.so

That's not the installed library.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Jakub Jelinek]

On Tue, Apr 16, 2013 at 12:11:06PM +0200, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > $ readelf -Wa libjava/.libs/libgcj_bc.so
> 
> That's not the installed library.

If that isn't the installed library, then the bug is that it isn't
installed.  Our spec file reshuffles libraries around heavily, so perhaps
the upstream Makefiles are buggy and the reshuffling just hides the bug,
but your patch isn't a fix.
Installed */libgcj_bc.so should be a copy of the dummy library
*/libjava/.libs/libgcj_bc.so, and */libgcj_bc.so.1.0.0 should be
a (relinked?) copy of the dummy library */libjava/.libs/libgcj_bc.so.1.0.0.

I'm pretty sure when the changes were added for gcc 4.2 it worked just fine,
but as our rpms don't use what this installs, it is possible it got broken
later on.

	Jakub

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> If that isn't the installed library, then the bug is that it isn't
> installed.

It's always been this way.

install-exec-hook: install-binPROGRAMS install-toolexeclibLTLIBRARIES \
	install-libexecsubPROGRAMS
## Support for libgcj_bc: dummy shared library used only at link-time.
if USE_LIBGCJ_BC
## Install libgcj_bc dummy lib in the target directory. We also need to delete
## libtool's .la file, this prevents libtool resetting the lib again 
## later.
	@echo Installing dummy lib libgcj_bc.so.1.0.0; \
	rm $(DESTDIR)$(toolexeclibdir)/libgcj_bc.so; \
	mv $(DESTDIR)$(toolexeclibdir)/libgcj_bc.so.1.0.0 $(DESTDIR)$(toolexeclibdir)/libgcj_bc.so; \
	$(libgcj_bc_dummy_LINK) -xc /dev/null -Wl,-soname,libgcj_bc.so.1 \
	-o $(DESTDIR)$(toolexeclibdir)/libgcj_bc.so.1.0.0 -lgcj || exit; \
	rm $(DESTDIR)$(toolexeclibdir)/libgcj_bc.so.1; \
	$(LN_S) libgcj_bc.so.1.0.0 $(DESTDIR)$(toolexeclibdir)/libgcj_bc.so.1; \
	rm $(DESTDIR)$(toolexeclibdir)/libgcj_bc.la;
endif

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> I'm pretty sure when the changes were added for gcc 4.2 it worked just fine,

I'm pretty sure it never worked as intented.

Andreas.

	* Makefile.am (toolexeclib_LTLIBRARIES) [USE_LIBGCJ_BC]: Use
	install/libgcj_bc.la instead of libgcj_bc.la.
	(noinst_LTLIBRARIES) [USE_LIBGCJ_BC]: Define.
	(install_libgcj_bc_la_SOURCES): Define.
	(install/libgcj_bc.la): New rule.
	* Makefile.in: Rebuild.
---
 libjava/Makefile.am |  9 +++++-
 libjava/Makefile.in | 92 +++++++++++++++++++++++++++++++++++------------------
 2 files changed, 69 insertions(+), 32 deletions(-)

diff --git a/libjava/Makefile.am b/libjava/Makefile.am
index a4941a9..208e632 100644
--- a/libjava/Makefile.am
+++ b/libjava/Makefile.am
@@ -212,7 +212,8 @@ LIBJAVA_CORE_EXTRA =
 endif
 
 if USE_LIBGCJ_BC
-toolexeclib_LTLIBRARIES += libgcj_bc.la
+toolexeclib_LTLIBRARIES += install/libgcj_bc.la
+noinst_LTLIBRARIES = libgcj_bc.la
 endif
 
 if XLIB_AWT
@@ -606,6 +607,7 @@ lib_gnu_awt_xlib_la_LINK = $(LIBLINK) $(lib_gnu_awt_xlib_la_LDFLAGS) \
 ## This lets us have one soname in BC objects and another in C++ ABI objects.
 ## This library is not linked against libgcj.
 libgcj_bc_la_SOURCES = libgcj_bc.c
+install_libgcj_bc_la_SOURCES = $(libgcj_bc_la_SOURCES)
 libgcj_bc_la_LDFLAGS = -rpath $(toolexeclibdir) -no-static -version-info 1:0:0 \
 	$(LIBGCJ_LD_SYMBOLIC_FUNCTIONS) $(LIBJAVA_LDFLAGS_NOUNDEF)
 libgcj_bc_la_DEPENDENCIES = libgcj.la $(libgcj_bc_la_version_dep)
@@ -628,6 +630,11 @@ libgcj_bc.la: $(libgcj_bc_la_OBJECTS) $(libgcj_bc_la_DEPENDENCIES)
 	rm .libs/libgcj_bc.so.1; \
 	$(LN_S) libgcj_bc.so.1.0.0 .libs/libgcj_bc.so.1
 
+## This rule creates the libgcj_bc library that is actually installed.
+install/libgcj_bc.la: $(libgcj_bc_la_OBJECTS) $(libgcj_bc_la_DEPENDENCIES) install/$(am__dirstamp)
+	$(libgcj_bc_la_LINK) $(am_libgcj_bc_la_rpath) $(libgcj_bc_la_LDFLAGS) \
+	$(libgcj_bc_la_OBJECTS) $(libgcj_bc_la_LIBADD) $(LIBS)
+
 ## Note that property_files is defined in sources.am.
 propertyo_files = $(patsubst classpath/resource/%,%,$(addsuffix .lo,$(property_files)))
 
-- 
1.8.2.1


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Jakub Jelinek]

On Tue, Apr 16, 2013 at 06:13:14PM +0200, Andreas Schwab wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > I'm pretty sure when the changes were added for gcc 4.2 it worked just fine,
> 
> I'm pretty sure it never worked as intented.

Both libraries are actually intended to be installed, not just one.
Does your patch achieve that?

	Jakub

Re: [PATCH] Fix linking with -findirect-dispatch

[Bryce McKinlay]

On Tue, Apr 16, 2013 at 5:13 PM, Andreas Schwab <schwab@suse.de> wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
>
>> I'm pretty sure when the changes were added for gcc 4.2 it worked just fine,
>
> I'm pretty sure it never worked as intented.

It certainly _did_ work as intended previously. Though I've not tested
it for a while.

Bryce

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> Both libraries are actually intended to be installed, not just one.

No, the other library is never installed, but generated on the fly.  See
<mvm8v4iu5q2.fsf@hawking.suse.de>.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Bryce McKinlay <bmckinlay@gmail.com> writes:

> It certainly _did_ work as intended previously.

Only by chance, when libtool has to relink the library during install.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix linking with -findirect-dispatch

[Matthias Klose]

Am 16.04.2013 11:55, schrieb Matthias Klose:
> Am 16.04.2013 11:48, schrieb Jakub Jelinek:
>> On Tue, Apr 16, 2013 at 11:37:07AM +0200, Andreas Schwab wrote:
>>> Jakub Jelinek <jakub@redhat.com> writes:
>>>
>>>> at dynamic link time it is a dummy library with no symbols that just
>>>> adds DT_NEEDED of the latest and greatest libgcj.so.N, which provides
>>>> all the symbols.
>>>
>>> Which is exactly the problem.  --no-copy-dt-needed-entries has been the
>>> default for a long time now.
>>
>> Why would that be a problem?  libgcj.so the linker sees (i.e. the dummy
>> library) doesn't intentionally have DT_NEEDED libgcj.so.N, programs and
>> shared libraries linked with -findirect-dispatch should be adding
>> libgcj_bc.so to DT_NEEDED, not libgcj.so.N.
>>
>> If this is caused by some recent broken linker change, then that should be
>> better reverted.
> 
> I don't see this with binutils 2.23.2.

I do see this now too, however the root of the problem seems to be a linker
which defaults to --as-needed (which is the case on SuSe afaik).  I can see this
without installing anything, just running the testsuite shows some hundred of
these failures.

  Matthias

Re: [PATCH] Fix linking with -findirect-dispatch

[Andrew Haley]

On 04/26/2013 12:22 PM, Matthias Klose wrote:
> I do see this now too, however the root of the problem seems to be a linker
> which defaults to --as-needed (which is the case on SuSe afaik).

Is this a non-standard thing?  So SuSe has a special --configure option
which does this?  We can always patch in --no-as-needed

Andrew.

Re: [PATCH] Fix linking with -findirect-dispatch

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> Both libraries are actually intended to be installed, not just one.
> Does your patch achieve that?

The patch makes sure that the standalone libgcj_bc library is correctly
built, instead of trying to install the dummy libgcj_bc library that
should only be used for testing.  Other than that nothing is changed.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."